Merge pull request #146 from syntasso/145-delete-static-deps

fix(#145): delete work for static deps
syntasso · May 29, 2024 · 082105c · 082105c
2 parents 398166c + 0682072
commit 082105c
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 29 deletions.
diff --git a/controllers/promise_controller.go b/controllers/promise_controller.go
@@ -353,13 +353,21 @@ func (r *PromiseReconciler) generateStatusAndMarkRequirements(ctx context.Contex
 }
 
 func (r *PromiseReconciler) reconcileDependencies(o opts, promise *v1alpha1.Promise, configurePipeline []v1alpha1.Pipeline) (*ctrl.Result, error) {
-	o.logger.Info("Applying static dependencies for Promise", "promise", promise.GetName())
 	if len(promise.Spec.Dependencies) > 0 {
-		if err := r.applyWorkForDependencies(o, promise); err != nil {
+		o.logger.Info("Applying static dependencies for Promise", "promise", promise.GetName())
+		if err := r.applyWorkForStaticDependencies(o, promise); err != nil {
 			o.logger.Error(err, "Error creating Works")
 			return nil, err
 		}
 	}
+
+	if len(promise.Spec.Dependencies) == 0 {
+		err := r.deleteWorkForStaticDependencies(o, promise)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	if len(configurePipeline) == 0 {
 		return nil, nil
 	}
@@ -990,7 +998,7 @@ func setStatusFieldsOnCRD(rrCRD *apiextensionsv1.CustomResourceDefinition) {
 	}
 }
 
-func (r *PromiseReconciler) applyWorkForDependencies(o opts, promise *v1alpha1.Promise) error {
+func (r *PromiseReconciler) applyWorkForStaticDependencies(o opts, promise *v1alpha1.Promise) error {
 	name := resourceutil.GenerateObjectName(promise.GetName() + "-static-deps")
 	work, err := v1alpha1.NewPromiseDependenciesWork(promise, name)
 	if err != nil {
@@ -1021,6 +1029,20 @@ func (r *PromiseReconciler) applyWorkForDependencies(o opts, promise *v1alpha1.P
 	return nil
 }
 
+func (r *PromiseReconciler) deleteWorkForStaticDependencies(o opts, promise *v1alpha1.Promise) error {
+	existingWork, err := resourceutil.GetWorkForStaticDependencies(r.Client, v1alpha1.SystemNamespace, promise.GetName())
+	if err != nil {
+		return err
+	}
+
+	if existingWork == nil {
+		return nil
+	}
+
+	o.logger.Info("deleting work for static dependencies", "namespace", existingWork.GetNamespace(), "name", existingWork.GetName())
+	return r.Client.Delete(o.ctx, existingWork)
+}
+
 func (r *PromiseReconciler) markRequiredPromiseAsRequired(ctx context.Context, version string, promise, requiredPromise *v1alpha1.Promise) {
 	requiredBy := v1alpha1.RequiredBy{
 		Promise: v1alpha1.PromiseSummary{

diff --git a/controllers/promise_controller_test.go b/controllers/promise_controller_test.go
@@ -194,6 +194,7 @@ var _ = Describe("PromiseController", func() {
 					})
 				})
 			})
+
 			When("the promise has requirements", func() {
 				BeforeEach(func() {
 					promise = promiseFromFile(promiseWithRequirements)
@@ -728,36 +729,36 @@ var _ = Describe("PromiseController", func() {
 		})
 
 		When("the promise is being updated", func() {
-			When("it contains static dependencies", func() {
-				BeforeEach(func() {
-					promise = promiseFromFile(promiseWithOnlyDepsPath)
-					promiseName = types.NamespacedName{
-						Name:      promise.GetName(),
-						Namespace: promise.GetNamespace(),
-					}
-					promiseCommonLabels = map[string]string{
-						"kratix-promise-id":      promise.GetName(),
-						"kratix.io/promise-name": promise.GetName(),
-					}
-					promiseResourcesName = types.NamespacedName{
-						Name:      promise.GetName() + "-promise-pipeline",
-						Namespace: "kratix-platform-system",
-					}
+			BeforeEach(func() {
+				promise = promiseFromFile(promiseWithOnlyDepsPath)
+				promiseName = types.NamespacedName{
+					Name:      promise.GetName(),
+					Namespace: promise.GetNamespace(),
+				}
+				promiseCommonLabels = map[string]string{
+					"kratix-promise-id":      promise.GetName(),
+					"kratix.io/promise-name": promise.GetName(),
+				}
+				promiseResourcesName = types.NamespacedName{
+					Name:      promise.GetName() + "-promise-pipeline",
+					Namespace: "kratix-platform-system",
+				}
 
-					Expect(fakeK8sClient.Create(ctx, promise)).To(Succeed())
-					Expect(fakeK8sClient.Get(ctx, promiseName, promise)).To(Succeed())
-					promise.UID = "1234abcd"
-					Expect(fakeK8sClient.Update(ctx, promise)).To(Succeed())
+				Expect(fakeK8sClient.Create(ctx, promise)).To(Succeed())
+				Expect(fakeK8sClient.Get(ctx, promiseName, promise)).To(Succeed())
+				promise.UID = "1234abcd"
+				Expect(fakeK8sClient.Update(ctx, promise)).To(Succeed())
 
-					setReconcileConfigureWorkflowToReturnFinished()
-					result, err := t.reconcileUntilCompletion(reconciler, promise, &opts{
-						funcs: []func(client.Object) error{autoMarkCRDAsEstablished},
-					})
-					Expect(err).NotTo(HaveOccurred())
-					Expect(result).To(Equal(ctrl.Result{}))
+				setReconcileConfigureWorkflowToReturnFinished()
+				result, err := t.reconcileUntilCompletion(reconciler, promise, &opts{
+					funcs: []func(client.Object) error{autoMarkCRDAsEstablished},
 				})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result).To(Equal(ctrl.Result{}))
+			})
 
-				It("re-reconciles until completetion", func() {
+			When("it contains static dependencies", func() {
+				It("re-reconciles until completion", func() {
 					Expect(fakeK8sClient.Get(ctx, promiseName, promise)).To(Succeed())
 					updatedPromise := promiseFromFile(promiseWithOnlyDepsUpdatedPath)
 					promise.Spec = updatedPromise.Spec
@@ -789,6 +790,26 @@ var _ = Describe("PromiseController", func() {
 				})
 			})
 
+			When("the static dependencies are removed", func() {
+				It("re-reconciles until completion", func() {
+					Expect(fakeK8sClient.Get(ctx, promiseName, promise)).To(Succeed())
+					promise.Spec.Dependencies = nil
+					Expect(fakeK8sClient.Update(ctx, promise)).To(Succeed())
+
+					setReconcileConfigureWorkflowToReturnFinished()
+					result, err := t.reconcileUntilCompletion(reconciler, promise, &opts{
+						funcs: []func(client.Object) error{autoMarkCRDAsEstablished},
+					})
+					Expect(err).NotTo(HaveOccurred())
+					Expect(result).To(Equal(ctrl.Result{}))
+
+					By("deleting the work", func() {
+						works := &v1alpha1.WorkList{}
+						Expect(fakeK8sClient.List(ctx, works)).To(Succeed())
+						Expect(works.Items).To(HaveLen(0))
+					})
+				})
+			})
 		})
 	})