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

TEST #30763

Closed
Closed

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Feb 14, 2024

tests for #30742

Figuring out the right "previous patch release version number" to
downgrade to in print-downgrade-version.sh turns out to be more complex
than expected [0][1][2][3].

This commit is an attempt to 1) fix issues with the current script and
2) overall make the script clearer, so we can avoid repeating these
mistakes.

As for the fixes, there are two things that are not correct with the
current version. First, we're trying to validate the existence of the
tag to downgrade to, in case the script runs on top of a release
preparation commit for which file VERSION has been updated to a value
that does not yet contains a corresponding tag. This part of the script
is actually OK, but not the way we call it in the IPsec workflow: we use
"fetch-tags: true" but "fetch-depth: 0" (the default), and the two are
not compatible, a shallow clone results in no tags being fetched.

To address this, we change our strategy: we fetch two commits in the CI
workflow, the latest commit and the rest of the history compressed into
a first, giant commit. This makes it possible to check whether the
VERSION file has been modified in the latest commit, in which case we
consider this is a release preparation commit and we decrement the
patch version number before returning it. Just make sure the VERSION
update comes in the last commit of your PRs.

The second issue is that we would return no value from the script if the
patch release is zero. This is to avoid any attempt to find a previous
patch release when working on a development branch. However, this logics
is incorrect (it comes from a previous version of the script where we
would always decrement the patch number). After the first release of a
new minor version, it's fine to have a patch number at 0. What we should
check instead is whether the version ends with "-dev".

This commit brings additional changes for clarity: more comments, and a
better separation between the "get latest patch release" and "get
previous stable branch" cases, moving the relevant code to independent
functions. We also edit the IPsec workflow to add some logs about the
version retrieved. The logs should also display the script's error
messages, if any, that are printed to stderr.

Sample output from the script:

    VERSION         Previous        Previous        Previous patch
                    stable          patch release   release (on top of
                    branch          (on a random    a commit updating
                                    commit)         VERSION)

    1.14.3          v1.13           v1.14.3         v1.14.2
    1.14.1          v1.13           v1.14.1         v1.14.0
    1.14.0          v1.13           v1.14.0         <error>
    1.14.1-dev      v1.13           <error>         <error>
    1.15.0-dev      v1.14           <error>         <error>
    1.13.90         v1.12           v1.13.90        v1.13.89
    2.0.1           <error>         v2.0.1          v2.0.0

[0] 56dfec2 ("contrib/scripts: Support patch releases in print-downgrade-version.sh")
[1] 4d7902f ("contrib/scripts: Remove special handling for patch release number 90")
[2] 5581963 ("ci/ipsec: Fix version retrieval for downgrades to closest patch release")
[3] 3803f53 ("ci/ipsec: Fix downgrade version for release preparation commits")

Fixes: 3803f53 ("ci/ipsec: Fix downgrade version for release preparation commits")
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet added dont-merge/preview-only Only for preview or testing, don't merge it. release-note/ci This PR makes changes to the CI. labels Feb 14, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.15 This PR represents a backport for Cilium 1.15.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. labels Feb 14, 2024
@qmonnet
Copy link
Member Author

qmonnet commented Feb 14, 2024

/ci-ipsec-upgrade

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet
Copy link
Member Author

qmonnet commented Feb 14, 2024

/ci-ipsec-upgrade

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet force-pushed the pr/qmonnet/ipsec/patch-release-downgrade-fix-more-1.15 branch from c9599e3 to a37e7ba Compare February 14, 2024 12:30
@qmonnet
Copy link
Member Author

qmonnet commented Feb 14, 2024

/ci-ipsec-upgrade

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet
Copy link
Member Author

qmonnet commented Feb 14, 2024

/ci-ipsec-upgrade

@qmonnet qmonnet closed this Feb 14, 2024
@qmonnet qmonnet deleted the pr/qmonnet/ipsec/patch-release-downgrade-fix-more-1.15 branch February 14, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This PR represents a backport for Cilium 1.15.x of a PR that was merged to main. dont-merge/preview-only Only for preview or testing, don't merge it. kind/backports This PR provides functionality previously merged into master. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant