tests: make provider upgrade test more stable #5306
Conversation
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
|
Three completely green runs in a row: https://github.com/crossplane/crossplane/actions/runs/7714921864 |
phisco
changed the title
| // Note(turkenh): There is a tiny instant after the upgrade where | ||
| // the provider was still reporting Installed/Healthy but the | ||
| // new version was not yet installed. This causes flakes in the | ||
| // test as ".spec.forProvider.conditionAfter[0].conditionReason: field not declared in schema" error. |
There was a problem hiding this comment.
interesting - yes i did remove this check in https://github.com/crossplane/crossplane/pull/5261/files#diff-62c5ceab3727526d0cc8982c7ba3fee597fd534d51e14119f12b1ee63594a842 because I thought it was no longer needed given how the upgrade scenario has changed.
Previously we were migrating from old provider-nop:v0.1.x which had a different schema than what's in the more modern v0.2.x. I thought this flaky condition with field not declared in schema would no longer happen given that the schema is no longer changing during the ugprade test now from v0.2.0 --> v0.2.1.
Perhaps this check is important to synchronize the timing of the test, but maybe the comment is no longer valid about what particular transient error will manifest if the timing is wrong? 🤔
probably best to restore everything as you have here, but i'm not 100% certain of the specifics of the flaky interaction now 😂 . either way it does seem valid to verify here that the provider has status.currentIdentifier pointing to the new v0.2.1 version - that does make sense!
There was a problem hiding this comment.
Actually, in the end this alone wasn't enough to get all green tests, the second commit did the trick, I could try a few runs without this just to check if we can remove it.
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
phisco
force-pushed
the
restore-check-for-provider-upgrade
branch
from
c4057fc
to
9284dd7
Compare
|
replaced by #5307 |
Description of your changes
Fixes recent flakiness in E2Es (hopefully).
Restore a check removed, I think erroneously, in #5261.
I have:
make reviewableto ensure this PR is ready for review.Added or updated unit tests.Linked a PR or a docs tracking issue to document this change.backport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.