Skip to content

🌱 Enable k8s upgrade in self hosted test #1732

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

Merged

Conversation

lentzi90
Copy link
Contributor

What this PR does / why we need it:

This enables the k8s upgrade that is part of the self hosted test. It was previously disabled since the dynamic installation of kubernetes components on the nodes made it hard to change the version in the middle of a test.

Also updated the README tables for what versions are tested.

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

Special notes for your reviewer:

  1. Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squashed commits
  • if necessary:
    • includes documentation
    • adds unit tests

/hold

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Oct 23, 2023
@lentzi90 lentzi90 marked this pull request as ready for review October 23, 2023 15:55
@netlify
Copy link

netlify bot commented Oct 23, 2023

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
🔨 Latest commit 4d500ed
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/65375b117cd9ab00085f7db2
😎 Deploy Preview https://deploy-preview-1732--kubernetes-sigs-cluster-api-openstack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 23, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lentzi90

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

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Oct 23, 2023
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 23, 2023
@lentzi90
Copy link
Contributor Author

/test pull-cluster-api-provider-openstack-e2e-full-test


Test status:

- `✓` tested
- `★` currently testing
- `✓` previously tested
Copy link
Contributor

Choose a reason for hiding this comment

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

this is very great update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All credit goes to @wwentland 😊

@lentzi90 lentzi90 changed the title 🌱 Enable k8s upgrade in slef hosted test 🌱 Enable k8s upgrade in self hosted test Oct 24, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit silly, but allow me to explain.
We need a separate OpenStackMachineTemplate with a different k8s version in order to do the upgrade. The test spec itself is making the switch from the initial template to the "upgrade to" template, that is specified in the e2e config.
Our normal template is already using k8s v1.28.2 so here I add a template witch v1.27.2 to get a different initial version.

The silly part is that we cannot upgrade to the "normal" template because it gets a dynamic name when applied in the test. All clusters are created with a random suffix and this propagates also to the template, so it is impossible to specify this template in the e2e config. We could change the naming scheme of the template to work around this. Let me know if you think this would be better. What I did instead was to just add another statically named template, so we end up with 2 extra templates for this test.

metadata:
name: upgrade-to-control-plane
labels:
clusterctl.cluster.x-k8s.io/move: ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This label is important to get the (not yet in use) template moved to the self hosted cluster. Otherwise it would be left behind since it is not part of the owner chain at that point.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, I think it's something I tried to add into CAPO before, IIRC it's moving stuffs from boostrap to deployed cluster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. To be clear, it is only needed on this template because it is not part of the cluster owner chain. All resources that are part of the cluster are moved automatically. For example, the OpenStackMachineTemplates that are used when initializing the workload cluster are automatically moved.

This enables the k8s upgrade that is part of the self hosted test. It
was previously disabled since the dynamic installation of kubernetes
components on the nodes made it hard to change the version in the middle
of a test.

Also updated the README tables for what versions are tested.
@lentzi90 lentzi90 force-pushed the lentzi90/selfhosted-upgrade branch from 8638e3e to 4d500ed Compare October 24, 2023 05:50
@jichenjc
Copy link
Contributor

/lgtm

leave unhold to someone who might want to take another look

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 24, 2023
@lentzi90
Copy link
Contributor Author

/test pull-cluster-api-provider-openstack-e2e-full-test

1 similar comment
@lentzi90
Copy link
Contributor Author

/test pull-cluster-api-provider-openstack-e2e-full-test

@lentzi90
Copy link
Contributor Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 25, 2023
@k8s-ci-robot k8s-ci-robot merged commit 9369184 into kubernetes-sigs:main Oct 25, 2023
@lentzi90 lentzi90 deleted the lentzi90/selfhosted-upgrade branch October 25, 2023 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. 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