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

Make the Image reconciliation generate deterministic build names #847

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

matthewmcnew
Copy link
Collaborator

kpack appears to periodically reconcile the Image before its internal Build lister is updated with the previous reconcile's Build. This leads to subsequent reconciles creating duplicate builds.

By using deterministic build names the reconcile of an image will result in an error attempting to create a new build if a image reconcile starts before the informer has the previously created build.

@matthewmcnew matthewmcnew linked an issue Oct 4, 2021 that may be closed by this pull request
@matthewmcnew
Copy link
Collaborator Author

The risk here would be that the build name for the next build has already been created manually outside of the image reconciler loop the image reconciler would never be able to successfully reconcile.

kpack appears to periodically reconcile the Image before its internal Build lister is updated with the previous reconcile's Build. This leads to subsequent reconciles creating duplicate builds.

By using deterministic build names the reconcile of an image will result in an error attempting to create a new build if a image reconcile starts before the informer has the previously created build.
@codecov-commenter
Copy link

Codecov Report

Merging #847 (e956740) into main (3e4bdcd) will decrease coverage by 1.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #847      +/-   ##
==========================================
- Coverage   68.69%   67.63%   -1.07%     
==========================================
  Files         114      114              
  Lines        5054     5014      -40     
==========================================
- Hits         3472     3391      -81     
- Misses       1210     1260      +50     
+ Partials      372      363       -9     
Impacted Files Coverage Δ
pkg/apis/build/v1alpha1/image_builds.go 0.00% <ø> (-58.98%) ⬇️
pkg/apis/build/v1alpha2/image_builds.go 69.13% <100.00%> (ø)
pkg/apis/build/v1alpha1/image_types.go 0.00% <0.00%> (-25.00%) ⬇️
pkg/apis/build/v1alpha1/build.go 5.68% <0.00%> (-9.10%) ⬇️
pkg/apis/build/v1alpha1/source_resolver.go 0.00% <0.00%> (-6.90%) ⬇️
pkg/apis/build/v1alpha1/cluster_stack_types.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e4bdcd...e956740. Read the comment docs.

@matthewmcnew matthewmcnew merged commit a14c908 into main Oct 4, 2021
@matthewmcnew matthewmcnew deleted the deterministic-build-names branch October 4, 2021 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Two builds created for a single image
4 participants