Skip to content

Commit 73f22f6

Browse files
committed
Avoid picking old helm-delete job
Signed-off-by: Jian Wang <w13915984028@gmail.com>
1 parent 255f905 commit 73f22f6

1 file changed

Lines changed: 23 additions & 3 deletions

File tree

pkg/controllers/chart/chart.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,31 @@ func (c *Controller) OnRemove(key string, chart *v1.HelmChart) (*v1.HelmChart, e
226226
return nil, nil
227227
}
228228

229+
if chart.DeletionTimestamp != nil {
230+
return nil, nil
231+
}
232+
229233
expectedJob, objs, err := c.getJobAndRelatedResources(chart)
230234
if err != nil {
231235
return nil, err
232236
}
233237

238+
// remove old helm-delete job if it was left
239+
job, err := c.jobCache.Get(chart.Namespace, expectedJob.Name)
240+
if err == nil && job.CreationTimestamp.Before(chart.DeletionTimestamp) {
241+
err = c.jobs.Delete(chart.Namespace, expectedJob.Name, &metav1.DeleteOptions{PropagationPolicy: &deletePolicy})
242+
if err != nil {
243+
if !apierrors.IsNotFound(err) {
244+
return nil, fmt.Errorf("fail to delete old helm-delete job %w", err)
245+
}
246+
// if IsNotFound, continue
247+
} else {
248+
// wait old job to be removed
249+
c.helms.EnqueueAfter(chart.Namespace, chart.Name, 1*time.Second)
250+
return nil, nil
251+
}
252+
}
253+
234254
// note: on the logic of running an apply here...
235255
// if the uninstall job does not exist, it will create it
236256
// 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
251271
time.Sleep(3 * time.Second)
252272

253273
// once we have run the above logic, we can now check if the job is complete
254-
job, err := c.jobCache.Get(chart.Namespace, expectedJob.Name)
274+
job, err = c.jobCache.Get(chart.Namespace, expectedJob.Name)
255275
if apierrors.IsNotFound(err) {
256276
// the above apply should have created it, something is wrong.
257277
// 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
269289
chartCopy.Status.JobName = job.Name
270290
newChart, err := c.helms.Update(chartCopy)
271291
if err != nil {
272-
return chart, fmt.Errorf("unable to update status of helm chart to add uninstall job name %s", chartCopy.Status.JobName)
292+
return chart, fmt.Errorf("unable to update status of helm chart to add uninstall job name %s, %w", chartCopy.Status.JobName, err)
273293
}
274294
return newChart, fmt.Errorf("waiting for delete of helm chart for %s by %s", key, job.Name)
275295
}
@@ -285,7 +305,7 @@ func (c *Controller) OnRemove(key string, chart *v1.HelmChart) (*v1.HelmChart, e
285305
WithSetID("helm-chart-registration").
286306
ApplyObjects()
287307
if err != nil {
288-
return nil, fmt.Errorf("unable to remove resources tied to HelmChart %s/%s: %s", chart.Namespace, chart.Name, err)
308+
return nil, fmt.Errorf("unable to remove resources tied to HelmChart %s/%s: %w", chart.Namespace, chart.Name, err)
289309
}
290310

291311
return chart, nil

0 commit comments

Comments
 (0)