-
Notifications
You must be signed in to change notification settings - Fork 108
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
feat(ci): validate state version with cached state #4074
Conversation
In the Test workflow we were using a different approach than the one being used in the Full sync test. Also, in the Full sync test the variable was LOWER_NET_NAME, but NETWORK was being used in the disk name, with caps.
Disk states synced to canopy and synced to the chain tip should have different names to reference correctly on actual and coming tests the needed disk.
Before building and pushing new cached states, validate that our local version is not lower than the one available in GCP
Co-authored-by: Deirdre Connolly <[email protected]>
…to fix-disk-naming
This reverts commit cbbfaf4.
If we're changing constants.rs we might need to rebuild disks instead of using the cached ones available in GCP
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.
I'm not sure I understand exactly what is going on in these tests, happy to catch up for a video call if that would help.
@teor2345 I've integrated the considerations from your last comment. |
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.
Looks good, but I have a bunch of questions about how it works in detail.
Regenerate stateful disks failed with #3991:
I'll restart the job. |
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.
Thanks for your patience with this!
@Mergifyio update |
✅ Branch has been successfully updated |
This can't be merged until we fix #4155 |
Please include the clean fix #4206 when next updating this branch. |
This isn't in the critical path for lightwalletd work, we can merge it after we get the lightwalletd integration tests working. |
6b1ad4e
to
c47cccb
Compare
Obsoleted by #4271 |
Motivation
We don't check the state version when updating the cached state, so old PRs can add a new "latest" cached state with the old state version. This won't break CI, but it adds 2.5 hours to the mergify queue.
We can check cached state version vs the version in the current branch being tested. Doing this requires making the cached state version easier to find in CI. One idea is adding the state version as disk image metadata or in the name of the disk, and then doing some Rust test / grep check to compare the latest cached state disk version with the in-branch version, and only using the cached state if they are the same version.
If the cached state version is higher than the branch version, we can not run the test, failing the test until the branch is rebased onto
main
.Either way, the goal is that 'old' state versions (older than the version of state on #main) should not be created and pushed to gcloud, where they can be used by other tests.
Fixes #3881
Depends-On: #4073
Solution
Review
@dconnolly can review this
Follow Up Work
cached state version
is lower than thebranch version