From 315c36db3ecda14b4100908646a5db91293ad0ee Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Mon, 21 Jul 2025 17:04:44 +0200 Subject: [PATCH] [cozystack-controller] Fix deleting workloads Signed-off-by: Andrei Kvapil --- internal/controller/workload_controller.go | 52 ++++------- .../controller/workload_controller_test.go | 87 +++++++++++++++++++ .../controller/workloadmonitor_controller.go | 21 ++++- 3 files changed, 120 insertions(+), 40 deletions(-) diff --git a/internal/controller/workload_controller.go b/internal/controller/workload_controller.go index e3e85b01..82607375 100644 --- a/internal/controller/workload_controller.go +++ b/internal/controller/workload_controller.go @@ -26,60 +26,38 @@ type WorkloadReconciler struct { Scheme *runtime.Scheme } +// workload_controller.go func (r *WorkloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - logger := log.FromContext(ctx) + log := log.FromContext(ctx) + w := &cozyv1alpha1.Workload{} - err := r.Get(ctx, req.NamespacedName, w) - if err != nil { + if err := r.Get(ctx, req.NamespacedName, w); err != nil { if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } - logger.Error(err, "Unable to fetch Workload") return ctrl.Result{}, err } - // it's being deleted, nothing to handle - if w.DeletionTimestamp != nil { - return ctrl.Result{}, nil + // If my monitor is gone, delete me. + monName, has := w.Labels["workloadmonitor.cozystack.io/name"] + if !has { + return ctrl.Result{}, r.Delete(ctx, w) } - - t := getMonitoredObject(w) - - if t == nil { - err = r.Delete(ctx, w) - if err != nil { - logger.Error(err, "failed to delete workload") - } + monitor := &cozyv1alpha1.WorkloadMonitor{} + if err := r.Get(ctx, types.NamespacedName{Namespace: w.Namespace, Name: monName}, monitor); apierrors.IsNotFound(err) { + return ctrl.Result{}, r.Delete(ctx, w) + } else if err != nil { + log.Error(err, "failed to get WorkloadMonitor", "monitor", monName) return ctrl.Result{}, err } - err = r.Get(ctx, types.NamespacedName{Name: t.GetName(), Namespace: t.GetNamespace()}, t) - - // found object, nothing to do - if err == nil { - if !t.GetDeletionTimestamp().IsZero() { - return ctrl.Result{RequeueAfter: deletionRequeueDelay}, nil - } - return ctrl.Result{}, nil - } - - // error getting object but not 404 -- requeue - if !apierrors.IsNotFound(err) { - logger.Error(err, "failed to get dependent object", "kind", t.GetObjectKind(), "dependent-object-name", t.GetName()) - return ctrl.Result{}, err - } - - err = r.Delete(ctx, w) - if err != nil { - logger.Error(err, "failed to delete workload") - } - return ctrl.Result{}, err + return ctrl.Result{}, nil } // SetupWithManager registers our controller with the Manager and sets up watches. func (r *WorkloadReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - // Watch WorkloadMonitor objects + // Watch Workload objects For(&cozyv1alpha1.Workload{}). Complete(r) } diff --git a/internal/controller/workload_controller_test.go b/internal/controller/workload_controller_test.go index 816c135e..b454adc3 100644 --- a/internal/controller/workload_controller_test.go +++ b/internal/controller/workload_controller_test.go @@ -1,10 +1,17 @@ package controller import ( + "context" "testing" cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) func TestUnprefixedMonitoredObjectReturnsNil(t *testing.T) { @@ -24,3 +31,83 @@ func TestPodMonitoredObject(t *testing.T) { t.Errorf(`getMonitoredObject(&Workload{Name: "%s"}) == %v, want &Pod{Name: "mypod"}`, w.Name, obj) } } + +func TestWorkloadReconciler_DeletesOnMissingMonitor(t *testing.T) { + scheme := runtime.NewScheme() + _ = cozyv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + // Workload with a non-existent monitor + w := &cozyv1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-foo", + Namespace: "default", + Labels: map[string]string{ + "workloadmonitor.cozystack.io/name": "missing-monitor", + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(w). + Build() + reconciler := &WorkloadReconciler{Client: fakeClient} + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "pod-foo", Namespace: "default"}} + + if _, err := reconciler.Reconcile(context.TODO(), req); err != nil { + t.Fatalf("Reconcile returned error: %v", err) + } + + // Expect workload to be deleted + err := fakeClient.Get(context.TODO(), req.NamespacedName, &cozyv1alpha1.Workload{}) + if !apierrors.IsNotFound(err) { + t.Errorf("expected workload to be deleted, got: %v", err) + } +} + +func TestWorkloadReconciler_KeepsWhenAllExist(t *testing.T) { + scheme := runtime.NewScheme() + _ = cozyv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + // Create a monitor and its backing Pod + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon", + Namespace: "default", + }, + } + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "default", + }, + } + w := &cozyv1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-foo", + Namespace: "default", + Labels: map[string]string{ + "workloadmonitor.cozystack.io/name": "mon", + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(monitor, pod, w). + Build() + reconciler := &WorkloadReconciler{Client: fakeClient} + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "pod-foo", Namespace: "default"}} + + if _, err := reconciler.Reconcile(context.TODO(), req); err != nil { + t.Fatalf("Reconcile returned error: %v", err) + } + + // Expect workload to remain + err := fakeClient.Get(context.TODO(), req.NamespacedName, &cozyv1alpha1.Workload{}) + if err != nil { + t.Errorf("expected workload to persist, got error: %v", err) + } +} diff --git a/internal/controller/workloadmonitor_controller.go b/internal/controller/workloadmonitor_controller.go index 4408800e..561de772 100644 --- a/internal/controller/workloadmonitor_controller.go +++ b/internal/controller/workloadmonitor_controller.go @@ -137,10 +137,15 @@ func (r *WorkloadMonitorReconciler) reconcileServiceForMonitor( _, err := ctrl.CreateOrUpdate(ctx, r.Client, workload, func() error { // Update owner references with the new monitor - updateOwnerReferences(workload.GetObjectMeta(), monitor) + updateOwnerReferences(workload.GetObjectMeta(), &svc) workload.Labels = svc.Labels + if workload.Labels == nil { + workload.Labels = map[string]string{} + } + workload.Labels["workloads.cozystack.io/monitor"] = monitor.Name + // Fill Workload status fields: workload.Status.Kind = monitor.Spec.Kind workload.Status.Type = monitor.Spec.Type @@ -184,7 +189,12 @@ func (r *WorkloadMonitorReconciler) reconcilePVCForMonitor( _, err := ctrl.CreateOrUpdate(ctx, r.Client, workload, func() error { // Update owner references with the new monitor - updateOwnerReferences(workload.GetObjectMeta(), monitor) + updateOwnerReferences(workload.GetObjectMeta(), &pvc) + + if workload.Labels == nil { + workload.Labels = map[string]string{} + } + workload.Labels["workloads.cozystack.io/monitor"] = monitor.Name workload.Labels = pvc.Labels @@ -255,7 +265,12 @@ func (r *WorkloadMonitorReconciler) reconcilePodForMonitor( metaLabels := r.getWorkloadMetadata(&pod) _, err := ctrl.CreateOrUpdate(ctx, r.Client, workload, func() error { // Update owner references with the new monitor - updateOwnerReferences(workload.GetObjectMeta(), monitor) + updateOwnerReferences(workload.GetObjectMeta(), &pod) + + if workload.Labels == nil { + workload.Labels = map[string]string{} + } + workload.Labels["workloads.cozystack.io/monitor"] = monitor.Name // Copy labels from the Pod if needed for k, v := range pod.Labels {