Garbage collection for Update objects (#810)

<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes

<!--Give us a brief description of what you've done and what it solves.
-->
Implements a retention policy for completed Update objects, based on a
simple TTL with the default value of 24h. Update objects that are in a
completed state and are older than the TTL are summarily deleted.

#### New API: `Update.spec.ttlAfterCompleted` 

A new field to enable garbage collection of this update, once completed.
Optional; no default value.

#### Update Controller

Updated to implement the cleanup logic. When a TTL is set on a completed
update, the controller looks to the last transition time of the
Completed status condition. If there's time remaining, the controller
requeues the object for later, otherwise it deletes the update object.

#### Stack Controller

Updated to assign a 24-hour TTL on the update objects that it creates. A
future enhancement would be to provide an `updateTemplate` field to
allow for further control. A user may edit an existing Update to change
the TTL.

#### Update Controller Tests

A new unit test was added to exercise the retention logic.

### Related issues (optional)

<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->

Closes #733

CHANGELOG.md

@@ -9,6 +9,7 @@ CHANGELOG
 - Surface update conflict errors when a stack is locked [#807](https://github.com/pulumi/pulumi-kubernetes-operator/pull/807)4 (add changelog entry)
 - Do not destroy the workspace pod if an authentication error occurs [#805](https://github.com/pulumi/pulumi-kubernetes-operator/pull/805)
 - Use 'parallel' policy for workspace pod rollouts to avoid stalls. [#802](https://github.com/pulumi/pulumi-kubernetes-operator/pull/802)
+- Garbage collection for Update objects. [#810](https://github.com/pulumi/pulumi-kubernetes-operator/pull/810)
 
 ## 2.0.0-beta.3 (2024-11-27)
 

deploy/crds/auto.pulumi.com_updates.yaml

@@ -100,6 +100,7 @@ spec:
                   type: string
                 type: array
               stackName:
+                description: Specify the Pulumi stack to select for the update.
                 type: string
               target:
                 description: Specify an exclusive list of resource URNs to update
@@ -111,7 +112,11 @@ spec:
                   TargetDependents allows updating of dependent targets discovered but not
                   specified in the Target list
                 type: boolean
+              ttlAfterCompleted:
+                description: TTL for a completed update object.
+                type: string
               type:
+                description: Type of the update to perform.
                 type: string
               workspaceName:
                 description: WorkspaceName is the workspace to update.

deploy/helm/pulumi-operator/crds/auto.pulumi.com_updates.yaml

@@ -100,6 +100,7 @@ spec:
                   type: string
                 type: array
               stackName:
+                description: Specify the Pulumi stack to select for the update.
                 type: string
               target:
                 description: Specify an exclusive list of resource URNs to update
@@ -111,7 +112,11 @@ spec:
                   TargetDependents allows updating of dependent targets discovered but not
                   specified in the Target list
                 type: boolean
+              ttlAfterCompleted:
+                description: TTL for a completed update object.
+                type: string
               type:
+                description: Type of the update to perform.
                 type: string
               workspaceName:
                 description: WorkspaceName is the workspace to update.

deploy/yaml/install.yaml

@@ -1008,6 +1008,14 @@ spec:
                   creating stacks that do not exist in the tracking git repo.
                   The default behavior is to create a stack if it doesn't exist.
                 type: boolean
+              workspaceReclaimPolicy:
+                description: |-
+                  WorkspaceReclaimPolicy specifies whether the workspace should be deleted or retained after the Stack is synced.
+                  The default behavior is to retain the workspace. Valid values are one of "Retain" or "Delete".
+                enum:
+                - Retain
+                - Delete
+                type: string
               workspaceTemplate:
                 description: |-
                   WorkspaceTemplate customizes the Workspace generated for this Stack. It
@@ -9860,6 +9868,10 @@ spec:
                   lastSuccessfulCommit:
                     description: Last commit successfully applied
                     type: string
+                  message:
+                    description: Message is the message surfacing any errors or additional
+                      information about the update.
+                    type: string
                   name:
                     description: Name is the name of the update object.
                     type: string
@@ -10632,6 +10644,14 @@ spec:
                   creating stacks that do not exist in the tracking git repo.
                   The default behavior is to create a stack if it doesn't exist.
                 type: boolean
+              workspaceReclaimPolicy:
+                description: |-
+                  WorkspaceReclaimPolicy specifies whether the workspace should be deleted or retained after the Stack is synced.
+                  The default behavior is to retain the workspace. Valid values are one of "Retain" or "Delete".
+                enum:
+                - Retain
+                - Delete
+                type: string
               workspaceTemplate:
                 description: |-
                   WorkspaceTemplate customizes the Workspace generated for this Stack. It
@@ -19408,6 +19428,10 @@ spec:
                   lastSuccessfulCommit:
                     description: Last commit successfully applied
                     type: string
+                  message:
+                    description: Message is the message surfacing any errors or additional
+                      information about the update.
+                    type: string
                   name:
                     description: Name is the name of the update object.
                     type: string
@@ -19541,6 +19565,7 @@ spec:
                   type: string
                 type: array
               stackName:
+                description: Specify the Pulumi stack to select for the update.
                 type: string
               target:
                 description: Specify an exclusive list of resource URNs to update
@@ -19552,7 +19577,11 @@ spec:
                   TargetDependents allows updating of dependent targets discovered but not
                   specified in the Target list
                 type: boolean
+              ttlAfterCompleted:
+                description: TTL for a completed update object.
+                type: string
               type:
+                description: Type of the update to perform.
                 type: string
               workspaceName:
                 description: WorkspaceName is the workspace to update.

operator/api/auto/v1alpha1/update_types.go

@@ -39,10 +39,13 @@ const (
 type UpdateSpec struct {
 	// WorkspaceName is the workspace to update.
 	WorkspaceName string `json:"workspaceName,omitempty"`
-	StackName     string `json:"stackName,omitempty"`
-
+	// Specify the Pulumi stack to select for the update.
+	StackName string `json:"stackName,omitempty"`
+	// Type of the update to perform.
 	Type UpdateType `json:"type,omitempty"`
-
+	// TTL for a completed update object.
+	// +optional
+	TtlAfterCompleted *metav1.Duration `json:"ttlAfterCompleted,omitempty"`
 	// Parallel is the number of resource operations to run in parallel at once
 	// (1 for no parallelism). Defaults to unbounded.
 	// +optional

operator/config/crd/bases/auto.pulumi.com_updates.yaml

@@ -100,6 +100,7 @@ spec:
                   type: string
                 type: array
               stackName:
+                description: Specify the Pulumi stack to select for the update.
                 type: string
               target:
                 description: Specify an exclusive list of resource URNs to update
@@ -111,7 +112,11 @@ spec:
                   TargetDependents allows updating of dependent targets discovered but not
                   specified in the Target list
                 type: boolean
+              ttlAfterCompleted:
+                description: TTL for a completed update object.
+                type: string
               type:
+                description: Type of the update to perform.
                 type: string
               workspaceName:
                 description: WorkspaceName is the workspace to update.

operator/e2e/e2e_test.go

@@ -239,7 +239,6 @@ func dumpLogs(t *testing.T, namespace, name string) {
 	})
 }
 
-
 func dumpEvents(t *testing.T, namespace string) {
 	t.Cleanup(func() {
 		if !t.Failed() {

operator/internal/controller/auto/update_controller.go

@@ -98,6 +98,21 @@ func (r *UpdateReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	rs := newReconcileSession(r.Client, obj)
 
 	if rs.complete.Status == metav1.ConditionTrue {
+		// implement ttl for completed updates
+		if obj.DeletionTimestamp.IsZero() && obj.Spec.TtlAfterCompleted != nil {
+			remainingTtl := rs.complete.LastTransitionTime.Add(obj.Spec.TtlAfterCompleted.Duration).Sub(time.Now())
+			if remainingTtl <= 0 {
+				l.Info("Deleting completed update (ttl has expired)")
+				err = r.Delete(ctx, obj)
+				if err != nil {
+					return ctrl.Result{}, fmt.Errorf("failed to delete completed update: %w", err)
+				}
+				return ctrl.Result{}, nil
+			} else {
+				l.V(1).Info("Retaining completed update", "remainingTtl", remainingTtl)
+				return ctrl.Result{RequeueAfter: remainingTtl}, nil
+			}
+		}
 		l.V(1).Info("Ignoring completed update")
 		return ctrl.Result{}, nil
 	}

operator/internal/controller/auto/update_controller_test.go

@@ -32,9 +32,9 @@ import (
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	utilrand "k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -46,56 +46,85 @@ import (
 	autov1alpha1 "github.com/pulumi/pulumi-kubernetes-operator/v2/operator/api/auto/v1alpha1"
 )
 
-var _ = PDescribe("Update Controller", func() {
-	Context("When reconciling a resource", func() {
-		const resourceName = "test-resource"
+var _ = Describe("Update Controller", func() {
+	var r *UpdateReconciler
+	var objName types.NamespacedName
+	var obj *autov1alpha1.Update
+	// var progressing, complete, failed *metav1.Condition
 
-		ctx := context.Background()
-
-		typeNamespacedName := types.NamespacedName{
-			Name:      resourceName,
-			Namespace: "default", // TODO(user):Modify as needed
+	BeforeEach(func(ctx context.Context) {
+		objName = types.NamespacedName{
+			Name:      fmt.Sprintf("update-%s", utilrand.String(8)),
+			Namespace: "default",
+		}
+		obj = &autov1alpha1.Update{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      objName.Name,
+				Namespace: objName.Namespace,
+			},
 		}
-		update := &autov1alpha1.Update{}
+	})
 
-		BeforeEach(func() {
-			By("creating the custom resource for the Kind Update")
-			err := k8sClient.Get(ctx, typeNamespacedName, update)
-			if err != nil && errors.IsNotFound(err) {
-				resource := &autov1alpha1.Update{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      resourceName,
-						Namespace: "default",
-					},
-					// TODO(user): Specify other spec details if needed.
-				}
-				Expect(k8sClient.Create(ctx, resource)).To(Succeed())
-			}
+	JustBeforeEach(func(ctx context.Context) {
+		Expect(k8sClient.Create(ctx, obj)).To(Succeed())
+		DeferCleanup(func(ctx context.Context) {
+			_ = k8sClient.Delete(ctx, obj)
 		})
+		r = &UpdateReconciler{
+			Client: k8sClient,
+			Scheme: k8sClient.Scheme(),
+		}
+	})
 
-		AfterEach(func() {
-			// TODO(user): Cleanup logic after each test, like removing the resource instance.
-			resource := &autov1alpha1.Update{}
-			err := k8sClient.Get(ctx, typeNamespacedName, resource)
-			Expect(err).NotTo(HaveOccurred())
+	reconcileF := func(ctx context.Context) (result reconcile.Result, err error) {
+		result, err = r.Reconcile(ctx, reconcile.Request{NamespacedName: objName})
 
-			By("Cleanup the specific resource instance Update")
-			Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
-		})
-		It("should successfully reconcile the resource", func() {
-			By("Reconciling the created resource")
-			controllerReconciler := &UpdateReconciler{
-				Client: k8sClient,
-				Scheme: k8sClient.Scheme(),
-			}
+		// refresh the object and find its status condition(s)
+		obj = &autov1alpha1.Update{}
+		_ = k8sClient.Get(ctx, objName, obj)
+
+		// progressing = meta.FindStatusCondition(obj.Status.Conditions, autov1alpha1.UpdateConditionTypeProgressing)
+		// failed = meta.FindStatusCondition(obj.Status.Conditions, autov1alpha1.UpdateConditionTypeFailed)
+		// complete = meta.FindStatusCondition(obj.Status.Conditions, autov1alpha1.UpdateConditionTypeComplete)
+
+		return
+	}
 
-			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
-				NamespacedName: typeNamespacedName,
-			})
-			Expect(err).NotTo(HaveOccurred())
-			// TODO(user): Add more specific assertions depending on your controller's reconciliation logic.
-			// Example: If you expect a certain status condition after reconciliation, verify it here.
+	Describe("Retention", func() {
+		JustBeforeEach(func(ctx context.Context) {
+			// make the update be in a completed state to test the retention logic.
+			completedAt := metav1.NewTime(time.Now().Add(-5 * time.Minute))
+			obj.Status.ObservedGeneration = obj.Generation
+			obj.Status.Conditions = []metav1.Condition{
+				{Type: autov1alpha1.UpdateConditionTypeProgressing, Status: metav1.ConditionFalse, Reason: "Test", LastTransitionTime: completedAt},
+				{Type: autov1alpha1.UpdateConditionTypeComplete, Status: metav1.ConditionTrue, Reason: "Test", LastTransitionTime: completedAt},
+				{Type: autov1alpha1.UpdateConditionTypeFailed, Status: metav1.ConditionFalse, Reason: "Test", LastTransitionTime: completedAt},
+			}
+			Expect(k8sClient.Status().Update(ctx, obj)).To(Succeed())
 		})
+
+		DescribeTableSubtree("TtlAfterCompleted",
+			func(ttl *metav1.Duration, expectDeletion bool) {
+				BeforeEach(func() {
+					obj.Spec.TtlAfterCompleted = ttl
+				})
+				It("reconciles", func(ctx context.Context) {
+					result, err := reconcileF(ctx)
+					Expect(err).NotTo(HaveOccurred())
+					if expectDeletion {
+						Expect(obj.UID).To(BeEmpty())
+					} else {
+						Expect(obj.UID).ToNot(BeEmpty())
+						if ttl != nil {
+							Expect(result.RequeueAfter).ToNot(BeZero())
+						}
+					}
+				})
+			},
+			Entry("TTL is not set", nil, false),
+			Entry("TTL has not expired", &metav1.Duration{Duration: 1 * time.Hour}, false),
+			Entry("TTL has expired", &metav1.Duration{Duration: 1 * time.Minute}, true),
+		)
 	})
 })
 

operator/internal/controller/auto/workspace_controller.go

@@ -504,9 +504,9 @@ func newStatefulSet(ctx context.Context, w *autov1alpha1.Workspace, source *sour
 			Labels:    labels,
 		},
 		Spec: appsv1.StatefulSetSpec{
-			Selector:    &metav1.LabelSelector{MatchLabels: labels},
-			ServiceName: nameForService(w),
-			Replicas:    ptr.To[int32](1),
+			Selector:            &metav1.LabelSelector{MatchLabels: labels},
+			ServiceName:         nameForService(w),
+			Replicas:            ptr.To[int32](1),
 			PodManagementPolicy: appsv1.ParallelPodManagement,
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{

operator/internal/controller/pulumi/stack_controller.go

@@ -80,6 +80,7 @@ const (
 	pulumiFinalizer          = "finalizer.stack.pulumi.com"
 	programRefIndexFieldName = ".spec.programRef.name"      // this is an arbitrary string, named for the field it indexes
 	fluxSourceIndexFieldName = ".spec.fluxSource.sourceRef" // an arbitrary name, named for the field it indexes
+	ttlForCompletedUpdate    = time.Hour * 24
 )
 
 const (
@@ -1507,10 +1508,11 @@ func (sess *stackReconcilerSession) newUp(ctx context.Context, o *pulumiv1.Stack
 			Namespace: sess.namespace,
 		},
 		Spec: autov1alpha1.UpdateSpec{
-			WorkspaceName: sess.ws.Name,
-			StackName:     sess.stack.Stack,
-			Type:          autov1alpha1.UpType,
-			Message:       ptr.To(message),
+			WorkspaceName:     sess.ws.Name,
+			StackName:         sess.stack.Stack,
+			Type:              autov1alpha1.UpType,
+			TtlAfterCompleted: &metav1.Duration{Duration: ttlForCompletedUpdate},
+			Message:           ptr.To(message),
 			// ExpectNoChanges:  ptr.To(o.Spec.ExpectNoRefreshChanges),
 			Target:           o.Spec.Targets,
 			TargetDependents: ptr.To(o.Spec.TargetDependents),
@@ -1537,10 +1539,11 @@ func (sess *stackReconcilerSession) newDestroy(ctx context.Context, o *pulumiv1.
 			Namespace: sess.namespace,
 		},
 		Spec: autov1alpha1.UpdateSpec{
-			WorkspaceName: sess.ws.Name,
-			StackName:     sess.stack.Stack,
-			Type:          autov1alpha1.DestroyType,
-			Message:       ptr.To(message),
+			WorkspaceName:     sess.ws.Name,
+			StackName:         sess.stack.Stack,
+			Type:              autov1alpha1.DestroyType,
+			TtlAfterCompleted: &metav1.Duration{Duration: ttlForCompletedUpdate},
+			Message:           ptr.To(message),
 		},
 	}