From 84d9500aac0ff635fbfa2381054caea9ec9223d1 Mon Sep 17 00:00:00 2001 From: trafalgarzzz Date: Thu, 10 Aug 2023 16:37:53 +0800 Subject: [PATCH 1/4] Clean up orphaned thinruntime resources Signed-off-by: trafalgarzzz --- pkg/ddc/thin/shutdown.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/pkg/ddc/thin/shutdown.go b/pkg/ddc/thin/shutdown.go index 601c4fbf4b6..4aadc7eae16 100644 --- a/pkg/ddc/thin/shutdown.go +++ b/pkg/ddc/thin/shutdown.go @@ -69,7 +69,17 @@ func (t *ThinEngine) destroyMaster() (err error) { if err != nil { return } + return + } + + // When upgrade Fluid to v1.0.0+ from a lower version, there may be some orphaned configmaps when deleting a ThinRuntime if it's created before the upgradation. + // Detect such orphaned configmaps and clean them up. + err = t.cleanUpOrphanedResources() + if err != nil { + t.Log.Info("WARNING: failed to delete orphaned resource, some resources may not be cleaned up in the cluster", "err", err) + err = nil } + return } @@ -251,3 +261,20 @@ func (t *ThinEngine) cleanAll() (err error) { return nil } + +func (t *ThinEngine) cleanUpOrphanedResources() (err error) { + orphanedConfigMapName := fmt.Sprintf("%s-runtimeset", t.name) + cm, err := kubeclient.GetConfigmapByName(t.Client, orphanedConfigMapName, t.namespace) + if err != nil { + return err + } + + if cm != nil { + if err = kubeclient.DeleteConfigMap(t.Client, t.name, t.namespace); err != nil && utils.IgnoreNotFound(err) != nil { + return err + } + t.Log.Info("Found orphaned configmap, successfully deleted it", "configmap", orphanedConfigMapName) + } + + return nil +} From 69904071b8272842369033060eff8bb2a5d46f23 Mon Sep 17 00:00:00 2001 From: trafalgarzzz Date: Thu, 10 Aug 2023 16:39:46 +0800 Subject: [PATCH 2/4] Add owner reference to ThinRuntime resources Signed-off-by: trafalgarzzz --- charts/thin/templates/config/runtime.yaml | 9 +++++++++ .../thin/templates/fuseconfig/fuseconfig.yaml | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/charts/thin/templates/config/runtime.yaml b/charts/thin/templates/config/runtime.yaml index 77233a86253..042f44370ba 100644 --- a/charts/thin/templates/config/runtime.yaml +++ b/charts/thin/templates/config/runtime.yaml @@ -7,6 +7,15 @@ metadata: chart: {{ template "thin.chart" . }} release: {{ .Release.Name }} heritage: {{ .Release.Service }} + ownerReferences: + {{- if .Values.owner.enabled }} + - apiVersion: {{ .Values.owner.apiVersion }} + blockOwnerDeletion: {{ .Values.owner.blockOwnerDeletion }} + controller: {{ .Values.owner.controller }} + kind: {{ .Values.owner.kind }} + name: {{ .Values.owner.name }} + uid: {{ .Values.owner.uid }} + {{- end }} data: runtime.json: | {{ .Values.runtimeValue }} diff --git a/charts/thin/templates/fuseconfig/fuseconfig.yaml b/charts/thin/templates/fuseconfig/fuseconfig.yaml index 034260d8ead..c2423675f90 100644 --- a/charts/thin/templates/fuseconfig/fuseconfig.yaml +++ b/charts/thin/templates/fuseconfig/fuseconfig.yaml @@ -9,6 +9,15 @@ metadata: release: {{ .Release.Name }} heritage: {{ .Release.Service }} role: thin-fuse + ownerReferences: + {{- if .Values.owner.enabled }} + - apiVersion: {{ .Values.owner.apiVersion }} + blockOwnerDeletion: {{ .Values.owner.blockOwnerDeletion }} + controller: {{ .Values.owner.controller }} + kind: {{ .Values.owner.kind }} + name: {{ .Values.owner.name }} + uid: {{ .Values.owner.uid }} + {{- end }} data: config.json: | {{ .Values.fuse.configValue }} @@ -23,6 +32,15 @@ metadata: release: {{ .Release.Name }} heritage: {{ .Release.Service }} role: thin-fuse + ownerReferences: + {{- if .Values.owner.enabled }} + - apiVersion: {{ .Values.owner.apiVersion }} + blockOwnerDeletion: {{ .Values.owner.blockOwnerDeletion }} + controller: {{ .Values.owner.controller }} + kind: {{ .Values.owner.kind }} + name: {{ .Values.owner.name }} + uid: {{ .Values.owner.uid }} + {{- end }} type: Opaque stringData: config.json: | From df4988eb153c54f578248407cb9881431a90c76d Mon Sep 17 00:00:00 2001 From: trafalgarzzz Date: Thu, 10 Aug 2023 17:54:31 +0800 Subject: [PATCH 3/4] Fix unit tests Signed-off-by: trafalgarzzz --- pkg/ddc/thin/shutdown.go | 2 +- pkg/ddc/thin/shutdown_test.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/ddc/thin/shutdown.go b/pkg/ddc/thin/shutdown.go index 4aadc7eae16..ef8b029d487 100644 --- a/pkg/ddc/thin/shutdown.go +++ b/pkg/ddc/thin/shutdown.go @@ -270,7 +270,7 @@ func (t *ThinEngine) cleanUpOrphanedResources() (err error) { } if cm != nil { - if err = kubeclient.DeleteConfigMap(t.Client, t.name, t.namespace); err != nil && utils.IgnoreNotFound(err) != nil { + if err = kubeclient.DeleteConfigMap(t.Client, orphanedConfigMapName, t.namespace); err != nil && utils.IgnoreNotFound(err) != nil { return err } t.Log.Info("Found orphaned configmap, successfully deleted it", "configmap", orphanedConfigMapName) diff --git a/pkg/ddc/thin/shutdown_test.go b/pkg/ddc/thin/shutdown_test.go index 356c508d214..a604c5ed068 100644 --- a/pkg/ddc/thin/shutdown_test.go +++ b/pkg/ddc/thin/shutdown_test.go @@ -233,10 +233,19 @@ func TestThinEngine_destroyMaster(t *testing.T) { } } + orphanedCm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fluid", + Name: "test-runtimeset", + }, + } + client := fake.NewFakeClientWithScheme(testScheme, orphanedCm) + engine := ThinEngine{ name: "test", namespace: "fluid", Log: fake.NullLogger(), + Client: client, runtime: &datav1alpha1.ThinRuntime{ Spec: datav1alpha1.ThinRuntimeSpec{ Fuse: datav1alpha1.ThinFuseSpec{}, @@ -270,6 +279,12 @@ func TestThinEngine_destroyMaster(t *testing.T) { t.Errorf("fail to exec check helm release: %v", err) } + if cm, err := kubeclient.GetConfigmapByName(engine.Client, orphanedCm.Name, orphanedCm.Namespace); err != nil { + t.Errorf("fail to delete orphaned resources: %v", err) + } else if cm != nil { + t.Errorf("orphaned configmap should be cleaned up") + } + // check release error err = gohook.Hook(helm.CheckRelease, mockExecCheckReleaseErr, nil) if err != nil { From 3f2a789d2db1cc2db34171082edfeaf0fa1968ce Mon Sep 17 00:00:00 2001 From: trafalgarzzz Date: Thu, 10 Aug 2023 18:17:17 +0800 Subject: [PATCH 4/4] Refactor code Signed-off-by: trafalgarzzz --- pkg/ddc/thin/shutdown.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/ddc/thin/shutdown.go b/pkg/ddc/thin/shutdown.go index ef8b029d487..fff5a364280 100644 --- a/pkg/ddc/thin/shutdown.go +++ b/pkg/ddc/thin/shutdown.go @@ -69,15 +69,14 @@ func (t *ThinEngine) destroyMaster() (err error) { if err != nil { return } - return - } - - // When upgrade Fluid to v1.0.0+ from a lower version, there may be some orphaned configmaps when deleting a ThinRuntime if it's created before the upgradation. - // Detect such orphaned configmaps and clean them up. - err = t.cleanUpOrphanedResources() - if err != nil { - t.Log.Info("WARNING: failed to delete orphaned resource, some resources may not be cleaned up in the cluster", "err", err) - err = nil + } else { + // When upgrade Fluid to v1.0.0+ from a lower version, there may be some orphaned configmaps when deleting a ThinRuntime if it's created before the upgradation. + // Detect such orphaned configmaps and clean them up. + err = t.cleanUpOrphanedResources() + if err != nil { + t.Log.Info("WARNING: failed to delete orphaned resource, some resources may not be cleaned up in the cluster", "err", err) + err = nil + } } return