Fix GC

api/v1alpha1/utils.go

@@ -1,5 +1,7 @@
 package v1alpha1
 
+import "strconv"
+
 // LabelControllerManaged is a label on all resources managed by the controllers
 const LabelControllerManaged = "cosmo-workspace.github.io/controller-managed"
 
@@ -17,3 +19,29 @@ func SetControllerManaged(obj LabelHolder) {
 	labels[LabelControllerManaged] = "1"
 	obj.SetLabels(labels)
 }
+
+// +kubebuilder:object:generate=false
+type AnnotationHolder interface {
+	GetAnnotations() map[string]string
+	SetAnnotations(map[string]string)
+}
+
+// AnnotationPruneDisabled is a bool annotation for each child resources not to be deleted in GC
+const AnnotationPruneDisabled = "cosmo-workspace.github.io/prune-disabled"
+
+func IsPruneDisabled(obj AnnotationHolder) bool {
+	ann := obj.GetAnnotations()
+	if ann == nil {
+		return false
+	}
+	v, ok := ann[AnnotationPruneDisabled]
+	if !ok {
+		return false
+	}
+	isDisabled, err := strconv.ParseBool(v)
+	if err != nil {
+		// invalid bool value might be accidentally set while trying to be true
+		return true
+	}
+	return isDisabled
+}

internal/controllers/instance_controller.go

@@ -115,14 +115,13 @@ func (r *instanceReconciler) reconcileObjects(ctx context.Context, inst cosmov1a
 	log := clog.FromContext(ctx).WithCaller()
 	errs := make([]error, 0)
 
-	lastAppliedMap := sliceToObjectMap(inst.GetStatus().LastApplied)
-
 	lastApplied := make([]cosmov1alpha1.ObjectRef, len(inst.GetStatus().LastApplied))
 	copy(lastApplied, inst.GetStatus().LastApplied)
 
 	currAppliedMap := make(map[types.UID]cosmov1alpha1.ObjectRef)
+
+	// check dry-run apply on first reconciliation
 	if len(inst.GetStatus().LastApplied) == 0 {
-		// first reconcile
 		for _, built := range objects {
 			if _, err := r.dryrunApply(ctx, &built, r.FieldManager); err != nil {
 				// ignore NotFound in case the template contains a dependency resource that was not found.
@@ -161,12 +160,11 @@ func (r *instanceReconciler) reconcileObjects(ctx context.Context, inst cosmov1a
 				created, err := r.apply(ctx, &built, r.FieldManager)
 				if err != nil {
 					errs = append(errs, fmt.Errorf("failed to create resource: kind = %s name = %s: %w", built.GetKind(), built.GetName(), err))
-					continue
+				} else {
+					r.Recorder.Eventf(inst, corev1.EventTypeNormal, "Synced", "%s %s created", built.GetKind(), built.GetName())
 				}
-
-				r.Recorder.Eventf(inst, corev1.EventTypeNormal, "Synced", "%s %s created", built.GetKind(), built.GetName())
-
 				currAppliedMap[created.GetUID()] = unstToObjectRef(created)
+
 			} else {
 				errs = append(errs, fmt.Errorf("failed to get resource: kind = %s name = %s: %w", built.GetKind(), built.GetName(), err))
 				continue
@@ -179,11 +177,7 @@ func (r *instanceReconciler) reconcileObjects(ctx context.Context, inst cosmov1a
 				errs = append(errs, fmt.Errorf("dryrun failed: kind=%s name=%s: %w", built.GetKind(), built.GetName(), err))
 				continue
 			}
-			if l, ok := lastAppliedMap[desired.GetUID()]; !ok {
-				currAppliedMap[desired.GetUID()] = l
-			} else {
-				currAppliedMap[desired.GetUID()] = unstToObjectRef(desired)
-			}
+			currAppliedMap[desired.GetUID()] = unstToObjectRef(desired)
 
 			// compare current with the desired state
 			if !kubeutil.LooseDeepEqual(current, desired) {
@@ -194,44 +188,29 @@ func (r *instanceReconciler) reconcileObjects(ctx context.Context, inst cosmov1a
 				log.DumpObject(r.Scheme, &built, "applying object")
 				if _, err := r.apply(ctx, &built, r.FieldManager); err != nil {
 					errs = append(errs, fmt.Errorf("failed to apply resource %s %s: %w", built.GetKind(), built.GetName(), err))
-					continue
+				} else {
+					r.Recorder.Eventf(inst, corev1.EventTypeNormal, "Synced", "%s %s is not desired state, synced", built.GetKind(), built.GetName())
 				}
-
-				r.Recorder.Eventf(inst, corev1.EventTypeNormal, "Synced", "%s %s is not desired state, synced", built.GetKind(), built.GetName())
-
-				currAppliedMap[desired.GetUID()] = unstToObjectRef(desired)
 			}
 		}
 	}
 
-	for k, v := range currAppliedMap {
-		lastAppliedMap[k] = v
-	}
-
 	// garbage collection
-	shouldDeletes := objectRefNotExistsInMap(lastApplied, currAppliedMap)
-	for _, d := range shouldDeletes {
-		log.Info("start garbage collection", "apiVersion", d.APIVersion, "kind", d.Kind, "name", d.Name)
-		delete(lastAppliedMap, d.UID)
-
-		var obj unstructured.Unstructured
-		obj.SetAPIVersion(d.APIVersion)
-		obj.SetKind(d.Kind)
-		err := r.Get(ctx, types.NamespacedName{Name: d.GetName(), Namespace: d.Namespace}, &obj)
-		if err != nil {
-			if !apierrs.IsNotFound(err) {
-				log.Error(err, "failed to get object to be deleted", "apiVersion", d.APIVersion, "kind", d.Kind, "name", d.Name)
+	if len(errs) == 0 && !cosmov1alpha1.IsPruneDisabled(inst) {
+		log.Info("start garbage collection")
+		shouldDeletes := objectRefNotExistsInMap(lastApplied, currAppliedMap)
+		for _, d := range shouldDeletes {
+			if skip, err := prune(ctx, r.Client, d); err != nil {
+				log.Error(err, "failed to delete unused obj", "pruneAPIVersion", d.APIVersion, "pruneKind", d.Kind, "pruneName", d.Name, "pruneNamespace", d.Namespace)
+				r.Recorder.Eventf(inst, corev1.EventTypeWarning, "GCFailed", "failed to delete unused obj: kind=%s name=%s namespace=%s", d.Kind, d.Name, d.Namespace)
+			} else if !skip {
+				log.Info("deleted unmanaged object", "apiVersion", d.APIVersion, "kind", d.Kind, "name", d.Name, "namespace", d.Namespace)
+				r.Recorder.Eventf(inst, corev1.EventTypeNormal, "GC", "deleted unmanaged object: kind=%s name=%s namespace=%s", d.Kind, d.Name, d.Namespace)
 			}
-			continue
-		}
-
-		if err := r.Delete(ctx, &obj); err != nil {
-			r.Recorder.Eventf(inst, corev1.EventTypeWarning, "GCFailed", "failed to delete unused obj: %s %s", obj.GetKind(), obj.GetName())
 		}
-		r.Recorder.Eventf(inst, corev1.EventTypeNormal, "GC", "deleted unmanaged object: %s %s", obj.GetKind(), obj.GetName())
 	}
 
-	inst.GetStatus().LastApplied = objectRefMapToSlice(lastAppliedMap)
+	inst.GetStatus().LastApplied = objectRefMapToSlice(currAppliedMap)
 	inst.GetStatus().LastAppliedObjectsCount = len(inst.GetStatus().LastApplied)
 
 	return errs
@@ -287,3 +266,27 @@ func objectRefNotExistsInMap(s []cosmov1alpha1.ObjectRef, m map[types.UID]cosmov
 	}
 	return notExists
 }
+
+func prune(ctx context.Context, r client.Client, d cosmov1alpha1.ObjectRef) (skip bool, err error) {
+	log := clog.FromContext(ctx).WithValues("pruneAPIVersion", d.APIVersion, "pruneKind", d.Kind, "pruneName", d.Name, "pruneNamespace", d.Namespace)
+
+	var obj unstructured.Unstructured
+	obj.SetAPIVersion(d.APIVersion)
+	obj.SetKind(d.Kind)
+	err = r.Get(ctx, types.NamespacedName{Name: d.Name, Namespace: d.Namespace}, &obj)
+	if err != nil {
+		log.Error(err, "failed to get object to be deleted")
+		return true, nil
+	}
+
+	if obj.GetUID() != d.UID {
+		log.Error(err, "target object UID is changed. skip pruning", "desiredUID", d.UID, "currentUID", obj.GetUID())
+		return true, nil
+	}
+	if cosmov1alpha1.IsPruneDisabled(&obj) {
+		log.Debug().Info("skip pruning by annotation", "apiVersion")
+		return true, nil
+	}
+
+	return false, r.Delete(ctx, &obj)
+}

internal/controllers/user_controller.go

@@ -8,7 +8,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
@@ -157,26 +156,18 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		log.Debug().Info("status updated")
 	}
 
-	// garbage collection
-	shouldDeletes := objectRefNotExistsInMap(lastAddons, currAddonsMap)
-	for _, d := range shouldDeletes {
-		log.Info("start garbage collection", "apiVersion", d.APIVersion, "kind", d.Kind, "name", d.Name)
-
-		var obj unstructured.Unstructured
-		obj.SetAPIVersion(d.APIVersion)
-		obj.SetKind(d.Kind)
-		err := r.Get(ctx, types.NamespacedName{Name: d.GetName(), Namespace: d.Namespace}, &obj)
-		if err != nil {
-			if !apierrs.IsNotFound(err) {
-				log.Error(err, "failed to get object to be deleted", "apiVersion", d.APIVersion, "kind", d.Kind, "name", d.Name)
+	if user.Status.Phase != "AddonFailed" && !cosmov1alpha1.IsPruneDisabled(&user) {
+		log.Info("start garbage collection")
+		shouldDeletes := objectRefNotExistsInMap(lastAddons, currAddonsMap)
+		for _, d := range shouldDeletes {
+			if skip, err := prune(ctx, r.Client, d); err != nil {
+				log.Error(err, "failed to delete unused addon", "pruneAPIVersion", d.APIVersion, "pruneKind", d.Kind, "pruneName", d.Name, "pruneNamespace", d.Namespace)
+				r.Recorder.Eventf(&user, corev1.EventTypeWarning, "GCFailed", "failed to delete unused addon: kind=%s name=%s namespace=%s", d.Kind, d.Name, d.Namespace)
+			} else if !skip {
+				log.Info("deleted unmanaged addon", "apiVersion", d.APIVersion, "kind", d.Kind, "name", d.Name, "namespace", d.Namespace)
+				r.Recorder.Eventf(&user, corev1.EventTypeNormal, "GC", "deleted unmanaged addon: kind=%s name=%s namespace=%s", d.Kind, d.Name, d.Namespace)
 			}
-			continue
-		}
-
-		if err := r.Delete(ctx, &obj); err != nil {
-			r.Recorder.Eventf(&user, corev1.EventTypeWarning, "GCFailed", "failed to delete unused addon: %s %s", obj.GetKind(), obj.GetName())
 		}
-		r.Recorder.Eventf(&user, corev1.EventTypeNormal, "GC", "deleted unmanaged addon: %s %s", obj.GetKind(), obj.GetName())
 	}
 
 	log.Debug().Info("finish reconcile")

pkg/transformer/metadata.go

@@ -45,11 +45,12 @@ func (t *MetadataTransformer) Transform(src *unstructured.Unstructured) (*unstru
 	labels[cosmov1alpha1.LabelKeyTemplateName] = t.tmplName
 	obj.SetLabels(labels)
 
-	// Set owner reference
-	err := ctrl.SetControllerReference(t.inst, obj, t.scheme)
-	if err != nil {
-		return nil, fmt.Errorf("failed to set owner reference on %s: %w", obj.GetObjectKind().GroupVersionKind(), err)
+	if !cosmov1alpha1.IsPruneDisabled(t.inst) && !cosmov1alpha1.IsPruneDisabled(obj) {
+		// Set owner reference
+		err := ctrl.SetControllerReference(t.inst, obj, t.scheme)
+		if err != nil {
+			return nil, fmt.Errorf("failed to set owner reference on %s: %w", obj.GetObjectKind().GroupVersionKind(), err)
+		}
 	}
-
 	return obj, nil
 }

pkg/transformer/metadata_test.go

@@ -270,6 +270,57 @@ metadata:
 			},
 			wantErr: true,
 		},
+		{
+			name: "not put owner ref when prune-disabled annotated",
+			fields: fields{
+				inst: &cosmov1alpha1.Instance{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "cs1",
+						Namespace: "cosmo-user-tom",
+						Annotations: map[string]string{
+							cosmov1alpha1.AnnotationPruneDisabled: "1",
+						},
+					},
+					Spec: cosmov1alpha1.InstanceSpec{
+						Template: cosmov1alpha1.TemplateRef{
+							Name: "code-server",
+						},
+						Override: cosmov1alpha1.OverrideSpec{},
+						Vars:     map[string]string{"{{TEST}}": "OK"},
+					},
+				},
+				disableNamePrefix: false,
+				scheme:            scheme,
+			},
+			args: args{
+				src: `apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  annotations:
+    cosmo/ingress-patch-enable: "true"
+    kubernetes.io/ingress.class: alb
+  name: test
+  namespace: cosmo-user-tom
+spec:
+  host: example.com
+`,
+			},
+			want: `apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  annotations:
+    cosmo/ingress-patch-enable: "true"
+    kubernetes.io/ingress.class: alb
+  labels:
+    cosmo-workspace.github.io/instance: cs1
+    cosmo-workspace.github.io/template: code-server
+  name: cs1-test
+  namespace: cosmo-user-tom
+spec:
+  host: example.com
+`,
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {