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

Fix systemVM template name in metadata file #5598

Merged
merged 1 commit into from Oct 21, 2021

Conversation

Pearl1594
Copy link
Contributor

@Pearl1594 Pearl1594 commented Oct 21, 2021

Description

This PR fixes the issue observed during upgrades, wherein, when a SystemVM template is already registered prior upgrade, the auto-upgrade path still gets invoked - which it shouldn't. The reason for this behavior is the incorrect name of the systemVM template placed in the metadata.ini file, which is used as an input during the upgrade process to identify the template names, checksums, download path, etc. In case of noredist builds, this would cause unnecessary re-registration; in case of non noredist builds, this could lead to the upgrade failing, as it fails to identify the registered template and invokes the auto-upgrade logic - which would fail(expected) due to the absence of the template files.

For ex, a snippet of the metadata.ini file (for KVM SystemVM template details)

...
[kvm]
templatename = systemvm-kvm-4.16 <--- incorrect template name (should be systemvm-kvm-4.16.0)
checksum = 2f3747a597396d4ee6a2605648808d1a
downloadurl = https://download.cloudstack.org/systemvm/4.16/systemvmtemplate-4.16.0-kvm.qcow2.bz2
filename = systemvmtemplate-4.16.0-kvm.qcow2.bz2
...

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

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Upgraded the env from 4.15.2 to 4.16, by first registering the systemVM template - upgrade successful & doesn't re-register the template as template is found to be already present.

Pos Fix, the snippet of metadata.ini file looks as follows:

...
[kvm]
templatename = systemvm-kvm-4.16.0
checksum = 2f3747a597396d4ee6a2605648808d1a
downloadurl = https://download.cloudstack.org/systemvm/4.16/systemvmtemplate-4.16.0-kvm.qcow2.bz2
filename = systemvmtemplate-4.16.0-kvm.qcow2.bz2
...

@vladimirpetrov
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@vladimirpetrov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@rohityadavcloud
Copy link
Member

@Pearl1594 @vladimirpetrov I suppose we'll need to do oss (or non- noredist) packages build, or @GutoVeronezi can you help review/test this?

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

code looks good and makes functional sense, but a security update might force us to issue a tiny number for a new template, i.e. 4.16.1.1
Do we cater for such a scenario?

@Pearl1594
Copy link
Contributor Author

@DaanHoogland It doesn't currently the naming convention considered is systemvm--x.y.z
I wasn't actually aware that we do have security updates where in the version is incremented as you mentioned - 4.16.x.y - however, does the systemvm template registration naming convention change? i.e., do we register the template with the name systemvm-kvm-4.16.1.1 or it continues to be systemvm-kvm-4.16.1 ?

@blueorangutan
Copy link

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1598

@Pearl1594
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

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

@GutoVeronezi
Copy link
Contributor

@Pearl1594 @vladimirpetrov I suppose we'll need to do oss (or non- noredist) packages build, or @GutoVeronezi can you help review/test this?

I will do some testing and return with the results.

Copy link
Contributor

@vladimirpetrov vladimirpetrov left a comment

Choose a reason for hiding this comment

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

LGTM based on manual testing, tested with regular and oss build.

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.

LGTM

I builded the packages without noredis, rolled back my lab to 4.15, seeded systemvm-template-4.16 and then upgraded the lab with 4.16. All worked as expected.

@nvazquez
Copy link
Contributor

Thanks @GutoVeronezi @vladimirpetrov @rhtyd @DaanHoogland, merging

@nvazquez nvazquez merged commit f2ca11f into apache:main Oct 21, 2021
@blueorangutan
Copy link

Trillian test result (tid-2454)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31482 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5598-t2454-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test Result Time (s) Test File

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

8 participants