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 fixed version to all addons #12416

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

olemarkus
Copy link
Member

This should ensure addon updates are triggered before rolling the cluster, as expected.
We also update addons before labeling masters to ensure that kops-controller is updated before it is scheduled.

Should hopefully fix #12249

/cc @zetaab

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/channels labels Sep 25, 2021
Copy link
Member

@hakman hakman left a comment

Choose a reason for hiding this comment

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

Can you set Version as KopsVersion? Should be good enough for what we want upgraded and won't be a strange fixed number.

@@ -105,6 +105,11 @@ func (b *BootstrapChannelBuilder) Build(c *fi.ModelBuilderContext) error {
}

for _, a := range addons.Spec.Addons {
// Older versions of channels that may be running on the upgrading cluster requires Version to be set
// We hardcode version to a high version to ensure an update is triggered on first run, and from then on
// only a hash change will trigger an addon update.
Copy link
Member

Choose a reason for hiding this comment

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

Consider mentioning "This can be removed when kops no longer supports k8s 1.22"

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately this follows kops versioning and we don't have any skew/upgrade jump policy. This will have to remain forever unless we change this

@olemarkus
Copy link
Member Author

Can you set Version as KopsVersion? Should be good enough for what we want upgraded and won't be a strange fixed number.

We need to set a version that is at least as high as the old addon versions. That is 4.0.0, I think. If we don't none of these addons will be updated.

@hakman
Copy link
Member

hakman commented Sep 26, 2021

Can you set Version as KopsVersion? Should be good enough for what we want upgraded and won't be a strange fixed number.

We need to set a version that is at least as high as the old addon versions. That is 4.0.0, I think. If we don't none of these addons will be updated.

My understanding is that they will be updated once new control-plane nodes are started, we just need the kOps ones (like kops-controller) to be updated before that.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2021
Ole Markus With added 2 commits September 30, 2021 15:25
This will make also older channels trigger addon updates
This will prevent an old kops-controller pod from being scheduled to the new master.
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 30, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rifelpet

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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 30, 2021
@k8s-ci-robot k8s-ci-robot merged commit 8449d60 into kubernetes:master Sep 30, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Sep 30, 2021
k8s-ci-robot added a commit that referenced this pull request Oct 2, 2021
…416-origin-release-1.22

Automated cherry pick of #12416: Add fixed version to all addons
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. area/channels cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/office-hours 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.

Unable to update cluster from 1.21 -> 1.22, kops controller crashlooping in AWS
4 participants