Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configkey to set 'cloud-name' cloud-init metadata #7964

Merged
merged 7 commits into from Sep 26, 2023

Conversation

mlsorensen
Copy link
Contributor

Description

This PR allows admin to set a "cloud-name" for cloud-init metadata. Configdrive has no special handling to determine what cloud it is being run on, it just generically sets the cloud-name to the datasource name. By providing this metadata key, we can allow admin to configure this for users.

Followed the same pattern for adding "cloud-domain" and other metadata.

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)

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?

Tested locally by first keeping the value unset and confirming there is no change to the data

# cloud-init query cloud-name
configdrive

Then set the config key metadata.custom.cloud.name and confirmed it was reflected in the VM

# cloud-init query cloud-name
marcuscloud

Also tested it works with VR as userdata provider:

$ curl http://192.168.151.10/latest/meta-data/cloud-name
marcuscloud

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #7964 (00dcaf7) into main (f049d4d) will increase coverage by 16.08%.
Report is 185 commits behind head on main.
The diff coverage is 50.00%.

@@              Coverage Diff              @@
##               main    #7964       +/-   ##
=============================================
+ Coverage     13.06%   29.14%   +16.08%     
- Complexity     9098    30374    +21276     
=============================================
  Files          2720     5100     +2380     
  Lines        257465   358328   +100863     
  Branches      40145    52310    +12165     
=============================================
+ Hits          33633   104439    +70806     
- Misses       219605   239530    +19925     
- Partials       4227    14359    +10132     
Flag Coverage Δ
simulator-marvin-tests 25.13% <37.50%> (?)
uitests 4.87% <ø> (?)
unit-tests 14.42% <28.57%> (?)

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

Files Changed Coverage Δ
...n/java/com/cloud/vm/VirtualMachineManagerImpl.java 37.59% <ø> (+31.23%) ⬆️
.../main/java/com/cloud/network/NetworkModelImpl.java 48.78% <33.33%> (+37.66%) ⬆️
...a/com/cloud/network/router/CommandSetupHelper.java 70.73% <33.33%> (+69.75%) ⬆️
.../src/main/java/com/cloud/network/NetworkModel.java 100.00% <100.00%> (ø)
.../main/java/com/cloud/vm/VirtualMachineManager.java 92.85% <100.00%> (+0.54%) ⬆️

... and 3666 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

mlsorensen and others added 3 commits September 15, 2023 09:21
Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>
Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>
…er.java

Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>
Copy link
Member

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

@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland a [SF] 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7093

@DaanHoogland
Copy link
Contributor

@blueorangutan test matrix

@blueorangutan
Copy link

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

Copy link
Contributor

@GutoVeronezi GutoVeronezi left a comment

Choose a reason for hiding this comment

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

CLGTM

Did not test the changes, though.

@blueorangutan
Copy link

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

Test Result Time (s) Test File

@blueorangutan
Copy link

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

Test Result Time (s) Test File

@rohityadavcloud rohityadavcloud added this to the 4.19.0.0 milestone Sep 26, 2023
@rohityadavcloud rohityadavcloud merged commit 155a307 into apache:main Sep 26, 2023
27 of 28 checks passed
@blueorangutan
Copy link

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

Test Result Time (s) Test File
test_list_vms_metrics_admin Error 3619.31 test_metrics_api.py
test_list_vms_metrics_history Error 4.67 test_metrics_api.py
test_list_volumes_metrics_history Error 3604.72 test_metrics_api.py
test_01_deploy_vm_on_specific_host Error 3602.91 test_vm_deployment_planner.py
test_02_deploy_vm_on_specific_cluster Error 24.86 test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_pod Error 3602.86 test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster Error 9.57 test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_pod Error 1.34 test_vm_deployment_planner.py
test_13_migrate_volume_and_change_offering Error 21.82 test_volumes.py
ContextSuite context=TestVolumes>:teardown Error 34.76 test_volumes.py

shwstppr pushed a commit to shapeblue/cloudstack that referenced this pull request Oct 12, 2023
* Allow configkey to set 'cloud-name' cloud-init metadata

* Update engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java

Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>

* Update server/src/main/java/com/cloud/network/NetworkModelImpl.java

Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>

* Update server/src/main/java/com/cloud/network/router/CommandSetupHelper.java

Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>

* Revert "Update server/src/main/java/com/cloud/network/router/CommandSetupHelper.java"

This reverts commit 8abc3e3.

* Revert "Update server/src/main/java/com/cloud/network/NetworkModelImpl.java"

This reverts commit 7f239be.

* Rework/Fix review code suggestions

---------

Co-authored-by: Marcus Sorensen <mls@apple.com>
Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>
(cherry picked from commit 155a307)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
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.

None yet

6 participants