fix: update owner references (#773)

eraser-dev · Jul 28, 2023 · 575b3ef · 575b3ef
1 parent f08f9f3
commit 575b3ef
Show file tree

Hide file tree

Showing 5 changed files with 218 additions and 5 deletions.
diff --git a/controllers/imagecollector/imagecollector_controller.go b/controllers/imagecollector/imagecollector_controller.go
@@ -49,10 +49,12 @@ import (
 
 	"github.com/Azure/eraser/pkg/logger"
 	"github.com/Azure/eraser/pkg/metrics"
+	eraserUtils "github.com/Azure/eraser/pkg/utils"
 	sdkmetric "go.opentelemetry.io/otel/sdk/metric"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 const (
@@ -253,6 +255,23 @@ func (r *Reconciler) handleJobDeletion(ctx context.Context, job *eraserv1.ImageJ
 	if err != nil {
 		return ctrl.Result{}, err
 	}
+
+	template := corev1.PodTemplate{}
+	if err := r.Get(ctx,
+		types.NamespacedName{
+			Namespace: eraserUtils.GetNamespace(),
+			Name:      job.GetName(),
+		},
+		&template,
+	); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	log.Info("Deleting pod template", "template", template.Name)
+	if err := r.Delete(ctx, &template); err != nil {
+		return ctrl.Result{}, err
+	}
+
 	log.Info("end job deletion")
 	return ctrl.Result{}, nil
 }
@@ -449,17 +468,28 @@ func (r *Reconciler) createImageJob(ctx context.Context) (ctrl.Result, error) {
 		return reconcile.Result{}, err
 	}
 
+	// get manager pod with label control-plane=controller-manager
+	podList := corev1.PodList{}
+	if err := r.List(ctx, &podList, client.InNamespace(utils.GetNamespace()), client.MatchingLabels{"control-plane": "controller-manager"}); err != nil {
+		log.Info("Unable to list controller-manager pod")
+	}
+	if len(podList.Items) != 1 {
+		log.Info("Incorrect number of controller-manager pods", "number of pods", len(podList.Items))
+	}
+	managerPod := &podList.Items[0]
+
 	namespace := utils.GetNamespace()
 	template := corev1.PodTemplate{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      job.GetName(),
 			Namespace: namespace,
 			OwnerReferences: []metav1.OwnerReference{
-				*metav1.NewControllerRef(job, eraserv1.GroupVersion.WithKind("ImageJob")),
+				*metav1.NewControllerRef(managerPod, managerPod.GroupVersionKind()),
 			},
 		},
 		Template: jobTemplate,
 	}
+
 	err = r.Create(ctx, &template)
 	if err != nil {
 		log.Error(err, "Could not create collector PodTemplate")

diff --git a/controllers/imagejob/imagejob_controller.go b/controllers/imagejob/imagejob_controller.go
@@ -55,6 +55,8 @@ const (
 	collectorJobType     = "collector"
 	manualJobType        = "manual"
 	removerContainer     = "remover"
+	managerLabelValue    = "controller-manager"
+	managerLabelKey      = "control-plane"
 )
 
 var log = logf.Log.WithName("controller").WithValues("process", "imagejob-controller")
@@ -120,22 +122,48 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 			Type: &corev1.Pod{},
 		},
 		&handler.EnqueueRequestForOwner{
-			OwnerType:    &eraserv1.ImageJob{},
+			OwnerType:    &corev1.PodTemplate{},
 			IsController: true,
 		},
+		predicate.Funcs{
+			CreateFunc: func(e event.CreateEvent) bool {
+				return e.Object.GetNamespace() == eraserUtils.GetNamespace()
+			},
+			UpdateFunc: func(e event.UpdateEvent) bool {
+				return e.ObjectNew.GetNamespace() == eraserUtils.GetNamespace()
+			},
+			DeleteFunc: func(e event.DeleteEvent) bool {
+				return e.Object.GetNamespace() == eraserUtils.GetNamespace()
+			},
+		},
 	)
 	if err != nil {
 		return err
 	}
 
+	// watch for changes to imagejob podTemplate (owned by controller manager pod)
 	err = c.Watch(
 		&source.Kind{
 			Type: &corev1.PodTemplate{},
 		},
 		&handler.EnqueueRequestForOwner{
-			OwnerType:    &eraserv1.ImageJob{},
+			OwnerType:    &corev1.Pod{},
 			IsController: true,
 		},
+		predicate.Funcs{
+			CreateFunc: func(e event.CreateEvent) bool {
+				ownerLabels, ok := e.Object.GetLabels()[managerLabelKey]
+				return ok && ownerLabels == managerLabelValue
+			},
+			UpdateFunc: func(e event.UpdateEvent) bool {
+				ownerLabels, ok := e.ObjectNew.GetLabels()[managerLabelKey]
+				return ok && ownerLabels == managerLabelValue
+			},
+			DeleteFunc: func(e event.DeleteEvent) bool {
+				ownerLabels, ok := e.Object.GetLabels()[managerLabelKey]
+				return ok && ownerLabels == managerLabelValue
+			},
+		},
 	)
 	if err != nil {
 		return err
@@ -370,7 +398,7 @@ func (r *Reconciler) handleNewJob(ctx context.Context, imageJob *eraserv1.ImageJ
 				Namespace:    eraserUtils.GetNamespace(),
 				GenerateName: "eraser-" + nodeName + "-",
 				OwnerReferences: []metav1.OwnerReference{
-					*metav1.NewControllerRef(imageJob, imageJob.GroupVersionKind()),
+					*metav1.NewControllerRef(&template, template.GroupVersionKind()),
 				},
 			},
 		}

diff --git a/controllers/imagelist/imagelist_controller.go b/controllers/imagelist/imagelist_controller.go
@@ -47,6 +47,7 @@ import (
 	"github.com/Azure/eraser/pkg/logger"
 	"github.com/Azure/eraser/pkg/metrics"
 	"github.com/Azure/eraser/pkg/utils"
+	eraserUtils "github.com/Azure/eraser/pkg/utils"
 	sdkmetric "go.opentelemetry.io/otel/sdk/metric"
 )
 
@@ -233,6 +234,22 @@ func (r *Reconciler) handleJobDeletion(ctx context.Context, job *eraserv1.ImageJ
 		return ctrl.Result{}, err
 	}
 
+	template := corev1.PodTemplate{}
+	if err := r.Get(ctx,
+		types.NamespacedName{
+			Namespace: eraserUtils.GetNamespace(),
+			Name:      job.GetName(),
+		},
+		&template,
+	); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	log.Info("Deleting pod template", "template", template.Name)
+	if err := r.Delete(ctx, &template); err != nil {
+		return ctrl.Result{}, err
+	}
+
 	return ctrl.Result{}, nil
 }
 
@@ -364,12 +381,22 @@ func (r *Reconciler) handleImageListEvent(ctx context.Context, imageList *eraser
 		return reconcile.Result{}, err
 	}
 
+	// get manager pod with label control-plane=controller-manager
+	podList := corev1.PodList{}
+	if err = r.List(ctx, &podList, client.InNamespace(utils.GetNamespace()), client.MatchingLabels{"control-plane": "controller-manager"}); err != nil {
+		log.Info("Unable to list controller-manager pod")
+	}
+	if len(podList.Items) != 1 {
+		log.Info("Incorrect number of controller-manager pods", "number of pods", len(podList.Items))
+	}
+	managerPod := &podList.Items[0]
+
 	template := corev1.PodTemplate{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      job.GetName(),
 			Namespace: utils.GetNamespace(),
 			OwnerReferences: []metav1.OwnerReference{
-				*metav1.NewControllerRef(job, eraserv1.GroupVersion.WithKind("ImageJob")),
+				*metav1.NewControllerRef(managerPod, managerPod.GroupVersionKind()),
 			},
 		},
 		Template: jobTemplate,

diff --git a/test/e2e/tests/collector_delete_deployment/eraser_test.go b/test/e2e/tests/collector_delete_deployment/eraser_test.go
@@ -0,0 +1,69 @@
+//go:build e2e
+// +build e2e
+
+package e2e
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Azure/eraser/test/e2e/util"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"sigs.k8s.io/e2e-framework/klient/wait"
+	"sigs.k8s.io/e2e-framework/klient/wait/conditions"
+	"sigs.k8s.io/e2e-framework/pkg/envconf"
+	"sigs.k8s.io/e2e-framework/pkg/features"
+)
+
+func TestDeleteDeployment(t *testing.T) {
+	deleteDeploymentFeat := features.New("Delete deployment should delete eraser pods").
+		Assess("Non-vulnerable image successfully deleted from all nodes", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			ctxT, cancel := context.WithTimeout(ctx, util.Timeout)
+			defer cancel()
+			util.CheckImageRemoved(ctxT, t, util.GetClusterNodes(t), util.NonVulnerableImage)
+
+			return ctx
+		}).
+		Assess("Delete deployment", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			if err := util.GetPodLogs(t); err != nil {
+				t.Error("error getting eraser pod logs", err)
+			}
+
+			if err := util.KubectlDelete(cfg.KubeconfigFile(), util.TestNamespace, []string{"deployment", "eraser-controller-manager"}); err != nil {
+				t.Error("unable to delete eraser-controller-manager deployment")
+			}
+
+			return ctx
+		}).
+		Assess("Check eraser pods are deleted", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			if err := util.GetPodLogs(t); err != nil {
+				t.Error("error getting eraser pod logs", err)
+			}
+
+			c, err := cfg.NewClient()
+			if err != nil {
+				t.Fatal("Failed to create new client", err)
+			}
+
+			var ls corev1.PodList
+			err = c.Resources().List(ctx, &ls, func(o *metav1.ListOptions) {
+				o.LabelSelector = labels.SelectorFromSet(map[string]string{util.ImageJobTypeLabelKey: util.CollectorLabel}).String()
+			})
+			if err != nil {
+				t.Errorf("could not list pods: %v", err)
+			}
+
+			err = wait.For(conditions.New(c.Resources()).ResourcesDeleted(&ls), wait.WithTimeout(util.Timeout))
+			if err != nil {
+				t.Errorf("error waiting for pods to be deleted: %v", err)
+			}
+
+			return ctx
+		}).
+		Feature()
+
+	util.Testenv.Test(t, deleteDeploymentFeat)
+}
diff --git a/test/e2e/tests/collector_delete_deployment/main_test.go b/test/e2e/tests/collector_delete_deployment/main_test.go
@@ -0,0 +1,59 @@
+//go:build e2e
+// +build e2e
+
+package e2e
+
+import (
+	"os"
+	"testing"
+
+	eraserv1alpha1 "github.com/Azure/eraser/api/v1alpha1"
+	"github.com/Azure/eraser/test/e2e/util"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/e2e-framework/pkg/env"
+	"sigs.k8s.io/e2e-framework/pkg/envconf"
+	"sigs.k8s.io/e2e-framework/pkg/envfuncs"
+)
+
+func TestMain(m *testing.M) {
+	utilruntime.Must(eraserv1alpha1.AddToScheme(scheme.Scheme))
+
+	removerImage := util.ParsedImages.RemoverImage
+	managerImage := util.ParsedImages.ManagerImage
+	collectorImage := util.ParsedImages.CollectorImage
+
+	util.Testenv = env.NewWithConfig(envconf.New())
+	// Create KinD Cluster
+	util.Testenv.Setup(
+		envfuncs.CreateKindClusterWithConfig(util.KindClusterName, util.NodeVersion, util.KindConfigPath),
+		envfuncs.CreateNamespace(util.TestNamespace),
+		util.LoadImageToCluster(util.KindClusterName, util.ManagerImage, util.ManagerTarballPath),
+		util.LoadImageToCluster(util.KindClusterName, util.RemoverImage, util.RemoverTarballPath),
+		util.LoadImageToCluster(util.KindClusterName, util.RemoverImage, util.RemoverTarballPath),
+		util.LoadImageToCluster(util.KindClusterName, util.CollectorImage, util.CollectorTarballPath),
+		util.LoadImageToCluster(util.KindClusterName, util.NonVulnerableImage, ""),
+		util.HelmDeployLatestEraserRelease(util.TestNamespace,
+			"--set", util.ScannerEnable.Set("false"),
+			"--set", util.CollectorEnable.Set("false"),
+			"--set", util.RemoverImageRepo.Set(removerImage.Repo),
+			"--set", util.RemoverImageTag.Set(removerImage.Tag),
+			"--set", util.ManagerImageRepo.Set(managerImage.Repo),
+			"--set", util.ManagerImageTag.Set(managerImage.Tag),
+		),
+		util.UpgradeEraserHelm(util.TestNamespace,
+			"--set", util.ScannerEnable.Set("false"),
+			"--set", util.RemoverImageRepo.Set(removerImage.Repo),
+			"--set", util.RemoverImageTag.Set(removerImage.Tag),
+			"--set", util.CollectorEnable.Set("true"),
+			"--set", util.CollectorImageRepo.Set(collectorImage.Repo),
+			"--set", util.CollectorImageTag.Set(collectorImage.Tag),
+			"--set", util.ManagerImageRepo.Set(managerImage.Repo),
+			"--set", util.ManagerImageTag.Set(managerImage.Tag),
+			"--set", util.CleanupOnSuccessDelay.Set("2m"),
+		),
+	).Finish(
+		envfuncs.DestroyKindCluster(util.KindClusterName),
+	)
+	os.Exit(util.Testenv.Run(m))
+}