Skip to content

Update lifecycle release selector for s390x #2232

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update lifecycle release selector for s390x #2232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion automation/check-patch.e2e-lifecycle-k8s.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ main() {
export E2E_TEST_TIMEOUT=4h
else
# Don't run all upgrade tests in regular PRs, stick to those released under HCO
export RELEASES_SELECTOR="{0.89.3,0.91.1,0.93.0,0.95.0,99.0.0}"
case "$(go env GOARCH)" in
Copy link
Collaborator

@oshoval oshoval Apr 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks

instead the change please consider exporting the RELEASES_SELECTOR you want in the s390x lane
i do understand that you will need to update it from time to time, but it can be nicer
wdyt ?

alternative is to auto detect if a release has s390x and skip it if not, and then no need additional changes anymore
It will be best if we can find something minimal that does the job please

cc @RamLavi wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good suggestion!

I think we can approach it like this:

  1. Add version 0.98.2 to the list in check-patch.e2e-lifecycle-k8s.sh.

if we don't want to do that then we can skip lifecycle for s390x as s390x supported version 0.98.2 is not release under HCO yet

if we are okay with 1 then
For the s390x Prow lane, we can use RELEASES_DESELECTOR env instead of RELEASES_SELECTOR:

cluster-network-addons-operator/test/releases/releases.go

Line 62 in ec5c996

if releasesDeselectorRaw, found := os.LookupEnv("RELEASES_DESELECTOR"); found {

However, one issue I noticed is that 0.98.2 is not currently part of the populated release list. not sure why ???
You can confirm that from the output shown here: https://sharepad.io/p/5L7nfm9

It seems that the version isn't added in the releases initialization:

cluster-network-addons-operator/test/releases/releases.go

Line 46 in ec5c996

Expect(validateReleasesSelector(releases, releasesSelectorRaw)).NotTo(HaveOccurred())

So before anything else, we’ll need to ensure 0.98.2 is included in the releases definition.

Copy link
Collaborator

@oshoval oshoval Apr 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for looking on it deeply

0.98.2 is just a release on main, so HCO can consume it, since it doesnt belong to a stable branch we should not add it to RELEASES_SELECTOR

Please just set RELEASES_SELECTOR as you wish in the job, but make sure please that someone maintains it

Possible idea for the future - to think on a way that auto select the 3 latest stable branches that are formal, or at least to have one place where we add it manually in the beginning and from there, helper scripts are populating it to the required place, i.e 390x lane

Copy link
Contributor

Choose a reason for hiding this comment

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

just setting export RELEASES_SELECTOR=0.98.2 is not working, it is failing at below check, as explained earlier

cluster-network-addons-operator/test/releases/releases.go

Line 46 in ec5c996

Expect(validateReleasesSelector(releases, releasesSelectorRaw)).NotTo(HaveOccurred())

Setting RELEASES_SELECTOR alone won't work until we add 0.98.2 to the populated releases list.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we can't add it
we add only latest patch on stables and this isn't stable

can you please try to think about other way / debug it deeper ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@RamLavi wdyt ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should use 0.98.2, as @oshoval said.
The first release that will have a stable branch AND support the new arch - IIUC is the next one (0.99.0) - which is currently pending merge (once quay outage is over it sould be merged quickly).
I suggest to use that version instead

Copy link
Collaborator

Choose a reason for hiding this comment

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

0.99 is out (well on main still)
once there is a stable branch
#2284
and then we can try according suggested in this thread ?
(strive to keep it as simple as possible please, and as low maintenance we need)

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, noted.

s390x)
export RELEASES_SELECTOR="{0.98.2,99.0.0}"
;;
*)
export RELEASES_SELECTOR="{0.89.3,0.91.1,0.93.0,0.95.0,99.0.0}"
;;
esac
fi

make cluster-down
Expand Down
17 changes: 16 additions & 1 deletion test/e2e/lifecycle/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package test

import (
"fmt"
"runtime"
"time"

. "github.com/onsi/ginkgo/v2"
Expand All @@ -13,6 +14,9 @@ import (

const podsDeploymentTimeout = 20 * time.Minute

// CNAO supports multiarch from this release
const multiArchStartRelease = "0.98.2"

var _ = Context("Cluster Network Addons Operator", func() {
testUpgrade := func(oldRelease, newRelease Release) {
Context(fmt.Sprintf("when operator in version %s is installed and supported spec configured", oldRelease.Version), func() {
Expand Down Expand Up @@ -84,7 +88,18 @@ var _ = Context("Cluster Network Addons Operator", func() {

// Run tests upgrading from each released version to the latest/main
releases := Releases()
for _, oldRelease := range releases[:len(releases)-1] {

start := 0
if runtime.GOARCH == "s390x" {
for index, release := range releases {
if release.Version == multiArchStartRelease {
start = index
break
}
}
}

for _, oldRelease := range releases[start : len(releases)-1] {
testUpgrade(oldRelease, LatestRelease())
}
})