From 287e75090fbf4b30ccc8c2c2a84189056f40a78f Mon Sep 17 00:00:00 2001 From: EKS Distro PR Bot <75336432+eks-distro-pr-bot@users.noreply.github.com> Date: Wed, 21 Feb 2024 21:14:04 -0600 Subject: [PATCH] Fix machine rollout during mgmt delete (#7676) Co-authored-by: Tanvir Tatla --- controllers/cluster_controller.go | 2 +- pkg/api/v1alpha1/cluster.go | 15 +++++++++++++ pkg/api/v1alpha1/cluster_test.go | 18 ++++++++++++++++ pkg/api/v1alpha1/cluster_types.go | 13 ++++++++++++ pkg/api/v1alpha1/cluster_types_test.go | 11 ++++++++++ pkg/workflows/management/delete_cluster.go | 14 +------------ .../management/delete_install_eksa.go | 1 + pkg/workflows/management/delete_test.go | 21 +++++++------------ 8 files changed, 68 insertions(+), 27 deletions(-) diff --git a/controllers/cluster_controller.go b/controllers/cluster_controller.go index 88c48c38400b..3351f4482650 100644 --- a/controllers/cluster_controller.go +++ b/controllers/cluster_controller.go @@ -467,7 +467,7 @@ func (r *ClusterReconciler) reconcileDelete(ctx context.Context, log logr.Logger return ctrl.Result{}, errors.New("deleting self-managed clusters is not supported") } - if cluster.IsReconcilePaused() { + if cluster.IsReconcilePaused() && !cluster.CanDeleteWhenPaused() { log.Info("Cluster reconciliation is paused, won't process cluster deletion") return ctrl.Result{}, nil } diff --git a/pkg/api/v1alpha1/cluster.go b/pkg/api/v1alpha1/cluster.go index 6ecbd1393dec..578c43c2a8b1 100644 --- a/pkg/api/v1alpha1/cluster.go +++ b/pkg/api/v1alpha1/cluster.go @@ -290,6 +290,21 @@ func (c *Cluster) PauseReconcile() { c.Annotations[pausedAnnotation] = "true" } +// AllowDeleteWhilePaused adds the allow-delete-when-paused annotation to the cluster. +func (c *Cluster) AllowDeleteWhilePaused() { + if c.Annotations == nil { + c.Annotations = map[string]string{} + } + c.Annotations[AllowDeleteWhenPausedAnnotation] = "true" +} + +// PreventDeleteWhilePaused removes the allow-delete-when-paused annotation to the cluster. +func (c *Cluster) PreventDeleteWhilePaused() { + if c.Annotations != nil { + delete(c.Annotations, AllowDeleteWhenPausedAnnotation) + } +} + func (c *Cluster) ClearPauseAnnotation() { if c.Annotations != nil { delete(c.Annotations, pausedAnnotation) diff --git a/pkg/api/v1alpha1/cluster_test.go b/pkg/api/v1alpha1/cluster_test.go index 17edf8ca0996..39146ef1e1ee 100644 --- a/pkg/api/v1alpha1/cluster_test.go +++ b/pkg/api/v1alpha1/cluster_test.go @@ -1616,6 +1616,24 @@ func TestCluster_AddRemoveManagedByCLIAnnotation(t *testing.T) { g.Expect(ok).To(BeFalse()) } +func TestCluster_AddRemoveAllowDeleteWhenPausedAnnotation(t *testing.T) { + g := NewWithT(t) + c := &Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster_test", + }, + } + c.AllowDeleteWhilePaused() + val, ok := c.Annotations[AllowDeleteWhenPausedAnnotation] + + g.Expect(ok).To(BeTrue()) + g.Expect(val).To(ContainSubstring("true")) + + c.PreventDeleteWhilePaused() + _, ok = c.Annotations[AllowDeleteWhenPausedAnnotation] + g.Expect(ok).To(BeFalse()) +} + func TestClusterClearTinkerbellIPAnnotation(t *testing.T) { g := NewWithT(t) c := &Cluster{ diff --git a/pkg/api/v1alpha1/cluster_types.go b/pkg/api/v1alpha1/cluster_types.go index 77e83f1013d5..336dd77e13a3 100644 --- a/pkg/api/v1alpha1/cluster_types.go +++ b/pkg/api/v1alpha1/cluster_types.go @@ -56,6 +56,10 @@ const ( // ControlEndpointDefaultPort defaults cluster control plane endpoint port if not specified. ControlEndpointDefaultPort = "6443" + + // AllowDeleteWhenPausedAnnotation is an annotation applied to an EKS-A cluster that allows the deletion of the cluster + // when paused. + AllowDeleteWhenPausedAnnotation = "anywhere.eks.amazonaws.com/allow-delete-when-paused" ) // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. @@ -1542,6 +1546,15 @@ func (c *Cluster) IsManagedByCLI() bool { return ok && val == "true" } +// CanDeleteWhenPaused returns true if the cluster has the allow-delete-when-paused annotation. +func (c *Cluster) CanDeleteWhenPaused() bool { + if len(c.Annotations) == 0 { + return false + } + val, ok := c.Annotations[AllowDeleteWhenPausedAnnotation] + return ok && val == "true" +} + // +kubebuilder:object:root=true // ClusterList contains a list of Cluster. type ClusterList struct { diff --git a/pkg/api/v1alpha1/cluster_types_test.go b/pkg/api/v1alpha1/cluster_types_test.go index 4eaed495352c..9f5b3f225a64 100644 --- a/pkg/api/v1alpha1/cluster_types_test.go +++ b/pkg/api/v1alpha1/cluster_types_test.go @@ -157,6 +157,17 @@ func TestClusterSetSelfManaged(t *testing.T) { g.Expect(c.IsSelfManaged()).To(BeTrue()) } +func TestClusterCanDeleteWhenPaused(t *testing.T) { + c := &v1alpha1.Cluster{} + c.AllowDeleteWhilePaused() + + g := NewWithT(t) + g.Expect(c.CanDeleteWhenPaused()).To(BeTrue()) + + c.PreventDeleteWhilePaused() + g.Expect(c.CanDeleteWhenPaused()).To(BeFalse()) +} + func TestClusterManagementClusterEqual(t *testing.T) { testCases := []struct { testName string diff --git a/pkg/workflows/management/delete_cluster.go b/pkg/workflows/management/delete_cluster.go index 81497b2e9e91..98aebffcf36c 100644 --- a/pkg/workflows/management/delete_cluster.go +++ b/pkg/workflows/management/delete_cluster.go @@ -13,19 +13,7 @@ type deleteManagementCluster struct{} func (s *deleteManagementCluster) Run(ctx context.Context, commandContext *task.CommandContext) task.Task { logger.Info("Deleting management cluster") - err := commandContext.ClusterManager.ResumeEKSAControllerReconcile(ctx, commandContext.BootstrapCluster, commandContext.ClusterSpec, commandContext.Provider) - if err != nil { - commandContext.SetError(err) - return &workflows.CollectMgmtClusterDiagnosticsTask{} - } - - err = commandContext.ClusterManager.AddManagedByCLIAnnotationForCluster(ctx, commandContext.BootstrapCluster, commandContext.ClusterSpec, commandContext.Provider) - if err != nil { - commandContext.SetError(err) - return &workflows.CollectMgmtClusterDiagnosticsTask{} - } - - err = commandContext.ClusterDeleter.Run(ctx, commandContext.ClusterSpec, *commandContext.BootstrapCluster) + err := commandContext.ClusterDeleter.Run(ctx, commandContext.ClusterSpec, *commandContext.BootstrapCluster) if err != nil { commandContext.SetError(err) return &workflows.CollectMgmtClusterDiagnosticsTask{} diff --git a/pkg/workflows/management/delete_install_eksa.go b/pkg/workflows/management/delete_install_eksa.go index cfe70b43a627..b7dd96e6c93c 100644 --- a/pkg/workflows/management/delete_install_eksa.go +++ b/pkg/workflows/management/delete_install_eksa.go @@ -25,6 +25,7 @@ func (s *installEksaComponentsOnBootstrapForDeleteTask) Run(ctx context.Context, } commandContext.ClusterSpec.Cluster.PauseReconcile() + commandContext.ClusterSpec.Cluster.AllowDeleteWhilePaused() commandContext.ClusterSpec.Cluster.SetFinalizers([]string{"clusters.anywhere.eks.amazonaws.com/finalizer"}) commandContext.ClusterSpec.Cluster.AddManagedByCLIAnnotation() err = applyClusterSpecOnBootstrapForDeleteTask(ctx, commandContext.ClusterSpec, commandContext.BootstrapCluster, commandContext.ClientFactory) diff --git a/pkg/workflows/management/delete_test.go b/pkg/workflows/management/delete_test.go index a3ceee4d9356..32c2cac65325 100644 --- a/pkg/workflows/management/delete_test.go +++ b/pkg/workflows/management/delete_test.go @@ -183,16 +183,11 @@ func (c *deleteTestSetup) expectInstallEksaComponentsBootstrap(err1, err2, err3, ) } -func (c *deleteTestSetup) expectDeleteCluster(err1, err2, err3, err4 error) { +func (c *deleteTestSetup) expectDeleteCluster(err1, err2 error) { gomock.InOrder( + c.clusterDeleter.EXPECT().Run(c.ctx, c.clusterSpec, *c.bootstrapCluster).Return(err1).AnyTimes(), - c.clusterManager.EXPECT().ResumeEKSAControllerReconcile(c.ctx, c.bootstrapCluster, c.clusterSpec, c.provider).Return(err1).AnyTimes(), - - c.clusterManager.EXPECT().AddManagedByCLIAnnotationForCluster(c.ctx, c.bootstrapCluster, c.clusterSpec, c.provider).Return(err2).AnyTimes(), - - c.clusterDeleter.EXPECT().Run(c.ctx, c.clusterSpec, *c.bootstrapCluster).Return(err3).AnyTimes(), - - c.provider.EXPECT().PostClusterDeleteValidate(c.ctx, c.bootstrapCluster).Return(err4).AnyTimes(), + c.provider.EXPECT().PostClusterDeleteValidate(c.ctx, c.bootstrapCluster).Return(err2).AnyTimes(), ) } @@ -225,7 +220,7 @@ func TestDeleteRunSuccess(t *testing.T) { test.expectMoveCAPI(nil, nil) test.expectInstallEksaComponentsBootstrap(nil, nil, nil, nil, nil) test.expectApplyOnBootstrap(nil) - test.expectDeleteCluster(nil, nil, nil, nil) + test.expectDeleteCluster(nil, nil) test.expectCleanupGitRepo(nil) test.expectDeleteBootstrap(nil) test.expectCreateNamespace() @@ -303,7 +298,7 @@ func TestDeleteRunFailInstallCAPI(t *testing.T) { test.expectCreateBootstrap(nil) test.expectPreCAPI(nil) test.expectInstallCAPI(fmt.Errorf("")) - test.expectDeleteCluster(fmt.Errorf(""), nil, nil, nil) + test.expectDeleteCluster(fmt.Errorf(""), nil) test.expectSaveLogsManagement() err := test.run() @@ -462,7 +457,7 @@ func TestDeleteRunFailPostDelete(t *testing.T) { test.expectInstallEksaComponentsBootstrap(nil, nil, nil, nil, nil) test.expectCreateNamespace() test.expectApplyOnBootstrap(nil) - test.expectDeleteCluster(nil, nil, nil, fmt.Errorf("")) + test.expectDeleteCluster(nil, fmt.Errorf("")) test.expectSaveLogsManagement() err := test.run() @@ -484,7 +479,7 @@ func TestDeleteRunFailCleanupGit(t *testing.T) { test.expectInstallEksaComponentsBootstrap(nil, nil, nil, nil, nil) test.expectCreateNamespace() test.expectApplyOnBootstrap(nil) - test.expectDeleteCluster(nil, nil, nil, nil) + test.expectDeleteCluster(nil, nil) test.expectCleanupGitRepo(fmt.Errorf("")) test.expectSaveLogsWorkload() test.expectSaveLogsManagement() @@ -508,7 +503,7 @@ func TestDeleteRunFailDeleteBootstrap(t *testing.T) { test.expectInstallEksaComponentsBootstrap(nil, nil, nil, nil, nil) test.expectApplyOnBootstrap(nil) test.expectCreateNamespace() - test.expectDeleteCluster(nil, nil, nil, nil) + test.expectDeleteCluster(nil, nil) test.expectCleanupGitRepo(nil) test.expectDeleteBootstrap(fmt.Errorf("")) test.expectSaveLogsManagement()