From ac90ec8482850051f75d05d7aa1982e72082d3e8 Mon Sep 17 00:00:00 2001 From: gurjotkaur20 Date: Fri, 19 May 2023 23:16:37 +0530 Subject: [PATCH 1/4] fix: delay removal of orphanPVC to avoid race condition --- controllers/druid/handler.go | 65 +++++++++++++++++++++++++++++++++--- controllers/druid/util.go | 10 ++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/controllers/druid/handler.go b/controllers/druid/handler.go index d81d037b..0a63918d 100644 --- a/controllers/druid/handler.go +++ b/controllers/druid/handler.go @@ -9,6 +9,8 @@ import ( "fmt" "regexp" "sort" + "strconv" + "time" autoscalev2 "k8s.io/api/autoscaling/v2" networkingv1 "k8s.io/api/networking/v1" @@ -490,12 +492,65 @@ func deleteOrphanPVC(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid for i, pvc := range pvcList { if !ContainsString(mountedPVC, pvc.GetName()) { - err := writers.Delete(ctx, sdk, drd, pvcList[i], emitEvents, &client.DeleteAllOfOptions{}) - if err != nil { - return err + + if _, ok := pvc.GetLabels()["toBeDeleted"]; ok { + deletionTS := pvc.GetLabels()["deletionTimestamp"] + + deletionTimestamp, err := strconv.ParseInt(deletionTS, 10, 64) + + if err != nil { + msg := fmt.Sprintf("Unable to parse label deletionTimestamp [%s:%s]", deletionTS, pvcList[i].GetName()) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + return err + } + + timeNow := time.Now().Unix() + timeDiff := timeDifference(deletionTimestamp, timeNow) + + if timeDiff >= int64(time.Second/time.Second)*60 { + // delete pvc + err = writers.Delete(ctx, sdk, drd, pvcList[i], emitEvents, &client.DeleteAllOfOptions{}) + if err != nil { + return err + } else { + msg := fmt.Sprintf("Deleted orphaned pvc [%s:%s] successfully", pvcList[i].GetName(), drd.Namespace) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + } + } else { + // wait for 60s + msg := fmt.Sprintf("pvc [%s:%s] mark to be deleted after %ds", pvcList[i].GetName(), drd.Namespace, timeDiff) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + } } else { - msg := fmt.Sprintf("Deleted orphaned pvc [%s:%s] successfully", pvcList[i].GetName(), drd.Namespace) - logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + // set labels when pvc comes for deletion for the deletion, then wait for 60s to delete it + getPvcLabels := pvc.GetLabels() + getPvcLabels["toBeDeleted"] = "yes" + getPvcLabels["deletionTimestamp"] = strconv.FormatInt(time.Now().Unix(), 10) + pvc.SetLabels(getPvcLabels) + + _, err = writers.Update(ctx, sdk, drd, pvc, emitEvents) + if err != nil { + return err + } else { + msg := fmt.Sprintf("pvc mark to be deleted, added labels %s and %s successfully [%s]", "toBeDeleted", "deletionTimestamp", pvc.GetName()) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + } + } + } else { + // do not delete pvc + if _, ok := pvc.GetLabels()["toBeDeleted"]; ok { + getPvcLabels := pvc.GetLabels() + delete(getPvcLabels, "toBeDeleted") + delete(getPvcLabels, "deletionTimestamp") + pvc.SetLabels(getPvcLabels) + + _, err = writers.Update(ctx, sdk, drd, pvc, emitEvents) + if err != nil { + return err + } else { + msg := fmt.Sprintf("Deleted labels %s and %s successfully in pvc [%s]", "toBeDeleted", "deletionTimestamp", pvc.GetName()) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + } } } } diff --git a/controllers/druid/util.go b/controllers/druid/util.go index b30c2e48..4e779719 100644 --- a/controllers/druid/util.go +++ b/controllers/druid/util.go @@ -5,6 +5,7 @@ import ( "reflect" "strconv" "strings" + "time" ) func firstNonEmptyStr(s1 string, s2 string) string { @@ -77,3 +78,12 @@ func Str2Int(s string) int { } return i } + +// to find the time difference between two epoch times +func timeDifference(epochTime1, epochTime2 int64) int64 { + t1 := time.Unix(epochTime1, 0) + t2 := time.Unix(epochTime2, 0) + + diff := time.Duration(t2.Sub(t1)) + return int64(diff.Seconds()) +} From ba70b59662e408467c3fcb9fdce71479eb705c1a Mon Sep 17 00:00:00 2001 From: gurjotkaur20 Date: Fri, 19 May 2023 23:44:23 +0530 Subject: [PATCH 2/4] fix: updated log messages --- controllers/druid/handler.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/druid/handler.go b/controllers/druid/handler.go index 0a63918d..81a37e6a 100644 --- a/controllers/druid/handler.go +++ b/controllers/druid/handler.go @@ -518,11 +518,11 @@ func deleteOrphanPVC(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid } } else { // wait for 60s - msg := fmt.Sprintf("pvc [%s:%s] mark to be deleted after %ds", pvcList[i].GetName(), drd.Namespace, timeDiff) + msg := fmt.Sprintf("pvc [%s:%s] marked to be deleted after %ds", pvcList[i].GetName(), drd.Namespace, timeDiff) logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) } } else { - // set labels when pvc comes for deletion for the deletion, then wait for 60s to delete it + // set labels when pvc comes for deletion for the first time getPvcLabels := pvc.GetLabels() getPvcLabels["toBeDeleted"] = "yes" getPvcLabels["deletionTimestamp"] = strconv.FormatInt(time.Now().Unix(), 10) @@ -532,7 +532,7 @@ func deleteOrphanPVC(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid if err != nil { return err } else { - msg := fmt.Sprintf("pvc mark to be deleted, added labels %s and %s successfully [%s]", "toBeDeleted", "deletionTimestamp", pvc.GetName()) + msg := fmt.Sprintf("marked pvc for deletion , added labels %s and %s successfully [%s]", "toBeDeleted", "deletionTimestamp", pvc.GetName()) logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) } } @@ -548,7 +548,7 @@ func deleteOrphanPVC(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid if err != nil { return err } else { - msg := fmt.Sprintf("Deleted labels %s and %s successfully in pvc [%s]", "toBeDeleted", "deletionTimestamp", pvc.GetName()) + msg := fmt.Sprintf("unmarked pvc for deletion, removed labels %s and %s successfully in pvc [%s]", "toBeDeleted", "deletionTimestamp", pvc.GetName()) logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) } } From 721255238ac97d5e7e1e22e2ff0dd05128191f37 Mon Sep 17 00:00:00 2001 From: gurjotkaur20 Date: Sun, 11 Jun 2023 13:34:53 +0530 Subject: [PATCH 3/4] fix: deletionTimeStamp label name changed and refactoring deleteOrphanPVC --- controllers/druid/handler.go | 100 ++++++++++++++++++++++------------- 1 file changed, 62 insertions(+), 38 deletions(-) diff --git a/controllers/druid/handler.go b/controllers/druid/handler.go index 81a37e6a..bb29e7d5 100644 --- a/controllers/druid/handler.go +++ b/controllers/druid/handler.go @@ -33,6 +33,8 @@ import ( const ( druidOpResourceHash = "druidOpResourceHash" defaultCommonConfigMountPath = "/druid/conf/druid/_common" + toBeDeletedLabel = "toBeDeleted" + deletionTSLabel = "deletionTS" ) var logger = logf.Log.WithName("druid_operator_handler") @@ -493,63 +495,36 @@ func deleteOrphanPVC(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid if !ContainsString(mountedPVC, pvc.GetName()) { - if _, ok := pvc.GetLabels()["toBeDeleted"]; ok { - deletionTS := pvc.GetLabels()["deletionTimestamp"] - - deletionTimestamp, err := strconv.ParseInt(deletionTS, 10, 64) - + if _, ok := pvc.GetLabels()[toBeDeletedLabel]; ok { + err := checkPVCLabelsAndDelete(ctx, sdk, drd, emitEvents, pvcList[i]) if err != nil { - msg := fmt.Sprintf("Unable to parse label deletionTimestamp [%s:%s]", deletionTS, pvcList[i].GetName()) - logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) return err } - - timeNow := time.Now().Unix() - timeDiff := timeDifference(deletionTimestamp, timeNow) - - if timeDiff >= int64(time.Second/time.Second)*60 { - // delete pvc - err = writers.Delete(ctx, sdk, drd, pvcList[i], emitEvents, &client.DeleteAllOfOptions{}) - if err != nil { - return err - } else { - msg := fmt.Sprintf("Deleted orphaned pvc [%s:%s] successfully", pvcList[i].GetName(), drd.Namespace) - logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) - } - } else { - // wait for 60s - msg := fmt.Sprintf("pvc [%s:%s] marked to be deleted after %ds", pvcList[i].GetName(), drd.Namespace, timeDiff) - logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) - } } else { // set labels when pvc comes for deletion for the first time getPvcLabels := pvc.GetLabels() - getPvcLabels["toBeDeleted"] = "yes" - getPvcLabels["deletionTimestamp"] = strconv.FormatInt(time.Now().Unix(), 10) - pvc.SetLabels(getPvcLabels) + getPvcLabels[toBeDeletedLabel] = "yes" + getPvcLabels[deletionTSLabel] = strconv.FormatInt(time.Now().Unix(), 10) - _, err = writers.Update(ctx, sdk, drd, pvc, emitEvents) + err = setPVCLabels(ctx, sdk, drd, emitEvents, pvcList[i], getPvcLabels, true) if err != nil { return err } else { - msg := fmt.Sprintf("marked pvc for deletion , added labels %s and %s successfully [%s]", "toBeDeleted", "deletionTimestamp", pvc.GetName()) - logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + // do nothing } } } else { // do not delete pvc - if _, ok := pvc.GetLabels()["toBeDeleted"]; ok { + if _, ok := pvc.GetLabels()[toBeDeletedLabel]; ok { getPvcLabels := pvc.GetLabels() - delete(getPvcLabels, "toBeDeleted") - delete(getPvcLabels, "deletionTimestamp") - pvc.SetLabels(getPvcLabels) + delete(getPvcLabels, toBeDeletedLabel) + delete(getPvcLabels, deletionTSLabel) - _, err = writers.Update(ctx, sdk, drd, pvc, emitEvents) + err = setPVCLabels(ctx, sdk, drd, emitEvents, pvcList[i], getPvcLabels, false) if err != nil { return err } else { - msg := fmt.Sprintf("unmarked pvc for deletion, removed labels %s and %s successfully in pvc [%s]", "toBeDeleted", "deletionTimestamp", pvc.GetName()) - logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + // do nothing } } } @@ -558,6 +533,55 @@ func deleteOrphanPVC(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid return nil } +func checkPVCLabelsAndDelete(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid, emitEvents EventEmitter, pvc object) error { + deletionTS := pvc.GetLabels()[deletionTSLabel] + + parsedDeletionTS, err := strconv.ParseInt(deletionTS, 10, 64) + + if err != nil { + msg := fmt.Sprintf("Unable to parse label %s [%s:%s]", deletionTSLabel, deletionTS, pvc.GetName()) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + return err + } + + timeNow := time.Now().Unix() + timeDiff := timeDifference(parsedDeletionTS, timeNow) + + if timeDiff >= int64(time.Second/time.Second)*60 { + // delete pvc + err = writers.Delete(ctx, sdk, drd, pvc, emitEvents, &client.DeleteAllOfOptions{}) + if err != nil { + return err + } else { + msg := fmt.Sprintf("Deleted orphaned pvc [%s:%s] successfully", pvc.GetName(), drd.Namespace) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + } + } else { + // wait for 60s + msg := fmt.Sprintf("pvc [%s:%s] marked to be deleted after %ds", pvc.GetName(), drd.Namespace, 60-timeDiff) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + } + return nil +} + +func setPVCLabels(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid, emitEvents EventEmitter, pvc object, labels map[string]string, isSetLabel bool) error { + + pvc.SetLabels(labels) + _, err := writers.Update(ctx, sdk, drd, pvc, emitEvents) + if err != nil { + return err + } else { + if isSetLabel { + msg := fmt.Sprintf("marked pvc for deletion , added labels %s and %s successfully [%s]", toBeDeletedLabel, deletionTSLabel, pvc.GetName()) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + } else { + msg := fmt.Sprintf("unmarked pvc for deletion, removed labels %s and %s successfully in pvc [%s]", toBeDeletedLabel, deletionTSLabel, pvc.GetName()) + logger.Info(msg, "name", drd.Name, "namespace", drd.Namespace) + } + } + return nil +} + func executeFinalizers(ctx context.Context, sdk client.Client, m *v1alpha1.Druid, emitEvents EventEmitter) error { if ContainsString(m.ObjectMeta.Finalizers, finalizerName) { From 6d715fd87c638e7aaab2337ca5439f40824b4ebc Mon Sep 17 00:00:00 2001 From: gurjotkaur20 Date: Wed, 21 Jun 2023 16:06:17 +0530 Subject: [PATCH 4/4] fix: removed else --- controllers/druid/handler.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/controllers/druid/handler.go b/controllers/druid/handler.go index bb29e7d5..6baf0b37 100644 --- a/controllers/druid/handler.go +++ b/controllers/druid/handler.go @@ -509,8 +509,6 @@ func deleteOrphanPVC(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid err = setPVCLabels(ctx, sdk, drd, emitEvents, pvcList[i], getPvcLabels, true) if err != nil { return err - } else { - // do nothing } } } else { @@ -523,8 +521,6 @@ func deleteOrphanPVC(ctx context.Context, sdk client.Client, drd *v1alpha1.Druid err = setPVCLabels(ctx, sdk, drd, emitEvents, pvcList[i], getPvcLabels, false) if err != nil { return err - } else { - // do nothing } } }