Skip to content

Conversation

@DaanHoogland
Copy link
Contributor

Description

This PR...

Fixes: #8956

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

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?

@DaanHoogland DaanHoogland changed the base branch from main to 4.19 April 26, 2024 07:10
@DaanHoogland DaanHoogland changed the title Use parameter dcId as wrapperto prevent NPE Use parameter dcId as wrapper to prevent NPE Apr 26, 2024
Copy link
Member

@weizhouapache weizhouapache 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

not tested yet

@weizhouapache
Copy link
Member

@blueorangutan package

@DaanHoogland
Copy link
Contributor Author

@rajujith, can you test your scenario with this?

@apache apache deleted a comment from blueorangutan Apr 26, 2024
@apache apache deleted a comment from blueorangutan Apr 26, 2024
@DaanHoogland
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland 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.

@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 14.96%. Comparing base (e409c6d) to head (f053582).

Files Patch % Lines
...tack/storage/endpoint/DefaultEndPointSelector.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               4.19    #8986    +/-   ##
==========================================
  Coverage     14.96%   14.96%            
- Complexity    10995    10998     +3     
==========================================
  Files          5373     5373            
  Lines        468989   468989            
  Branches      61009    60699   -310     
==========================================
+ Hits          70191    70202    +11     
+ Misses       391019   391006    -13     
- Partials       7779     7781     +2     
Flag Coverage Δ
uitests 4.31% <ø> (ø)
unittests 15.67% <0.00%> (+<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.

@blueorangutan
Copy link

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

Copy link
Contributor

@JoaoJandre JoaoJandre left a comment

Choose a reason for hiding this comment

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

CLGTM, I have no way to test the scenario on #8956.

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

clgtm

@KlausDornsbach
Copy link
Contributor

CLGTM.

Also done some testing:

  1. Remove NFS secondary storage.
  2. Create AWS Bucket, generate access and secret keys.
  3. Add the S3 secondary storage bucket to ACS with success and no errors on logs.

@weizhouapache
Copy link
Member

@blueorangutan test matrix

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-10055)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48254 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8986-t10055-xenserver-71.zip
Smoke tests completed. 129 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 348.66 test_events_resource.py

@blueorangutan
Copy link

[SF] Trillian test result (tid-10057)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48458 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8986-t10057-kvm-centos7.zip
Smoke tests completed. 129 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 410.46 test_events_resource.py

@blueorangutan
Copy link

[SF] Trillian test result (tid-10056)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 52438 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8986-t10056-vmware-67u3.zip
Smoke tests completed. 127 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 344.92 test_events_resource.py
test_02_balanced_drs_algorithm Error 438.45 test_cluster_drs.py
test_02_trigger_shutdown Failure 341.66 test_safe_shutdown.py

@DaanHoogland
Copy link
Contributor Author

@blueorangutan test alma9 vmware-80u1

@blueorangutan
Copy link

@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + vmware-80u1) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian Build Failed (tid-10069)

@DaanHoogland
Copy link
Contributor Author

@blueorangutan test alma9 vmware-70u3

@blueorangutan
Copy link

@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + vmware-70u3) has been kicked to run smoke tests

Copy link

@rajujith rajujith left a comment

Choose a reason for hiding this comment

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

LGTM.
I can add the S3 bucket as a secondary storage along with an NFS staging storage. I used a Minio S3 bucket.

@blueorangutan
Copy link

[SF] Trillian test result (tid-10071)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server a9
Total time taken: 49703 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8986-t10071-vmware-70u3.zip
Smoke tests completed. 128 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 330.87 test_events_resource.py
test_02_balanced_drs_algorithm Failure 132.44 test_cluster_drs.py

@DaanHoogland
Copy link
Contributor Author

/me investigating the test_02_balanced_drs_algorithm failure.

@DaanHoogland
Copy link
Contributor Author

@blueorangutan test alma9 vmware-70u3 keepEnv

@blueorangutan
Copy link

@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + vmware-70u3) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-10082)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server a9
Total time taken: 50727 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8986-t10082-vmware-70u3.zip
Smoke tests completed. 128 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 369.36 test_events_resource.py
test_02_trigger_shutdown Failure 346.47 test_safe_shutdown.py

@DaanHoogland DaanHoogland merged commit e520525 into apache:4.19 May 1, 2024
@DaanHoogland DaanHoogland deleted the ghi8956-zoneIdAsWrapper branch May 1, 2024 07:12
DaanHoogland added a commit that referenced this pull request May 6, 2024
* 4.19:
  linstor: disconnect-disk also search for resource name in Linstor (#9035)
  ui: add support to change Account role for admins (#9012)
  Use parameter dcId as wrapper to prevent NPE (#8986)
@DaanHoogland DaanHoogland added this to the 4.19.1.0 milestone May 7, 2024
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request May 17, 2024
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request May 17, 2024
* 4.19:
  linstor: disconnect-disk also search for resource name in Linstor (apache#9035)
  ui: add support to change Account role for admins (apache#9012)
  Use parameter dcId as wrapper to prevent NPE (apache#8986)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

Adding S3 secondary storage fails with NPE

8 participants