-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[v1.13] Test upgrade/downgrade to patch release for IPsec #29003
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
Conversation
bd42c3d
to
1e28ee1
Compare
1e28ee1
to
b0e2969
Compare
/test-backport-1.13 |
/ci-ipsec-upgrade |
Tests fail when upgrading from either minor or patch version on bpf-next, but I understand this has been the case before this PR. Other tests passed. I need to rebase this one now that 751b7c4 was merged. |
b0e2969
to
f912898
Compare
/test-backport-1.13 Job 'Cilium-PR-K8s-1.23-kernel-4.19' failed: Click to show.Test Name
Failure Output
Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.23-kernel-4.19/468/ If it is a flake and a GitHub issue doesn't already exist to track it, comment Then please upload the Jenkins artifacts to that issue. |
/ci-ipsec-upgrade |
/test-1.24-4.19 |
f912898
to
b3cbe89
Compare
Incremental diffdiff --git a/.github/workflows/tests-ipsec-upgrade.yaml b/.github/workflows/tests-ipsec-upgrade.yaml
index f4bd133140ea..3132c58ae04b 100644
--- a/.github/workflows/tests-ipsec-upgrade.yaml
+++ b/.github/workflows/tests-ipsec-upgrade.yaml
@@ -272,7 +272,6 @@ jobs:
mkdir -p cilium-junits
- name: Wait for images to be available
- if: ${{ matrix.mode != 'patch' }}
timeout-minutes: 30
shell: bash
run: | /test-backport-1.13 |
/ci-ipsec-upgrade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My changes LGTM, thanks.
b3cbe89
to
afb94d4
Compare
/test-1.26-net-next |
Failures on bpf-next are expected to be fixed with #29227. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
net-next should now be fixed, so probably worth rebasing to ensure CI passes. Might also be worth updating the commits here with references to the now-merged upstream commits.
[ upstream commit d51a932 ] - Add a script to get the Cilium version to downgrade to instead of hardcoding it in the workflow file. The script uses the top-level VERSION file to infer the previous version. - Check out the git branch to get the Helm chart instead of doing a wget to download the source archive. Signed-off-by: Michi Mutsuzaki <michi@isovalent.com> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
afb94d4
to
a413309
Compare
/test-backport-1.13 |
[ upstream commit 56dfec2 ] Script contrib/scripts/print-downgrade-version.sh is used to derive the name of the previous stable branch, based on the current version number found in the repository. This is useful for testing upgrades and dowgrades in CI, between the current branch and the previous stable branch. For some tests we need to perform similar checks between the current tip of a branch and the latest patch release on the branch. For example, when working on branch 1.14, we want to downgrade to the latest patch release, 1.14.3 at this time, then upgrade back to the tip of 1.14. On the main branch, this is not relevant, because we don't usually have patch releases on that branch. The current commit updates print-downgrade-version.sh to add support for patch releases. When a user pass "patch" as first argument to the patch, then instead of decrementing the minor version by one, the script decrements the patch release number by one, and prints the results. When the patch release number is 0 (new minor release) or 90 (release preparation), the script returns an error, because it is non-trivial to find the preceding patch release number in such cases (at least without Git and the Git history). From the workflow's perspective (for supporting upgrades from patch releases in a follow-up commit), for new minor releases, update/downgrade is already covered in this case by working with the previous stable branch; and for 90, we just don't have an easy way to retrieve the previous number. We make the script print errors on stderr, in order to make it easier to compare the string returned on stdout (empty in case of error). Some examples of numbers from VERSION and the corresponding values returned: VERSION Previous minor Previous patch release 1.14.3 v1.13 v1.14.2 1.14.1 v1.13 v1.14.0 1.14.0 v1.13 <error> 1.14.1-dev v1.13 v1.14.0 1.15.0-dev v1.14 <error> 1.13.90 v1.12 <error> In order to test the script easily, this commit also allows setting $VERSION from the command line, defaulting to the content of file VERSION if no value is provided. Let's also add the errexit and nounset options to the script. Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 31afd02 ] Previously cilium-config action always generates helm-set flags for image settings, but in some cases we can just rely on Chart.yaml since we always set chart-dir. This helps when: 1. We want to install release version instead of ci version. Currently cilium-config action sets `cilium-ci` unconditionally. 2. We have complicated image tag requirements such as `v1.14.1-beta.1`. [ Backport note: Addressed minor conflicts. ] Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 502bbc7 ] [1] changed the upgrade path from "v1.14 (branch tip) -> main -> v1.14 (branch tip)" to "v1.14.x (last release) -> main -> v1.14.x (last release)". The downside of the former path is that we catch any upgrade/downgrade regressions only after a release. This commit brings back the previous path. [1]: 31afd02 Signed-off-by: Martynas Pumputis <m@lambda.lt> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit ced884f ] The downgrade is still affected [1]. [1]: #26739 (comment) Signed-off-by: Martynas Pumputis <m@lambda.lt> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit ed59edc ] Currently, we test upgrades and downgrades for IPsec against the previous stable branch, for example: - On main branch (v1.15-dev): v1.14 (branch tip) -> main (PR HEAD) -> v1.14 (branch tip) - On older stable branches: v1.13 (branch tip) -> v1.14 (PR HEAD) -> v1.13 (branch tip) For stable branches, this commit adds support for testing upgrades and downgrades against the latest patch release as well, for example: - On v1.14: v1.14.4 (tag) -> v1.14 (PR HEAD) -> v1.14.4 (tag) The workflow currently fails on the main branch (this case is covered by the upgrade/downgrade test to the previous stable branch already). This is addressed by skipping most of the steps on main branch, in a follow-up commit. Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit c9dedb4 ] Skip upgrade/downgrade test to patch release when we fail to retrieve the number for the previous patch release. This happens mostly for the main branch (where testing upgrades/downgrades is covered by the tests to the previous stable (minor) release already). This may also happen on top of release preparation commits, where we set the patch number to 90, and where it is non-trivial to retrieve the previous patch release number. This case doesn't matter much, because commits for preparing releases are Not Expected To Break IPsec (TM). Signed-off-by: Quentin Monnet <quentin@isovalent.com>
a413309
to
dd6090a
Compare
/test-backport-1.13 |
/test-1.18-4.19 |
/test-1.23-4.19 |
Related to
Once this PR is merged, you can update the PR labels via:
or with