Skip to content

Conversation

@shwstppr
Copy link
Contributor

Description

Fixes #10163

Based on #10163 (comment)

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@codecov
Copy link

codecov bot commented Apr 16, 2025

Codecov Report

❌ Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.17%. Comparing base (db5b6a5) to head (1b02b91).
⚠️ Report is 20 commits behind head on 4.20.

Files with missing lines Patch % Lines
.../storage/resource/NfsSecondaryStorageResource.java 76.92% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               4.20   #10735   +/-   ##
=========================================
  Coverage     16.17%   16.17%           
- Complexity    13286    13299   +13     
=========================================
  Files          5656     5656           
  Lines        498015   498091   +76     
  Branches      60406    60416   +10     
=========================================
+ Hits          80538    80589   +51     
- Misses       408515   408536   +21     
- Partials       8962     8966    +4     
Flag Coverage Δ
uitests 4.00% <ø> (-0.01%) ⬇️
unittests 17.03% <76.92%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13072

@shwstppr shwstppr closed this May 16, 2025
@weizhouapache
Copy link
Member

@shwstppr
I would like to re-open and test this PR against 4.20
what do you think ?

@shwstppr shwstppr reopened this Sep 4, 2025
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@shwstppr shwstppr changed the base branch from 4.19 to 4.20 September 4, 2025 10:53
@shwstppr
Copy link
Contributor Author

shwstppr commented Sep 4, 2025

@weizhouapache I've re-opened and rebased it for 4.20

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@weizhouapache
Copy link
Member

@weizhouapache I've re-opened and rebased it for 4.20
thanks @shwstppr , I will test it

@weizhouapache weizhouapache self-assigned this Sep 4, 2025
@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-14210)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 66385 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10735-t14210-kvm-ol8.zip
Smoke tests completed. 138 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_DeleteDomain Error 364.93 test_accounts.py
test_forceDeleteDomain Failure 104.64 test_accounts.py
ContextSuite context=TestRemoteDiagnostics>:setup Error 0.00 test_diagnostics.py
test_list_vms_metrics_admin Error 3610.25 test_metrics_api.py
test_list_vms_metrics_history Error 6.53 test_metrics_api.py
test_list_volumes_metrics_history Error 4.46 test_metrics_api.py

@shwstppr shwstppr marked this pull request as ready for review September 7, 2025 05:16
@weizhouapache
Copy link
Member

@shwstppr
the issue #10163 is present in my testing

it looks like we need to set the storage netmask and gateway too

diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
index 454b98d0f6e..4eb45f2b5b9 100644
--- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
+++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
@@ -2744,8 +2744,8 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         if (_storageIp == null && _inSystemVM) {
             logger.warn("There is no storageip in /proc/cmdline, something wrong!");
         }
-        _storageNetmask = (String)params.get("storagenetmask");
-        _storageGateway = (String)params.get("storagegateway");
+        _storageNetmask = MapUtils.getString(params, "storagenetmask", _eth1mask);
+        _storageGateway = MapUtils.getString(params, "storagegateway", (String)params.get("localgw"));
         super.configure(name, params);
 
         _params = params;

Signed-off-by: Abhishek Kumar <[email protected]>
@shwstppr
Copy link
Contributor Author

shwstppr commented Sep 8, 2025

@weizhouapache thanks for the tests. I've made some changes

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14905

Signed-off-by: Abhishek Kumar <[email protected]>
@shwstppr
Copy link
Contributor Author

shwstppr commented Sep 8, 2025

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14907

@weizhouapache
Copy link
Member

@blueorangutan test

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian Build Failed (tid-14248)

@weizhouapache
Copy link
Member

@blueorangutan test

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-14251)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 62036 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10735-t14251-kvm-ol8.zip
Smoke tests completed. 139 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_04_extract_template Failure 133.10 test_templates.py
test_06_download_detached_volume Failure 436.77 test_volumes.py

@blueorangutan
Copy link

[SF] Trillian test result (tid-14255)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 63889 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10735-t14255-kvm-ol8.zip
Smoke tests completed. 141 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@weizhouapache
Copy link
Member

Verified OK

root@s-15-VM:~# grep 'Storage network not configured' /var/log/cloud.log 
2025-09-10T07:40:53,726 INFO  [storage.resource.NfsSecondaryStorageResource] (main:[]) Storage network not configured, using management network[ip: 10.0.42.34, netmask: 255.255.240.0, gateway: 10.0.32.1] for storage traffic

root@s-15-VM:~# grep 'ip route add' /var/log/cloud.log 
root@s-15-VM:~# 

Copy link
Contributor

@harikrishna-patnala harikrishna-patnala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM

@weizhouapache
Copy link
Member

Merging based on approvals and manual test results

@weizhouapache weizhouapache merged commit 38006b2 into apache:4.20 Sep 11, 2025
26 checks passed
@DaanHoogland DaanHoogland deleted the fix-nfs-static-route branch September 15, 2025 07:16
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Sep 15, 2025
* ssvm: use mgmt network if no storage network

Fixes apache#10163

Based on apache#10163 (comment)

Signed-off-by: Abhishek Kumar <[email protected]>

* update

Signed-off-by: Abhishek Kumar <[email protected]>

* fix

Signed-off-by: Abhishek Kumar <[email protected]>

---------

Signed-off-by: Abhishek Kumar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSVM System VM fails to add static route to NFS server (ip route add x.x.x.x via null)

4 participants