From 06cdab56f651042418e9f90d5f159fcb889d16e0 Mon Sep 17 00:00:00 2001 From: LiHui Date: Tue, 10 Aug 2021 11:17:51 +0800 Subject: [PATCH] Fix typo && Add comments Signed-off-by: LiHui --- pkg/constants/constants.go | 7 +++-- .../helm_application_controller.go | 28 +++++++++++-------- .../helm_application_controller_test.go | 2 +- .../workspacetemplate_controller.go | 7 +++-- pkg/models/openpitrix/utils.go | 2 +- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index a273f03dd..903a77355 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -41,7 +41,7 @@ const ( ChartApplicationIdLabelKey = "application.kubesphere.io/app-id" ChartApplicationVersionIdLabelKey = "application.kubesphere.io/app-version-id" CategoryIdLabelKey = "application.kubesphere.io/app-category-id" - DangingAppCleanupKey = "application.kubesphere.io/app-cleanup" + DanglingAppCleanupKey = "application.kubesphere.io/app-cleanup" CreatorAnnotationKey = "kubesphere.io/creator" UsernameLabelKey = "kubesphere.io/username" DevOpsProjectLabelKey = "kubesphere.io/devopsproject" @@ -71,8 +71,9 @@ const ( OpenpitrixAttachmentTag = "Attachment" OpenpitrixRepositoryTag = "Repository" OpenpitrixManagementTag = "App Management" - CleanupDangingAppOngoing = "ongoing" - CleanupDangingAppDone = "done" + + CleanupDanglingAppOngoing = "ongoing" + CleanupDanglingAppDone = "done" DevOpsCredentialTag = "DevOps Credential" DevOpsPipelineTag = "DevOps Pipeline" diff --git a/pkg/controller/openpitrix/helmapplication/helm_application_controller.go b/pkg/controller/openpitrix/helmapplication/helm_application_controller.go index 95020d166..e9bb5c0c3 100644 --- a/pkg/controller/openpitrix/helmapplication/helm_application_controller.go +++ b/pkg/controller/openpitrix/helmapplication/helm_application_controller.go @@ -82,7 +82,7 @@ func (r *ReconcileHelmApplication) Reconcile(request reconcile.Request) (reconci if !inAppStore(app) { // The workspace of this app is being deleting, clean up this app - if err := r.cleanupDangingApp(context.TODO(), app); err != nil { + if err := r.cleanupDanglingApp(context.TODO(), app); err != nil { return reconcile.Result{}, err } @@ -189,10 +189,19 @@ func inAppStore(app *v1alpha1.HelmApplication) bool { return strings.HasSuffix(app.Name, v1alpha1.HelmApplicationAppStoreSuffix) } -// cleanupDangingApp delete the app when it is not active and not suspended, -// sets the workspace label to empty and remove parts of the appversion when app state are active or suspended -func (r *ReconcileHelmApplication) cleanupDangingApp(ctx context.Context, app *v1alpha1.HelmApplication) error { - if app.Annotations[constants.DangingAppCleanupKey] == constants.CleanupDangingAppOngoing { +// cleanupDanglingApp deletes the app when it is not active and not suspended, +// sets the workspace label to empty and remove parts of the appversion when app state is active or suspended. +// +// When one workspace is being deleting, we can delete all the app which are not active or suspended of this workspace, +// but when an app has been promoted to app store, we have to deal with it specially. +// If we just delete that app, then this app will be deleted from app store too. +// If we leave it alone, and user creates a workspace with the same name sometime, +// then this app will appear in this new workspace which confuses the user. +// So we need to delete all the appversion which are not active or suspended first, +// then remove the workspace label from the app. And on the console of ks, we will show something +// like "(workspace deleted)" to user for this app. +func (r *ReconcileHelmApplication) cleanupDanglingApp(ctx context.Context, app *v1alpha1.HelmApplication) error { + if app.Annotations != nil && app.Annotations[constants.DanglingAppCleanupKey] == constants.CleanupDanglingAppOngoing { // Just delete the app when the state is not active or not suspended. if app.Status.State != v1alpha1.StateActive && app.Status.State != v1alpha1.StateSuspended { err := r.Delete(ctx, app) @@ -224,7 +233,7 @@ func (r *ReconcileHelmApplication) cleanupDangingApp(ctx context.Context, app *v } } - // Marks the app that the workspace to which it belongs has been deleted. + // Mark the app that the workspace to which it belongs has been deleted. var appInStore v1alpha1.HelmApplication err = r.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s%s", app.GetHelmApplicationId(), v1alpha1.HelmApplicationAppStoreSuffix)}, &appInStore) @@ -237,7 +246,7 @@ func (r *ReconcileHelmApplication) cleanupDangingApp(ctx context.Context, app *v if appCopy.Annotations == nil { appCopy.Annotations = map[string]string{} } - appCopy.Annotations[constants.DangingAppCleanupKey] = constants.CleanupDangingAppDone + appCopy.Annotations[constants.DanglingAppCleanupKey] = constants.CleanupDanglingAppDone patchedApp := client.MergeFrom(&appInStore) err = r.Patch(ctx, appCopy, patchedApp) @@ -248,10 +257,7 @@ func (r *ReconcileHelmApplication) cleanupDangingApp(ctx context.Context, app *v } appCopy := app.DeepCopy() - if appCopy.Annotations == nil { - appCopy.Annotations = map[string]string{} - } - appCopy.Annotations[constants.DangingAppCleanupKey] = constants.CleanupDangingAppDone + appCopy.Annotations[constants.DanglingAppCleanupKey] = constants.CleanupDanglingAppDone // Remove the workspace label, or if user creates a workspace with the same name, this app will show in the new workspace. if appCopy.Labels == nil { appCopy.Labels = map[string]string{} diff --git a/pkg/controller/openpitrix/helmapplication/helm_application_controller_test.go b/pkg/controller/openpitrix/helmapplication/helm_application_controller_test.go index 2faa78484..bea3a216a 100644 --- a/pkg/controller/openpitrix/helmapplication/helm_application_controller_test.go +++ b/pkg/controller/openpitrix/helmapplication/helm_application_controller_test.go @@ -86,7 +86,7 @@ var _ = Describe("helmApplication", func() { } appCopy := localApp.DeepCopy() appCopy.Annotations = map[string]string{} - appCopy.Annotations[constants.DangingAppCleanupKey] = constants.CleanupDangingAppOngoing + appCopy.Annotations[constants.DanglingAppCleanupKey] = constants.CleanupDanglingAppOngoing patchData := client.MergeFrom(&localApp) err = k8sClient.Patch(context.Background(), appCopy, patchData) return err == nil diff --git a/pkg/controller/workspacetemplate/workspacetemplate_controller.go b/pkg/controller/workspacetemplate/workspacetemplate_controller.go index 6cb11cddf..fca707490 100644 --- a/pkg/controller/workspacetemplate/workspacetemplate_controller.go +++ b/pkg/controller/workspacetemplate/workspacetemplate_controller.go @@ -425,10 +425,13 @@ func (r *Reconciler) deleteHelmApps(ctx context.Context, ws string) error { return err } for _, app := range apps.Items { - if _, exists := app.Annotations[constants.DangingAppCleanupKey]; !exists { + if app.Annotations == nil { + app.Annotations = map[string]string{} + } + if _, exists := app.Annotations[constants.DanglingAppCleanupKey]; !exists { // Mark the app, the cleanup is in the application controller. appCopy := app.DeepCopy() - appCopy.Annotations[constants.DangingAppCleanupKey] = constants.CleanupDangingAppOngoing + appCopy.Annotations[constants.DanglingAppCleanupKey] = constants.CleanupDanglingAppOngoing appPatch := client.MergeFrom(&app) err = r.Patch(ctx, appCopy, appPatch) if err != nil { diff --git a/pkg/models/openpitrix/utils.go b/pkg/models/openpitrix/utils.go index 051b914e9..0ea7ba7ae 100644 --- a/pkg/models/openpitrix/utils.go +++ b/pkg/models/openpitrix/utils.go @@ -339,7 +339,7 @@ func convertApp(app *v1alpha1.HelmApplication, versions []*v1alpha1.HelmApplicat out.AppVersionTypes = "helm" // If this keys exists, the workspace of this app has been deleted, set the isv to empty. - if _, exists := app.Annotations[constants.DangingAppCleanupKey]; !exists { + if _, exists := app.Annotations[constants.DanglingAppCleanupKey]; !exists { out.Isv = app.GetWorkspace() }