Skip to content

Fix latest tag retrieval for krel changelog #1135

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
merged 1 commit into from
Feb 26, 2020
Merged

Fix latest tag retrieval for krel changelog #1135

merged 1 commit into from
Feb 26, 2020

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Feb 26, 2020

What type of PR is this?

/kind bug

What this PR does / why we need it:

The issue fixed with this commit is that we cannot say if the target tag
to be built already exists in the local repository, which made it
impossible to krel to behave in an expected way. If a tag during
changelog generation already locally exists (but not remotely) seems to
differ in anago, which is the reason we now use a different approach:

  1. We correctly chose the release branch (or master) HEAD from the
    remote as end SHA because we cannot know which branch is currently
    checked out.
  2. We use a new helper function git.LatestGitHubTagsPerBranch which
    fetches the latest GitHub tags for each release branch and the
    master. This follows the logic that alpha releases are only on the
    master branch, whereas all other releases except the final tag are
    available only on release-x.y branches. The latest tag for the
    release branch (or master) will be our start commit for the release
    notes generation.

Which issue(s) this PR fixes:

Refers to kubernetes/sig-release#999

Special notes for your reviewer:

The original relnotes script used this implementation to achieve the same goal:

release/lib/gitlib.sh

Lines 163 to 186 in cadfbd5

gitlib::last_releases () {
local release
local branch_name
local latest_branch
declare -Ag LAST_RELEASE
logecho -n "Setting last releases by branch: "
for release in $($GHCURL $K8S_GITHUB_API/releases|\
jq -r '.[] | select(.draft==false) | .tag_name'); do
# Alpha releases only on master branch
if [[ $release =~ -alpha ]]; then
LAST_RELEASE[master]=${LAST_RELEASE[master]:-$release}
elif [[ $release =~ v([0-9]+\.[0-9]+)\.([0-9]+(-.+)?) ]]; then
# Latest vx.x.0 release goes on both master and release branch.
if [[ ${BASH_REMATCH[2]} == "0" ]]; then
LAST_RELEASE[master]=${LAST_RELEASE[master]:-$release}
fi
branch_name=release-${BASH_REMATCH[1]}
LAST_RELEASE[$branch_name]=${LAST_RELEASE[$branch_name]:-$release}
fi
done
logecho -r "$OK"
}

The general implementation has the drawback that we cannot generate changelogs for old releases any more, because we now rely on the latest tag from GitHub.

Does this PR introduce a user-facing change?:

- Updated `krel changelog` to correctly choose the start tag for a release

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 26, 2020
@k8s-ci-robot k8s-ci-robot added area/release-eng Issues or PRs related to the Release Engineering subproject sig/release Categorizes an issue or PR as relevant to SIG Release. labels Feb 26, 2020
The issue fixed with this commit is that we cannot say if the target tag
to be built already exists in the local repository, which made it
impossible to krel to behave in an expected way. If a tag during
changelog generation already locally exists (but not remotely) seems to
differ in `anago`, which is the reason we now use a different approach:

1. We correctly chose the release branch (or master) HEAD from the
   remote as end SHA because we cannot know which branch is currently
   checked out.
2. We use a new helper function `git.LatestGitHubTagsPerBranch` which
   fetches the latest GitHub tags for each release branch and the
   master. This follows the logic that `alpha` releases are only on the
   master branch, whereas all other releases except the final tag are
   available only on `release-x.y` branches. The latest tag for the
   release branch (or master) will be our start commit for the release
   notes generation.

Signed-off-by: Sascha Grunert <[email protected]>
Copy link
Member

@cpanato cpanato 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 Feb 26, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, saschagrunert

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 merged commit c99628f into kubernetes:master Feb 26, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Feb 26, 2020
@saschagrunert saschagrunert deleted the fix-changelog-latest-tag branch February 26, 2020 09:35
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/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/release Categorizes an issue or PR as relevant to SIG Release. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants