From 73f22f6613a452b9ed267188d72da341c32e1472 Mon Sep 17 00:00:00 2001
From: Jian Wang <w13915984028@gmail.com>
Date: Mon, 15 Apr 2024 11:26:11 +0200
Subject: [PATCH] Avoid picking old helm-delete job

Signed-off-by: Jian Wang <w13915984028@gmail.com>
---
 pkg/controllers/chart/chart.go | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/pkg/controllers/chart/chart.go b/pkg/controllers/chart/chart.go
index d82f3de5..9bcaae34 100644
--- a/pkg/controllers/chart/chart.go
+++ b/pkg/controllers/chart/chart.go
@@ -226,11 +226,31 @@ func (c *Controller) OnRemove(key string, chart *v1.HelmChart) (*v1.HelmChart, e
 		return nil, nil
 	}
 
+	if chart.DeletionTimestamp != nil {
+		return nil, nil
+	}
+
 	expectedJob, objs, err := c.getJobAndRelatedResources(chart)
 	if err != nil {
 		return nil, err
 	}
 
+	// remove old helm-delete job if it was left
+	job, err := c.jobCache.Get(chart.Namespace, expectedJob.Name)
+	if err == nil && job.CreationTimestamp.Before(chart.DeletionTimestamp) {
+		err = c.jobs.Delete(chart.Namespace, expectedJob.Name, &metav1.DeleteOptions{PropagationPolicy: &deletePolicy})
+		if err != nil {
+			if !apierrors.IsNotFound(err) {
+				return nil, fmt.Errorf("fail to delete old helm-delete job %w", err)
+			}
+			// if IsNotFound, continue
+		} else {
+			// wait old job to be removed
+			c.helms.EnqueueAfter(chart.Namespace, chart.Name, 1*time.Second)
+			return nil, nil
+		}
+	}
+
 	// note: on the logic of running an apply here...
 	// if the uninstall job does not exist, it will create it
 	// if the job already exists and it is uninstalling, nothing will change since there's no need to patch
@@ -251,7 +271,7 @@ func (c *Controller) OnRemove(key string, chart *v1.HelmChart) (*v1.HelmChart, e
 	time.Sleep(3 * time.Second)
 
 	// once we have run the above logic, we can now check if the job is complete
-	job, err := c.jobCache.Get(chart.Namespace, expectedJob.Name)
+	job, err = c.jobCache.Get(chart.Namespace, expectedJob.Name)
 	if apierrors.IsNotFound(err) {
 		// the above apply should have created it, something is wrong.
 		// if you are here, there must be a bug in the code.
@@ -269,7 +289,7 @@ func (c *Controller) OnRemove(key string, chart *v1.HelmChart) (*v1.HelmChart, e
 		chartCopy.Status.JobName = job.Name
 		newChart, err := c.helms.Update(chartCopy)
 		if err != nil {
-			return chart, fmt.Errorf("unable to update status of helm chart to add uninstall job name %s", chartCopy.Status.JobName)
+			return chart, fmt.Errorf("unable to update status of helm chart to add uninstall job name %s, %w", chartCopy.Status.JobName, err)
 		}
 		return newChart, fmt.Errorf("waiting for delete of helm chart for %s by %s", key, job.Name)
 	}
@@ -285,7 +305,7 @@ func (c *Controller) OnRemove(key string, chart *v1.HelmChart) (*v1.HelmChart, e
 		WithSetID("helm-chart-registration").
 		ApplyObjects()
 	if err != nil {
-		return nil, fmt.Errorf("unable to remove resources tied to HelmChart %s/%s: %s", chart.Namespace, chart.Name, err)
+		return nil, fmt.Errorf("unable to remove resources tied to HelmChart %s/%s: %w", chart.Namespace, chart.Name, err)
 	}
 
 	return chart, nil