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

Update helm chart to v0.26.0 #1038

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

damemi
Copy link
Contributor

@damemi damemi commented Jan 5, 2023

Fixes #1020

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 5, 2023
@damemi
Copy link
Contributor Author

damemi commented Jan 5, 2023

Ah

Failed to pull image "registry.k8s.io/descheduler/descheduler:v0.26.0": rpc error: code = NotFound desc = failed to pull and unpack image "registry.k8s.io/descheduler/descheduler:v0.26.0": failed to resolve reference "registry.k8s.io/descheduler/descheduler:v0.26.0": registry.k8s.io/descheduler/descheduler:v0.26.0: not found

So it looks like the updated helm chart version attempts to get installed. I'm thinking maybe this is the behavior now after #911? @knelasevero any ideas for that?

Some ways around this for releases:

  1. roll back/change introduce ct for local helm install test #911
  2. Build/tag the v0.26.0 image before merging the helm chart change.
  3. manually trigger the v0.26.0 image build (if possible? this could be the right way to do things going forward)

We use the helm chart change as the final "no-op" merge to trigger a build with the proper git tag (https://github.com/kubernetes-sigs/descheduler/blob/master/docs/release-guide.md#flowchart). But this step could be replaced with another non functional change such as readme updates. The important part is that functional code is merged before tagging the release in Git, followed by another GCB build to pull in the new git tag.

If there's no easy change to #911 we could make now, I'll just open a no-op docs change or something to trigger the build. Or manually trigger it

@a7i
Copy link
Contributor

a7i commented Jan 9, 2023

Option 2 doesn't seem so bad.

Another option would be to add a step to the GitHub Action to query the latest image release and use that as the AppVersion. In other words, always use the latest published image but it does seem add a bit of confusion as it would not use the AppVersion defined in the Chart.

Copy link
Member

@JaneLiuL JaneLiuL left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 9, 2023
@JaneLiuL
Copy link
Member

JaneLiuL commented Jan 9, 2023

prefer option 2 (Build/tag the v0.26.0 image before merging the helm chart change)

1 similar comment
@JaneLiuL
Copy link
Member

JaneLiuL commented Jan 9, 2023

prefer option 2 (Build/tag the v0.26.0 image before merging the helm chart change)

@knelasevero
Copy link
Contributor

In option 2 do you mean building locally and then pushing the image?

@knelasevero
Copy link
Contributor

If there's no easy change to #911 we could make now

Another option would be to add a step to the GitHub Action to query the latest image release and use that as the AppVersion. In other words, always use the latest published image but it does seem add a bit of confusion as it would not use the AppVersion defined in the Chart.

This is indeed an option, and not that hard to make I guess, but I agree that it is confusing. Probably having a doc change to be the no op step and then the helm chart step coming next is the best friction-less option going forward, and building it locally and pushing is ok to unblock this PR here?

@damemi
Copy link
Contributor Author

damemi commented Jan 9, 2023

We actually have some other changes under the v0.26.0 tag (like #995), so for this release that should be able to promote to the public image.

For the next release we'll need to think about this. It also might be good to add a "release-mode" blocker so that PRs don't merge automatically or accidentally while we're doing the release process.

@damemi
Copy link
Contributor Author

damemi commented Jan 13, 2023

/retest

@damemi
Copy link
Contributor Author

damemi commented Jan 13, 2023

Successful image tag, I'll work on updating the process for next release to address this.

@a7i
Copy link
Contributor

a7i commented Jan 13, 2023

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a7i

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 Jan 13, 2023
@k8s-ci-robot k8s-ci-robot merged commit 861c632 into kubernetes-sigs:master Jan 13, 2023
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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm chart version update to v0.26.0
5 participants