Remove proof-of-concept update mode

The initial implementation of image updates required a single image
policy, and updated every image field that used the image, in the git
repo. This mode has limited practical value, and rather than
elaborating on it, it would be better to concentrate on making the
more carefully thought-through "setters" mode.

Author: squaremo

api/v1alpha1/imageupdateautomation_types.go

@@ -21,9 +21,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
-// NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
-
 // ImageUpdateAutomationSpec defines the desired state of ImageUpdateAutomation
 type ImageUpdateAutomationSpec struct {
 	// Checkout gives the parameters for cloning the git repository,
@@ -58,10 +55,6 @@ type GitCheckoutSpec struct {
 // UpdateStrategy is a union of the various strategies for updating
 // the git repository.
 type UpdateStrategy struct {
-	// ImagePolicyRef if present means update all workloads using the
-	// given policy's image, to the policy's latest image reference.
-	// +optional
-	ImagePolicyRef *corev1.LocalObjectReference `json:"imagePolicyRef,omitempty"`
 	// Setters if present means update workloads using setters, via
 	// fields marked in the files themselves.
 	// +optional

api/v1alpha1/zz_generated.deepcopy.go

@@ -90,15 +90,6 @@ spec:
               description: Update gives the specification for how to update the files
                 in the repository
               properties:
-                imagePolicyRef:
-                  description: ImagePolicyRef if present means update all workloads
-                    using the given policy's image, to the policy's latest image reference.
-                  properties:
-                    name:
-                      description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
-                        TODO: Add other useful fields. apiVersion, kind, uid?'
-                      type: string
-                  type: object
                 setters:
                   description: Setters if present means update workloads using setters,
                     via fields marked in the files themselves.

config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml

@@ -121,28 +121,6 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(req ctrl.Request) (ctrl.Resu
 
 	updateStrat := auto.Spec.Update
 	switch {
-	case updateStrat.ImagePolicyRef != nil:
-		var policy imagev1alpha1_reflect.ImagePolicy
-		policyName := types.NamespacedName{
-			Namespace: auto.GetNamespace(),
-			Name:      updateStrat.ImagePolicyRef.Name,
-		}
-		if err := r.Get(ctx, policyName, &policy); err != nil {
-			if client.IgnoreNotFound(err) == nil {
-				log.Info("referenced ImagePolicy not found")
-				// assume we'll be told if the image policy turns up, or if this resource changes
-				return ctrl.Result{}, nil
-			}
-			return ctrl.Result{}, err
-		}
-		if err := updateAccordingToImagePolicy(ctx, tmp, &policy); err != nil {
-			if err == errImagePolicyNotReady {
-				log.Info("image policy does not have latest image ref", "imagepolicy", policyName)
-				// assume we'll be told if the image policy or this resource changes
-				return ctrl.Result{}, nil
-			}
-			return ctrl.Result{}, err
-		}
 	case updateStrat.Setters != nil:
 		// For setters we first want to compile a list of _all_ the
 		// policies in the same namespace (maybe in the future this
@@ -211,27 +189,12 @@ func (r *ImageUpdateAutomationReconciler) SetupWithManager(mgr ctrl.Manager) err
 		return err
 	}
 
-	// Index the image policy (if any) that each I-U-A refers to
-	if err := mgr.GetFieldIndexer().IndexField(ctx, &imagev1alpha1.ImageUpdateAutomation{}, imagePolicyKey, func(obj runtime.Object) []string {
-		updater := obj.(*imagev1alpha1.ImageUpdateAutomation)
-		if ref := updater.Spec.Update.ImagePolicyRef; ref != nil {
-			return []string{ref.Name}
-		}
-		return nil
-	}); err != nil {
-		return err
-	}
-
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&imagev1alpha1.ImageUpdateAutomation{}).
 		Watches(&source.Kind{Type: &sourcev1alpha1.GitRepository{}},
 			&handler.EnqueueRequestsFromMapFunc{
 				ToRequests: handler.ToRequestsFunc(r.automationsForGitRepo),
 			}).
-		Watches(&source.Kind{Type: &imagev1alpha1_reflect.ImagePolicy{}},
-			&handler.EnqueueRequestsFromMapFunc{
-				ToRequests: handler.ToRequestsFunc(r.automationsForImagePolicy),
-			}).
 		Complete(r)
 }
 
@@ -274,26 +237,6 @@ func (r *ImageUpdateAutomationReconciler) automationsForGitRepo(obj handler.MapO
 	return reqs
 }
 
-// automationsForImagePolicy fetches all the automations that refer to
-// a particular source.ImagePolicy object.
-func (r *ImageUpdateAutomationReconciler) automationsForImagePolicy(obj handler.MapObject) []reconcile.Request {
-	ctx := context.Background()
-	var autoList imagev1alpha1.ImageUpdateAutomationList
-	if err := r.List(ctx, &autoList, client.InNamespace(obj.Meta.GetNamespace()), client.MatchingFields{imagePolicyKey: obj.Meta.GetName()}); err != nil {
-		r.Log.Error(err, "failed to list ImageUpdateAutomations for ImagePolicy", "name", types.NamespacedName{
-			Name:      obj.Meta.GetName(),
-			Namespace: obj.Meta.GetNamespace(),
-		})
-		return nil
-	}
-	reqs := make([]reconcile.Request, len(autoList.Items), len(autoList.Items))
-	for i := range autoList.Items {
-		reqs[i].NamespacedName.Name = autoList.Items[i].GetName()
-		reqs[i].NamespacedName.Namespace = autoList.Items[i].GetNamespace()
-	}
-	return reqs
-}
-
 // --- git ops
 
 type repoAccess struct {
@@ -387,24 +330,6 @@ func commitAllAndPush(ctx context.Context, repo *gogit.Repository, access repoAc
 
 // --- updates
 
-var errImagePolicyNotReady = errors.New("ImagePolicy resource is not ready")
-
-// update the manifest files under path according to policy, by
-// replacing any mention of the policy's image repository with the
-// latest ref.
-func updateAccordingToImagePolicy(ctx context.Context, path string, policy *imagev1alpha1_reflect.ImagePolicy) error {
-	// the function that does the update expects an original and a
-	// replacement; but it only uses the repository part of the
-	// original, and it compares canonical forms (with the defaults
-	// filled in). Since the latest image will have the same
-	// repository, I can just pass that as the original.
-	latestRef := policy.Status.LatestImage
-	if latestRef == "" {
-		return errImagePolicyNotReady
-	}
-	return update.UpdateImageEverywhere(path, path, latestRef, latestRef)
-}
-
 // updateAccordingToSetters updates files under the root by treating
 // the given image policies as kyaml setters.
 func updateAccordingToSetters(ctx context.Context, path string, policies []imagev1alpha1_reflect.ImagePolicy) error {

controllers/imageupdateautomation_controller.go

@@ -174,95 +174,6 @@ var _ = Describe("ImageUpdateAutomation", func() {
 			Expect(k8sClient.Delete(context.Background(), policy)).To(Succeed())
 		})
 
-		Context("with ImagePolicyRef strategy", func() {
-
-			var (
-				updateKey           types.NamespacedName
-				updateByImagePolicy *imagev1alpha1.ImageUpdateAutomation
-			)
-
-			BeforeEach(func() {
-				updateKey = types.NamespacedName{
-					Namespace: gitRepoKey.Namespace,
-					Name:      "update-" + randStringRunes(5),
-				}
-				updateByImagePolicy = &imagev1alpha1.ImageUpdateAutomation{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      updateKey.Name,
-						Namespace: updateKey.Namespace,
-					},
-					Spec: imagev1alpha1.ImageUpdateAutomationSpec{
-						Checkout: imagev1alpha1.GitCheckoutSpec{
-							GitRepositoryRef: corev1.LocalObjectReference{
-								Name: gitRepoKey.Name,
-							},
-							Branch: defaultBranch,
-						},
-						Update: imagev1alpha1.UpdateStrategy{
-							ImagePolicyRef: &corev1.LocalObjectReference{
-								Name: policyKey.Name,
-							},
-						},
-						Commit: imagev1alpha1.CommitSpec{
-							MessageTemplate: commitMessage,
-						},
-					},
-				}
-				Expect(k8sClient.Create(context.Background(), updateByImagePolicy)).To(Succeed())
-				waitForNewHead(localRepo)
-			})
-
-			AfterEach(func() {
-				Expect(k8sClient.Delete(context.Background(), updateByImagePolicy)).To(Succeed())
-			})
-
-			It("updates to the most recent image", func() {
-				// having passed the BeforeEach, we should see a commit
-				head, _ := localRepo.Head()
-				commit, err := localRepo.CommitObject(head.Hash())
-				Expect(err).ToNot(HaveOccurred())
-				Expect(commit.Message).To(Equal(commitMessage))
-
-				tmp, err := ioutil.TempDir("", "gotest-imageauto")
-				Expect(err).ToNot(HaveOccurred())
-				defer os.RemoveAll(tmp)
-
-				_, err = git.PlainClone(tmp, false, &git.CloneOptions{
-					URL:           repoURL,
-					ReferenceName: plumbing.NewBranchReferenceName(defaultBranch),
-				})
-				Expect(err).ToNot(HaveOccurred())
-				test.ExpectMatchingDirectories(tmp, "testdata/appconfig-expected")
-			})
-
-			It("makes a commit when the policy changes", func() {
-				// make sure the first commit happened
-				head, _ := localRepo.Head()
-				commit, err := localRepo.CommitObject(head.Hash())
-				Expect(err).ToNot(HaveOccurred())
-				Expect(commit.Message).To(Equal(commitMessage))
-
-				// change the status and
-				// make sure there's a commit for that.
-				policy.Status.LatestImage = evenLatestImage
-				Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed())
-
-				Expect(err).ToNot(HaveOccurred())
-				waitForNewHead(localRepo)
-
-				tmp, err := ioutil.TempDir("", "gotest-imageauto")
-				Expect(err).ToNot(HaveOccurred())
-				defer os.RemoveAll(tmp)
-
-				_, err = git.PlainClone(tmp, false, &git.CloneOptions{
-					URL:           repoURL,
-					ReferenceName: plumbing.NewBranchReferenceName(defaultBranch),
-				})
-				Expect(err).ToNot(HaveOccurred())
-				test.ExpectMatchingDirectories(tmp, "testdata/appconfig-expected2")
-			})
-		})
-
 		Context("with Setters", func() {
 
 			var (

controllers/update_test.go

@@ -18,32 +18,6 @@ func TestUpdate(t *testing.T) {
 	RunSpecs(t, "Update suite")
 }
 
-var _ = Describe("Update image everywhere from policy", func() {
-	It("leaves a different image alone", func() {
-		tmp, err := ioutil.TempDir("", "gotest")
-		Expect(err).ToNot(HaveOccurred())
-		defer os.RemoveAll(tmp)
-		Expect(UpdateImageEverywhere("testdata/leave/original", tmp, "notused", "notused:v1.0.1")).To(Succeed())
-		test.ExpectMatchingDirectories(tmp, "testdata/leave/expected")
-	})
-
-	It("replaces the given image", func() {
-		tmp, err := ioutil.TempDir("", "gotest")
-		Expect(err).ToNot(HaveOccurred())
-		defer os.RemoveAll(tmp)
-		Expect(UpdateImageEverywhere("testdata/replace/original", tmp, "used", "used:v1.1.0")).To(Succeed())
-		test.ExpectMatchingDirectories(tmp, "testdata/replace/expected")
-	})
-
-	It("keeps comments intact", func() {
-		tmp, err := ioutil.TempDir("", "gotest")
-		Expect(err).ToNot(HaveOccurred())
-		defer os.RemoveAll(tmp)
-		Expect(UpdateImageEverywhere("testdata/replace/commented", tmp, "used", "used:v1.1.0")).To(Succeed())
-		test.ExpectMatchingDirectories(tmp, "testdata/replace/commented-expected")
-	})
-})
-
 var _ = Describe("Update image via kyaml setters2", func() {
 	It("updates the image marked with the image policy (setter) ref", func() {
 		tmp, err := ioutil.TempDir("", "gotest")