Skip to content

Conversation

stevehipwell
Copy link
Contributor

What this PR does / why we need it:
This PR adds support for releasing the Helm chart from the master branch while keeping support for releasing from a release branch if required (e.g. the chart has been modified to target the next release). This change will reduce the overhead for a chart release and allow us to be more responsive to chart changes.

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 #

CC @dgrisonnet

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 8, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: stevehipwell
Once this PR has been reviewed and has the lgtm label, please assign dgrisonnet for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@github-project-automation github-project-automation bot moved this to Needs Triage in SIG Instrumentation Sep 8, 2025
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 8, 2025
@stevehipwell stevehipwell force-pushed the refactor-helm-chart-release branch from c12fcb4 to 662c2c3 Compare September 8, 2025 10:15
@stevehipwell
Copy link
Contributor Author

/retest

@stevehipwell
Copy link
Contributor Author

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 12, 2025
@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 25, 2025
@stevehipwell stevehipwell force-pushed the refactor-helm-chart-release branch from 662c2c3 to 581cb0f Compare September 25, 2025 17:49
@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 25, 2025
@stevehipwell
Copy link
Contributor Author

@dgrisonnet could you please take a look at this?

RELEASE.md Outdated
A new chart should be released whenever a new version of Metrics Server is
released, but releases can also be done on an as-needed basis. Most chart
releases for the latest Metrics Server version can take place on the `master`
branch; but if release is a backport or the chart on `master` is no longer
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
branch; but if release is a backport or the chart on `master` is no longer
branch. However, if a release is a backport or the chart on `master` is no longer

RELEASE.md Outdated
released, but releases can also be done on an as-needed basis. Most chart
releases for the latest Metrics Server version can take place on the `master`
branch; but if release is a backport or the chart on `master` is no longer
compatible then the PR should target the release branch directly.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
compatible then the PR should target the release branch directly.
compatible, the PR should target the release branch.

if [[ "${branch_name}" == "master" ]]; then
release=true
else
branch_version_prefix="$(grep -Eo '[0-9]+\.[0-9]+$' <<<"${branch_name}")"
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little confused on why this is called branch_version_prefix when it looks like it is the branch_version. The prefix to me would be release-

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the version prefix (x.y) and the branch suffix, let me make it a bit clearer.

@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 29, 2025
@stevehipwell stevehipwell force-pushed the refactor-helm-chart-release branch from 581cb0f to 376addc Compare September 29, 2025 17:56
@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 29, 2025
@stevehipwell stevehipwell force-pushed the refactor-helm-chart-release branch from 376addc to 3ce1bd6 Compare September 29, 2025 18:05
@stevehipwell
Copy link
Contributor Author

Thanks for the review @TerryHowe, I've applied your suggestions.

Copy link
Member

@TerryHowe TerryHowe 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 Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Status: Needs Triage
Development

Successfully merging this pull request may close these issues.

3 participants