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

helmrepo: same revision different checksum scenario #691

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

darkowlzz
Copy link
Contributor

@darkowlzz darkowlzz commented Apr 27, 2022

This change prevents Reconciling and ArtifactOutdated conditions to be
set on HelmRepo when the checksum of a cached repo index changes.

Adds some tests to ensure that when the repo index is cached, the
revision and checksum of the returned artifact are the same as on the
existing object status.
Also adds checks for the returned artifact and chartRepo from
reconcileSource, to ensure that chartRepo is populated and the checksum
of a new potential artifact is always empty, as it's populated when the
artifact is written in the storage.

Adds tests for the change introduced in #685.

This change prevents Reconciling and ArtifactOutdated conditions to be
set on HelmRepo when the checksum of a cached repo index changes.

Adds some tests to ensure that when the repo index is cached, the
revision and checksum of the returned artifact are the same as on the
existing object status.
Also adds checks for the returned artifact and chartRepo from
reconcileSource, to ensure that chartRepo is populated and the checksum
of a new potential artifact is always empty, as it's populated when the
artifact is written in the storage.

Signed-off-by: Sunny <[email protected]>
@darkowlzz darkowlzz added the enhancement New feature or request label Apr 27, 2022
@@ -433,13 +434,15 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
chartRepo.Unload()

// Mark observations about the revision on the object.
if !obj.GetArtifact().HasRevision(newChartRepo.Checksum) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

newChartRepo.Checksum is always empty, resulting in this condition to be true always.

@hiddeco hiddeco added the area/helm Helm related issues and pull requests label Apr 27, 2022
@darkowlzz darkowlzz merged commit 745d6ee into main Apr 27, 2022
@darkowlzz darkowlzz deleted the cached-helmrepo-diff-checksum branch April 27, 2022 09:58
@pjbgf pjbgf added this to the GA milestone Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Helm related issues and pull requests enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants