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

Watched cross-ns image repos trigger reconcile #196

Merged
merged 2 commits into from
Nov 11, 2021

Conversation

relu
Copy link
Member

@relu relu commented Nov 11, 2021

Cross-namespace ImageRepository resources should trigger reconciles for
ImagePolicies that refer to them. Previously, this was only done for
resources in the same namespace.

Fixes #195

@relu relu force-pushed the fix-cross-ns-repo-image-reconcile branch from f0e5fd3 to 1460730 Compare November 11, 2021 14:15
Cross-namespace ImageRepository resources should trigger reconciles for
ImagePolicies that refer to them. Previously, this was only done for
resources in the same namespace.

Fixes #195

Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
@relu relu force-pushed the fix-cross-ns-repo-image-reconcile branch from 1460730 to 0dd4f70 Compare November 11, 2021 14:20
@@ -687,6 +688,19 @@ var _ = Describe("ImagePolicy controller", func() {
}, timeout, interval).Should(BeTrue())
Expect(pol.Status.LatestImage).To(Equal(imgRepo + ":1.0.1"))

// Updating the image should reconcile the cross-namespace policy
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -259,8 +259,7 @@ func (r *ImagePolicyReconciler) SetupWithManager(mgr ctrl.Manager, opts ImagePol
func (r *ImagePolicyReconciler) imagePoliciesForRepository(obj client.Object) []reconcile.Request {
ctx := context.Background()
var policies imagev1.ImagePolicyList
if err := r.List(ctx, &policies, client.InNamespace(obj.GetNamespace()),
client.MatchingFields{imageRepoKey: obj.GetName()}); err != nil {
if err := r.List(ctx, &policies, client.MatchingFields{imageRepoKey: obj.GetName()}); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This can result in collisions unless it includes the namespace (and the indexing should follow suit).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, @stefanprodan pointed this out and I'm working on it.

The imageRepoKey only contains the name of the ImageRepository resource.
This change uses the namespaced name to avoid collisions.

Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @relu

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.

ImagePolicy resources not reconciled by watched cross-namespace ImageRepositories the refer
3 participants