Skip to content
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!: protect against stale queue items #292

Merged
merged 5 commits into from
May 23, 2023

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented May 18, 2023

Part of go-vela/community#811
Blocks: go-vela/worker#478

This adds an ItemVersion field to Item with an ItemVersion constant that should be bumped whenever we make significant changes to Item or the pipeline.Build. This way, the worker can detect any stale builds that were queued before a Vela server upgrade or downgrade, and fail the build.

In the future, we could do something even nicer and ask the server to recompile and requeue the build.

This should allow the worker to invalidate stale queued builds.
A stale build is a build that was queued before a Vela upgrade or downgrade
if the two Vela versions do not share the same queue ItemVersion.
Make it clear that this is not the Vela version or the pipeline yaml version.
@cognifloyd cognifloyd added the feature Indicates a new feature label May 18, 2023
@cognifloyd cognifloyd self-assigned this May 18, 2023
@cognifloyd cognifloyd requested a review from a team as a code owner May 18, 2023 02:23
@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #292 (c60e160) into main (0da8c8b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #292   +/-   ##
=======================================
  Coverage   97.00%   97.00%           
=======================================
  Files          59       59           
  Lines        6545     6546    +1     
=======================================
+ Hits         6349     6350    +1     
  Misses        145      145           
  Partials       51       51           
Impacted Files Coverage Δ
item.go 100.00% <100.00%> (ø)

Copy link
Contributor

@plyr4 plyr4 left a comment

Choose a reason for hiding this comment

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

LGTM, that will make future updates a lot smoother.
should we mark this as breaking?

@cognifloyd cognifloyd changed the title feat: protect against stale queue items feat!: protect against stale queue items May 19, 2023
@cognifloyd
Copy link
Member Author

It's difficult to say if it is breaking or not. I added the !. 🤷

@cognifloyd cognifloyd requested review from wass3rw3rk, ecrupper and a team May 19, 2023 16:09
Comment on lines +24 to +25
// The 0-value is the implicit ItemVersion for queued Items that pre-date adding the field.
ItemVersion uint64 `json:"item_version"`
Copy link
Member Author

Choose a reason for hiding this comment

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

NB: This is downgrade safe because if a json object has an unknown field, it is ignored.

@ecrupper ecrupper merged commit 35a0d5f into main May 23, 2023
@ecrupper ecrupper deleted the feature/stale-data-protection branch May 23, 2023 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Indicates a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants