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

Improve CloudStack E2E template selection logic #5923

Merged
merged 1 commit into from
May 31, 2023

Conversation

panktishah26
Copy link
Member

@panktishah26 panktishah26 commented May 25, 2023

Description of changes:
For CloudStack e2e tests we get the template names from the secret value. When we change eks-d version, template names get out of sync which makes e2e test failure until we change the template name in the secret.

To avoid this, this PR includes a logic to select the right template from the test runner itself and by not depending on the secret value. It looks for template name in below order,

  1. Look for explicit configuration through an env var: "T_CLOUDSTACK_TEMPLATE_{osFamily}_{eks-d version}" eg. T_CLOUDSTACK_TEMPLATE_REDHAT_KUBERNETES_1_23_EKS_22. This should be used for explicit configuration, mostly in local development for overrides.

  2. If not present, look for a template if the default templates: "{eks-d version}-{osFamily}" eg. kubernetes-1-23-eks-22-redhat. This is what should be used most of the time in CI, the explicit configuration is not present but the right template has already been imported to cloudstack.

  3. If the template doesn't exist, default to the value of the default template env vars: eg. "T_CLOUDSTACK_TEMPLATE_REDHAT_1_23". This is a catch all condition. Mostly for edge cases where the bundle has been updated with a new eks-d version, but the the new template hasn't been imported yet. It also preserves backwards compatibility.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@eks-distro-bot eks-distro-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 25, 2023
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Merging #5923 (b2057ba) into main (5b2012a) will increase coverage by 0.01%.
The diff coverage is 88.88%.

❗ Current head b2057ba differs from pull request most recent head 4f1c7c1. Consider uploading reports for the commit 4f1c7c1 to get more accurate results

@@            Coverage Diff             @@
##             main    #5923      +/-   ##
==========================================
+ Coverage   73.63%   73.64%   +0.01%     
==========================================
  Files         452      452              
  Lines       37992    38016      +24     
==========================================
+ Hits        27974    27998      +24     
+ Misses       8395     8394       -1     
- Partials     1623     1624       +1     
Impacted Files Coverage Δ
pkg/executables/cmk.go 83.51% <88.88%> (+0.27%) ⬆️

... and 1 file with indirect coverage changes

pkg/executables/cmk.go Outdated Show resolved Hide resolved
pkg/executables/cmk.go Outdated Show resolved Hide resolved
pkg/executables/cmk.go Outdated Show resolved Hide resolved
pkg/executables/cmk.go Outdated Show resolved Hide resolved
pkg/executables/cmk.go Outdated Show resolved Hide resolved
test/framework/cloudstack.go Outdated Show resolved Hide resolved
test/framework/cloudstack.go Outdated Show resolved Hide resolved
test/framework/executables.go Outdated Show resolved Hide resolved
Copy link
Member

@g-gaston g-gaston left a comment

Choose a reason for hiding this comment

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

Looking great!

pkg/executables/cmk.go Outdated Show resolved Hide resolved
test/framework/cloudstack.go Outdated Show resolved Hide resolved
test/framework/template.go Outdated Show resolved Hide resolved
test/framework/template.go Outdated Show resolved Hide resolved
test/framework/template.go Outdated Show resolved Hide resolved
test/framework/vsphere.go Outdated Show resolved Hide resolved
test/framework/vsphere.go Outdated Show resolved Hide resolved
test/framework/vsphere.go Outdated Show resolved Hide resolved
@panktishah26 panktishah26 force-pushed the cloudstack-template-e2e branch 2 times, most recently from 3c08a3d to 4f1c7c1 Compare May 31, 2023 18:05
@panktishah26
Copy link
Member Author

/test eks-anywhere-presubmit

test/framework/cloudstack.go Outdated Show resolved Hide resolved
test/framework/vsphere.go Outdated Show resolved Hide resolved
Refactor vSphere and CloudStack template name code to fit for all the providers
@g-gaston
Copy link
Member

/lgtm

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eks-distro-bot eks-distro-bot merged commit 728e450 into aws:main May 31, 2023
ddjjia pushed a commit to ddjjia/eks-anywhere that referenced this pull request Jun 8, 2023
Refactor vSphere and CloudStack template name code to fit for all the providers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants