Skip to content

Commit

Permalink
upgrade/adminack: guarantee one admin ack check post-upgrade
Browse files Browse the repository at this point in the history
While looking into OCPBUGS-5505 I discovered that some 4.10->4.11
upgrade job runs perform an Admin Ack check, while some do not. 4.11 has
a `ack-4.11-kube-1.25-api-removals-in-4.12` gate, so these upgrade jobs
sometimes test that `Upgradeable` goes `false` after the ugprade, and
sometimes they do not. This is only determined by the polling race
condition: the check is executed once per 10 minutes, and we cancel the
polling after upgrade is completed. This means that in some cases we are
lucky and manage to run one check before the cancel, and sometimes we
are not and only check while still on the base version.

Add a guaranteed single check execution after the upgrade, so that admin
ack is always checked at least once with the upgrade target version.
Doing checks after `done` is signalled has prior art in the alert test.
  • Loading branch information
petr-muller committed Jan 10, 2023
1 parent c081a2a commit bbd64c9
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
4 changes: 4 additions & 0 deletions test/e2e/upgrade/adminack/adminack.go
Expand Up @@ -47,6 +47,10 @@ func (t *UpgradeTest) Test(f *framework.Framework, done <-chan struct{}, upgrade
go func() {
<-done
cancel()
// Perform one guaranteed check after the upgrade is complete. We cancel the polled check
// above, so we never know whether the poll was lucky to run at least once since the version
// was bumped.
(&clusterversionoperator.AdminAckTest{Oc: t.oc, Config: t.config}).Test(context.Background())
}()

adminAckTest := &clusterversionoperator.AdminAckTest{Oc: t.oc, Config: t.config, Poll: 10 * time.Minute}
Expand Down
Expand Up @@ -40,7 +40,9 @@ var adminAckGateRegexp = regexp.MustCompile(adminAckGateFmt)
// and verifies that the Upgradeable condition no longer complains about the ack.
func (t *AdminAckTest) Test(ctx context.Context) {
if t.Poll == 0 {
t.test(ctx, nil)
if err := t.test(ctx, nil); err != nil {
framework.Fail(err.Error())
}
return
}

Expand Down

0 comments on commit bbd64c9

Please sign in to comment.