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

Add K8s chart support tests for GKE #71

Merged
merged 3 commits into from
Mar 5, 2024
Merged

Add K8s chart support tests for GKE #71

merged 3 commits into from
Mar 5, 2024

Conversation

valaparthvi
Copy link
Collaborator

@valaparthvi valaparthvi commented Feb 26, 2024

What does this PR do?

This PR adds tests for k8s chart support test for GKE.

Which issue(s) this PR fixes (optional, in fixes #(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Fixes part of #49

Checklist:

  • Squashed commits into logical changes
  • Documentation
  • GitHub Actions (if applicable)

Special notes for your reviewer:

Env Vars to use:

CATTLE_TEST_CONFIG=github.com/rancher/hosted-providers-e2e/cattle-config-import.yaml
GCP_CREDENTIALS=<gcp_creds>
GKE_PROJECT_ID=<gcp-project>
GKE_ZONE=us-central1-c
KUBECONFIG=/etc/k3s/k3s.yaml
K8S_UPGRADE_MINOR_VERSION=1.28
PROVIDER=gke
RANCHER_HOSTNAME=hostname
RANCHER_PASSWORD=adminadmin
RANCHER_UPGRADE_VERSION=devel/2.8
RANCHER_VERSION=2.8.2

@valaparthvi valaparthvi force-pushed the upgrade-e2e branch 4 times, most recently from 28b7e96 to ad57a9a Compare February 28, 2024 07:07
@valaparthvi valaparthvi changed the title WIP: Add K8s chart support tests for GKE Add K8s chart support tests for GKE Feb 28, 2024
@valaparthvi valaparthvi force-pushed the upgrade-e2e branch 4 times, most recently from e6430f3 to dcd5704 Compare February 28, 2024 10:33
@valaparthvi valaparthvi requested a review from cpinjani February 28, 2024 10:41
@valaparthvi valaparthvi force-pushed the upgrade-e2e branch 2 times, most recently from 03677bd to 8e30148 Compare February 28, 2024 11:14
filename = strings.TrimSuffix(filename, ".go") // `.` is not allowed
filename = strings.ReplaceAll(filename, "/", "-") // `/` is not allowed
filename = strings.ToLower(filename) // string must be in lowercase
fileSplit := strings.Split(specReport.FileName(), "/") // abstract the filename
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change was made to avoid char length errors. Max permitted limit is 63.
The label will now be something like: line51_k8s_chart_support_provisioning_test

description: Runner template to use
default: hosted-prov-e2e-ci-runner-spot-n2-highmem-16-gl-template-v1
type: string
k8s_upgrade_minor_version:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
k8s_upgrade_minor_version:
k8s_version_to_test:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously it was k8s_version_to_test but that was slightly confusing since it indicates that you might have to provide a complete version, but that is not true. We only need to pass the minor version, for e.g. 1.28, the complete version will be computed programmatically.

Copy link
Collaborator

Choose a reason for hiding this comment

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

k8s_minor_version_to_test ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have removed the GH action for now. I will add it once AKS and EKS is implemented.

@valaparthvi valaparthvi force-pushed the upgrade-e2e branch 2 times, most recently from 9d10a00 to ee2c55e Compare February 29, 2024 15:20
@valaparthvi valaparthvi requested a review from cpinjani March 4, 2024 03:49
@@ -80,6 +80,13 @@ e2e-support-matrix-importing-tests: deps ## Run the 'SupportMatrixImporting' tes
e2e-support-matrix-provisioning-tests: deps ## Run the 'SupportMatrixProvisioning' test suite for a given ${PROVIDER}
ginkgo ${STANDARD_TEST_OPTIONS} --focus "SupportMatrixProvisioning" ./hosted/${PROVIDER}/support_matrix/

e2e-k8s-chart-support-importing-tests: deps ## Run the 'K8sChartSupportImport' test suite for a given ${PROVIDER}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Running "install-k3s install-helm install-cert-manager" would also be required in CI, no ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but I will deal with this later when I open PR for GH action.

})

var _ = AfterEach(func() {
// The test must restore the env to its original state, so we install rancher back to its original version and uninstall the operator charts
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the restore step required ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. To restore rancher back to it's original version so that we can start a new test, and also to uninstall operator charts, otherwise the tests running in succession will not work.

It(fmt.Sprintf("should successfully test k8s %s chart support on rancher %s", k8sUpgradedMinorVersion, rancherUpgradedVersion), func() {
commonChartSupportUpgrade(&ctx, cluster, clusterName, rancherUpgradedVersion, helpers.RancherHostname, k8sUpgradedMinorVersion)
})

Copy link
Collaborator

Choose a reason for hiding this comment

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

It(fmt.Sprintf("should successfully test k8s %s chart support provisioning on upgraded rancher %s", k8sUpgradedMinorVersion, rancherUpgradedVersion)

This TC will also be required to automated, can you add TODO ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@valaparthvi valaparthvi requested a review from cpinjani March 5, 2024 06:16
@valaparthvi valaparthvi merged commit 51f7ec4 into main Mar 5, 2024
2 checks passed
@valaparthvi valaparthvi deleted the upgrade-e2e branch March 5, 2024 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants