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

userdata: fix append scenarios #7741

Merged
merged 7 commits into from
Jul 19, 2023

Conversation

shwstppr
Copy link
Contributor

@shwstppr shwstppr commented Jul 12, 2023

Description

  • Fixes case of appending userdata when both template and vm data are either shellscript or cloudconfig
  • Fixes error when appending gzip userdata
  • Fixes case when userdata manual text from VM is not getting decoded-encoded correctly.
  • Fixes case of appending multipart data when both template and vm data contain same format types.
  • Refactor - moved validateUserData method to UserDataManager class
  • Refactor userdata test to check resultant multipart userdata thoroughly

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?

- Fixes case of appending userdata when both template and vm data are either shellscript or cloudconfig
- Fixes error when appending gzip userdata

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

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

@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #7741 (0db0f43) into main (caaf25b) will increase coverage by 0.01%.
The diff coverage is 68.04%.

@@             Coverage Diff              @@
##               main    #7741      +/-   ##
============================================
+ Coverage     13.41%   13.42%   +0.01%     
- Complexity     9365     9381      +16     
============================================
  Files          2745     2746       +1     
  Lines        258683   258695      +12     
  Branches      40291    40288       -3     
============================================
+ Hits          34691    34733      +42     
+ Misses       219621   219590      -31     
- Partials       4371     4372       +1     
Impacted Files Coverage Δ
.../cloud/configuration/ConfigurationManagerImpl.java 16.08% <ø> (-0.03%) ⬇️
...rver/src/main/java/com/cloud/vm/UserVmManager.java 100.00% <ø> (ø)
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java 7.78% <25.00%> (-0.09%) ⬇️
...pache/cloudstack/userdata/UserDataManagerImpl.java 23.40% <33.33%> (+23.40%) ⬆️
...cloudstack/userdata/CloudInitUserDataProvider.java 80.43% <84.12%> (+0.64%) ⬆️
.../com/cloud/configuration/ConfigurationManager.java 100.00% <100.00%> (ø)
...in/java/com/cloud/server/ManagementServerImpl.java 5.36% <100.00%> (-0.17%) ⬇️

... and 1 file with indirect coverage changes

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

@blueorangutan
Copy link

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

@shwstppr
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

@shwstppr a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-7062)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43226 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7741-t7062-kvm-centos7.zip
Smoke tests completed. 111 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_migrate_VM_and_root_volume Error 79.91 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 58.69 test_vm_life_cycle.py
test_hostha_enable_ha_when_host_in_maintenance Error 303.94 test_hostha_kvm.py
test_hostha_kvm_host_recovering Error 9.29 test_hostha_kvm.py

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.

@shwstppr can we target this to 4.18.1 ?

@shwstppr
Copy link
Contributor Author

@harikrishna-patnala based it on main as #7643 was merged on main

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, did not test it though

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr 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 6478

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr 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 6479

@shwstppr
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

@shwstppr a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-7096)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42800 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7741-t7096-kvm-centos7.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_08_arping_in_ssvm Failure 5.18 test_diagnostics.py
test_09_arping_in_cpvm Failure 5.20 test_diagnostics.py
test_deploy_vm_with_registered_userdata_with_override_policy_append Failure 109.77 test_register_userdata.py
test_01_migrate_VM_and_root_volume Error 85.09 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 51.49 test_vm_life_cycle.py

Copy link
Contributor

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

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

LGTM, manually tested all the possible scenarios

Registerd Userdata1 format Registered Userdata2 format Vm-userdata Content of userdata Result Comments
Cloud-config (text/cloud-config) Plain text Pass
Cloud-config (text/cloud-config) Shellscript Mime type Pass
Shell script Cloud-config (text/cloud-config) Mime type Pass
Cloud-config (text/cloud-config) gzip (user-data) Pass Error attempting to merge user data as a multipart user data. Reason: Gzipped user data can not be used together with other user data formats
gzip (user-data) Gzip data Pass
Cloud-config (mimetype) Mime type
Cloud-config (mimetype) Mime type Pass
Cloud-config (mimetype) gzip (user-data) Pass Error attempting to merge user data as a multipart user data. Reason: Gzipped user data can not be used together with other user data formats
Shell script gzip (user-data) Pass Error attempting to merge user data as a multipart user data. Reason: Gzipped user data can not be used together with other user data formats
Cloud-config (mimetype) Shell script Mime type Pass
Shell script Cloud-config (mimetype) Pass
Cloud-config (text/cloud-config) Cloud-config (text/cloud-config) Plain text Pass
Cloud-config (text/cloud-config) Cloud-config (text/cloud-config) Plain text Pass
Cloud-config (text/cloud-config) Shell script (directly pasting in the UI ) Mime type Pass
Shell script Cloud-config (text/cloud-config)directly pasting in the UI Mime type Pass
Cloud-config (mimetype) Cloud-config (mimetype) Mime type Pass

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@boring-cyborg boring-cyborg bot added component:integration-test Python Warning... Python code Ahead! labels Jul 18, 2023
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr 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 6497

@kiranchavala
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@kiranchavala a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-7102)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41302 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7741-t7102-kvm-centos7.zip
Smoke tests completed. 112 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_migrate_VM_and_root_volume Error 76.56 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 53.33 test_vm_life_cycle.py

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

@shwstppr shwstppr marked this pull request as ready for review July 19, 2023 09:20
@rohityadavcloud rohityadavcloud added this to the 4.19.0.0 milestone Jul 19, 2023
@rohityadavcloud rohityadavcloud merged commit 729e6d1 into apache:main Jul 19, 2023
26 of 27 checks passed
@rohityadavcloud rohityadavcloud deleted the fix-userdata-append-gzip branch July 19, 2023 09:48
shwstppr added a commit to shapeblue/cloudstack that referenced this pull request Oct 12, 2023
Fixes case of appending userdata when both template and vm data are either shellscript or cloudconfig
Fixes error when appending gzip userdata
Fixes case when userdata manual text from VM is not getting decoded-encoded correctly.
Fixes case of appending multipart data when both template and vm data contain same format types.
Refactor - moved validateUserData method to UserDataManager class
Refactor userdata test to check resultant multipart userdata thoroughly

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
(cherry picked from commit 729e6d1)
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.

5 participants