OCPBUGS-23362: Set status on SG deletion when AWS returns error

Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
openshift · Dec 21, 2023 · bee951b · bee951b
1 parent 0f7c8c4
commit bee951b
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 1 deletion.
diff --git a/api/hypershift/v1beta1/hostedcluster_conditions.go b/api/hypershift/v1beta1/hostedcluster_conditions.go
@@ -146,6 +146,12 @@ const (
 	// blocked from creating machines.
 	AWSDefaultSecurityGroupCreated ConditionType = "AWSDefaultSecurityGroupCreated"
 
+	// AWSDefaultSecurityGroupDeletion indicates whether the default security group
+	// for AWS workers has been deleted.
+	// A failure here indicates that the Security Group has some dependencies that
+	// there are still pending cloud resources to be deleted that are using that SG.
+	AWSDefaultSecurityGroupDeletion ConditionType = "AWSDefaultSecurityGroupDeletion"
+
 	// PlatformCredentialsFound indicates that credentials required for the
 	// desired platform are valid.
 	// A failure here is unlikely to resolve without the changing user input.

diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
@@ -306,15 +306,36 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
 
 	// Return early if deleted
 	if !hostedControlPlane.DeletionTimestamp.IsZero() {
-
+		condition := &metav1.Condition{
+			Type: string(hyperv1.AWSDefaultSecurityGroupDeletion),
+		}
 		if shouldCleanupCloudResources(r.Log, hostedControlPlane) {
 			if err := r.destroyAWSDefaultSecurityGroup(ctx, hostedControlPlane); err != nil {
+				condition.Message = err.Error()
+				condition.Reason = hyperv1.AWSErrorReason
+				condition.Status = metav1.ConditionFalse
+				meta.SetStatusCondition(&hostedControlPlane.Status.Conditions, *condition)
+
+				if err := r.Client.Status().Patch(ctx, hostedControlPlane, client.MergeFromWithOptions(originalHostedControlPlane, client.MergeFromWithOptimisticLock{})); err != nil {
+					return ctrl.Result{}, fmt.Errorf("failed to update status on hcp for security group deletion: %w. Condition error message: %v", err, condition.Message)
+				}
+
 				if awsErrorCode(err) == "UnauthorizedOperation" {
 					r.Log.Info("Skipping AWS default security group deletion because the operator is not authorized to delete it.")
 				} else {
 					return ctrl.Result{}, fmt.Errorf("failed to delete AWS default security group: %w", err)
 				}
+			} else {
+				condition.Message = hyperv1.AllIsWellMessage
+				condition.Reason = hyperv1.AsExpectedReason
+				condition.Status = metav1.ConditionTrue
+				meta.SetStatusCondition(&hostedControlPlane.Status.Conditions, *condition)
+
+				if err := r.Client.Status().Patch(ctx, hostedControlPlane, client.MergeFromWithOptions(originalHostedControlPlane, client.MergeFromWithOptimisticLock{})); err != nil {
+					return ctrl.Result{}, fmt.Errorf("failed to update status on hcp for security group deletion: %w. Condition message: %v", err, condition.Message)
+				}
 			}
+
 			done, err := r.removeCloudResources(ctx, hostedControlPlane)
 			if err != nil {
 				return ctrl.Result{}, fmt.Errorf("failed to ensure cloud resources are removed: %w", err)
@@ -4343,6 +4364,7 @@ func (r *HostedControlPlaneReconciler) destroyAWSDefaultSecurityGroup(ctx contex
 	}); err != nil {
 		return fmt.Errorf("failed to delete security group %s: %w", awssdk.StringValue(sg.GroupId), err)
 	}
+
 	return nil
 
 }

diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md
@@ -2981,6 +2981,12 @@ for AWS workers has been created.
 A failure here indicates that NodePools without a security group will be
 blocked from creating machines.</p>
 </td>
+</tr><tr><td><p>&#34;AWSDefaultSecurityGroupDeletion&#34;</p></td>
+<td><p>AWSDefaultSecurityGroupDeletion indicates whether the default security group
+for AWS workers has been deleted.
+A failure here indicates that the Security Group has some dependencies that
+there are still pending cloud resources to be deleted that are using that SG.</p>
+</td>
 </tr><tr><td><p>&#34;AWSEndpointAvailable&#34;</p></td>
 <td><p>AWSEndpointServiceAvailable indicates whether the AWS Endpoint has been
 created in the guest VPC</p>

diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
@@ -376,6 +376,34 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques
 		}
 	}
 
+	// Bubble up AWSDefaultSecurityGroupDeletion condition from the hostedControlPlane.
+	// We set this condition even if the HC is being deleted, so we can report blocking objects on deletion.
+	{
+		if hcp != nil && hcp.DeletionTimestamp != nil {
+			freshCondition := &metav1.Condition{
+				Type:               string(hyperv1.AWSDefaultSecurityGroupDeletion),
+				Status:             metav1.ConditionUnknown,
+				Reason:             hyperv1.StatusUnknownReason,
+				ObservedGeneration: hcluster.Generation,
+			}
+
+			securityGroupDeletionCondition := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.AWSDefaultSecurityGroupDeletion))
+			if securityGroupDeletionCondition != nil {
+				freshCondition = securityGroupDeletionCondition
+			}
+
+			oldCondition := meta.FindStatusCondition(hcluster.Status.Conditions, string(hyperv1.AWSDefaultSecurityGroupDeletion))
+			if oldCondition == nil || oldCondition.Message != freshCondition.Message {
+				freshCondition.ObservedGeneration = hcluster.Generation
+				meta.SetStatusCondition(&hcluster.Status.Conditions, *freshCondition)
+				// Persist status updates
+				if err := r.Client.Status().Update(ctx, hcluster); err != nil {
+					return ctrl.Result{}, fmt.Errorf("failed to update status: %w", err)
+				}
+			}
+		}
+	}
+
 	// Bubble up CloudResourcesDestroyed condition from the hostedControlPlane.
 	// We set this condition even if the HC is being deleted, so we can construct SLIs for deletion times.
 	{