diff --git a/api/hypershift/v1beta1/endpointservice_types.go b/api/hypershift/v1beta1/endpointservice_types.go index abb2df3893e3..0fee9824c3eb 100644 --- a/api/hypershift/v1beta1/endpointservice_types.go +++ b/api/hypershift/v1beta1/endpointservice_types.go @@ -119,6 +119,26 @@ type AWSEndpointServiceStatus struct { // SecurityGroupID is the ID of the security group. // +optional SecurityGroupID string `json:"securityGroupID,omitempty"` + + // sharedVPCEndpointRoleARN is the persisted ARN of the SharedVPC role used + // for EC2 endpoint and security group operations during deletion. + // + // The role ARN is copied from the HostedControlPlane so cleanup can continue + // after the HCP is gone, for example after a controller restart. + // +optional + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=2048 + SharedVPCEndpointRoleARN string `json:"sharedVPCEndpointRoleARN,omitempty"` + + // sharedVPCRoute53RoleARN is the persisted ARN of the SharedVPC role used + // for Route53 operations during deletion. + // + // The role ARN is copied from the HostedControlPlane so cleanup can continue + // after the HCP is gone, for example after a controller restart. + // +optional + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=2048 + SharedVPCRoute53RoleARN string `json:"sharedVPCRoute53RoleARN,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/hypershift/v1beta1/hosted_controlplane.go b/api/hypershift/v1beta1/hosted_controlplane.go index 9dc6fd0b1bc0..ed245ec6e767 100644 --- a/api/hypershift/v1beta1/hosted_controlplane.go +++ b/api/hypershift/v1beta1/hosted_controlplane.go @@ -309,6 +309,26 @@ const ( HostedControlPlaneDegraded ConditionType = "Degraded" EtcdSnapshotRestored ConditionType = "EtcdSnapshotRestored" CVOScaledDown ConditionType = "CVOScaledDown" + + // PrivateConnectivityCleanedUp signals whether platform private connectivity + // resources (AWS PrivateLink endpoints, Azure Private Endpoints) have been + // cleaned up during HostedControlPlane deletion. Set by platform controllers, + // or by CPO when no matching private connectivity resources exist, and gated + // with a 10-minute timeout in CPO's deletion path. The timeout allows + // HCP deletion to proceed even when cleanup is stuck (trading orphaned cloud + // resources for avoiding indefinite deletion blocking), while the condition + // provides visibility into whether cleanup completed successfully or timed out. + PrivateConnectivityCleanedUp ConditionType = "PrivateConnectivityCleanedUp" + + // PrivateConnectivityCleanupCompleteReason is set when platform controllers + // have finished cleaning up all private connectivity resources. + PrivateConnectivityCleanupCompleteReason = "CleanupComplete" + + // PrivateConnectivityCleanupTimedOutReason is set when the cleanup timeout + // elapsed before the platform controller signaled completion. When this occurs, + // cloud resources (endpoints, DNS zones, security groups) may be orphaned and + // require manual cleanup. + PrivateConnectivityCleanupTimedOutReason = "CleanupTimedOut" ) // HostedControlPlaneStatus defines the observed state of HostedControlPlane diff --git a/api/hypershift/v1beta1/hostedcluster_conditions.go b/api/hypershift/v1beta1/hostedcluster_conditions.go index 2a00effd35d0..9cbca182658e 100644 --- a/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/api/hypershift/v1beta1/hostedcluster_conditions.go @@ -70,6 +70,8 @@ const ( // ClusterVersionRetrievedUpdates bubbles up RetrievedUpdates from the CVO. ClusterVersionRetrievedUpdates ConditionType = "ClusterVersionRetrievedUpdates" + // HCP-only conditions (not propagated to HostedCluster). + // UnmanagedEtcdAvailable indicates whether a user-managed etcd cluster is // healthy. UnmanagedEtcdAvailable ConditionType = "UnmanagedEtcdAvailable" diff --git a/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/awsendpointservices.hypershift.openshift.io/AAA_ungated.yaml b/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/awsendpointservices.hypershift.openshift.io/AAA_ungated.yaml index 3e1106de17eb..873f29404277 100644 --- a/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/awsendpointservices.hypershift.openshift.io/AAA_ungated.yaml +++ b/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/awsendpointservices.hypershift.openshift.io/AAA_ungated.yaml @@ -196,6 +196,26 @@ spec: SecurityGroupID is the ID of the security group. maxLength: 255 type: string + sharedVPCEndpointRoleARN: + description: |- + sharedVPCEndpointRoleARN is the persisted ARN of the SharedVPC role used + for EC2 endpoint and security group operations during deletion. + + The role ARN is copied from the HostedControlPlane so cleanup can continue + after the HCP is gone, for example after a controller restart. + maxLength: 2048 + minLength: 1 + type: string + sharedVPCRoute53RoleARN: + description: |- + sharedVPCRoute53RoleARN is the persisted ARN of the SharedVPC role used + for Route53 operations during deletion. + + The role ARN is copied from the HostedControlPlane so cleanup can continue + after the HCP is gone, for example after a controller restart. + maxLength: 2048 + minLength: 1 + type: string type: object type: object served: true diff --git a/cmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/awsendpointservices.crd.yaml b/cmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/awsendpointservices.crd.yaml index 668d105a2414..3054986bddcd 100644 --- a/cmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/awsendpointservices.crd.yaml +++ b/cmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/awsendpointservices.crd.yaml @@ -198,6 +198,26 @@ spec: SecurityGroupID is the ID of the security group. maxLength: 255 type: string + sharedVPCEndpointRoleARN: + description: |- + sharedVPCEndpointRoleARN is the persisted ARN of the SharedVPC role used + for EC2 endpoint and security group operations during deletion. + + The role ARN is copied from the HostedControlPlane so cleanup can continue + after the HCP is gone, for example after a controller restart. + maxLength: 2048 + minLength: 1 + type: string + sharedVPCRoute53RoleARN: + description: |- + sharedVPCRoute53RoleARN is the persisted ARN of the SharedVPC role used + for Route53 operations during deletion. + + The role ARN is copied from the HostedControlPlane so cleanup can continue + after the HCP is gone, for example after a controller restart. + maxLength: 2048 + minLength: 1 + type: string type: object type: object served: true diff --git a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go b/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go index e48fa94bf958..0d7899a4ac8a 100644 --- a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go +++ b/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go @@ -13,8 +13,10 @@ import ( "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" "github.com/openshift/hypershift/support/awsapi" supportawsutil "github.com/openshift/hypershift/support/awsutil" + "github.com/openshift/hypershift/support/conditions" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/netutil" + "github.com/openshift/hypershift/support/statuspatching" "github.com/openshift/hypershift/support/upsert" "github.com/openshift/hypershift/support/util" @@ -242,6 +244,7 @@ type AWSEndpointServiceReconciler struct { type awsClientProvider interface { getClients(ctx context.Context) (awsapi.EC2API, awsapi.ROUTE53API, error) initializeWithHCP(log logr.Logger, hcp *hyperv1.HostedControlPlane) + initializeWithSharedVPCRoleARNs(endpointRoleARN, route53RoleARN string) getLocalHostedZoneID() string setLocalHostedZoneID(zoneID string) } @@ -334,6 +337,15 @@ func (b *clientBuilder) initializeWithHCP(log logr.Logger, hcp *hyperv1.HostedCo } } +func (b *clientBuilder) initializeWithSharedVPCRoleARNs(endpointRoleARN, route53RoleARN string) { + b.mu.Lock() + defer b.mu.Unlock() + + b.assumeSharedVPCEndpointRoleARN = endpointRoleARN + b.assumeSharedVPCRoute53RoleARN = route53RoleARN + b.initialized = true +} + func (b *clientBuilder) warnOnDifferentValues(log logr.Logger, hcp *hyperv1.HostedControlPlane) { newEndpointRoleARN := "" newRoute53RoleARN := "" @@ -375,6 +387,7 @@ func (r *AWSEndpointServiceReconciler) SetupWithManager(mgr ctrl.Manager) error RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](3*time.Second, 30*time.Second), MaxConcurrentReconciles: 10, }). + Watches(&hyperv1.HostedControlPlane{}, handler.EnqueueRequestsFromMapFunc(r.mapHCPToAWSEndpointServices())). Watches(&hyperv1.HostedControlPlane{}, handler.Funcs{UpdateFunc: r.enqueueOnAccessChange(mgr)}). Build(r) if err != nil { @@ -385,6 +398,28 @@ func (r *AWSEndpointServiceReconciler) SetupWithManager(mgr ctrl.Manager) error return nil } +func (r *AWSEndpointServiceReconciler) mapHCPToAWSEndpointServices() handler.MapFunc { + return func(ctx context.Context, obj client.Object) []reconcile.Request { + hcp, ok := obj.(*hyperv1.HostedControlPlane) + if !ok { + return nil + } + if hcp.DeletionTimestamp.IsZero() { + return nil + } + awsEndpointServiceList := &hyperv1.AWSEndpointServiceList{} + if err := r.List(ctx, awsEndpointServiceList, client.InNamespace(hcp.Namespace)); err != nil { + logr.FromContextOrDiscard(ctx).Error(err, "failed to list AWSEndpointService resources for HCP mapping", "namespace", hcp.Namespace) + return nil + } + var requests []reconcile.Request + for i := range awsEndpointServiceList.Items { + requests = append(requests, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&awsEndpointServiceList.Items[i])}) + } + return requests + } +} + func (r *AWSEndpointServiceReconciler) enqueueOnAccessChange(mgr ctrl.Manager) func(context.Context, event.UpdateEvent, workqueue.TypedRateLimitingInterface[reconcile.Request]) { return func(ctx context.Context, e event.UpdateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { logger := mgr.GetLogger() @@ -398,7 +433,6 @@ func (r *AWSEndpointServiceReconciler) enqueueOnAccessChange(mgr ctrl.Manager) f logger.Info("WARNING: enqueueOnAccessChange: old resource is not of type HostedControlPlane") return } - // Only enqueue awsendpointservices when there is a change in the endpointaccess value, otherwise ignore changes if newHCP.Spec.Platform.AWS != nil && oldHCP.Spec.Platform.AWS != nil && newHCP.Spec.Platform.AWS.EndpointAccess != oldHCP.Spec.Platform.AWS.EndpointAccess { awsEndpointServiceList := &hyperv1.AWSEndpointServiceList{} if err := r.List(context.Background(), awsEndpointServiceList, client.InNamespace(newHCP.Namespace)); err != nil { @@ -437,46 +471,21 @@ func (r *AWSEndpointServiceReconciler) Reconcile(ctx context.Context, req ctrl.R // Return early if deleted if !awsEndpointService.DeletionTimestamp.IsZero() { - if !controllerutil.ContainsFinalizer(awsEndpointService, finalizer) { - // If we previously removed our finalizer, don't delete again and return early - return ctrl.Result{}, nil - } - - // Best-effort initialization for deletion reconciles: after a controller restart - // the clientBuilder is uninitialized because initializeWithHCP is only called in - // the non-deletion path. If the HCP still exists, initialize from it so that - // getClients can succeed and deletion can proceed. - // - // Known issue (SharedVPC): when the HCP is already deleted, the SharedVPC role - // ARNs (needed for cross-account EC2/Route53 access) are lost. Initialization - // cannot happen, getClients will fail, and the finalizer will be preserved until - // the hypershift-operator force-removes it after the grace period — orphaning - // AWS resources in the shared VPC account. A proper fix requires persisting the - // SharedVPC role ARNs in the AWSEndpointService status. See - // TestReconcileDeletionSharedVPC for details. - hcpList := &hyperv1.HostedControlPlaneList{} - if err := r.List(ctx, hcpList, &client.ListOptions{Namespace: req.Namespace}); err == nil && len(hcpList.Items) == 1 { - r.awsClientBuilder.initializeWithHCP(log, &hcpList.Items[0]) - } + return r.reconcileEndpointServiceDeletion(ctx, awsEndpointService, log) + } - ec2Client, route53Client, err := r.awsClientBuilder.getClients(ctx) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to get AWS clients for endpoint service cleanup: %w", err) - } - completed, err := r.delete(ctx, awsEndpointService, ec2Client, route53Client) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to delete resource: %w", err) - } - if !completed { - return ctrl.Result{RequeueAfter: endpointServiceDeletionRequeueDuration}, nil - } - if controllerutil.ContainsFinalizer(awsEndpointService, finalizer) { - controllerutil.RemoveFinalizer(awsEndpointService, finalizer) - if err := r.Update(ctx, awsEndpointService); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to remove finalizer: %w", err) - } + // Check if HCP is being deleted — handle cleanup before adding CR finalizer + hcp, err := r.getHostedControlPlane(ctx, req.Namespace) + if err != nil { + return ctrl.Result{}, err + } + if hcp != nil && !hcp.DeletionTimestamp.IsZero() { + return r.reconcileHCPDeletion(ctx, awsEndpointService, hcp, log) + } + if hcp != nil { + if err := r.persistSharedVPCRoleARNs(ctx, awsEndpointService, hcp); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to persist SharedVPC role ARNs: %w", err) } - return ctrl.Result{}, nil } // Ensure the awsEndpointService has a finalizer for cleanup @@ -497,19 +506,9 @@ func (r *AWSEndpointServiceReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, nil } - // Fetch the HostedControlPlane - hcpList := &hyperv1.HostedControlPlaneList{} - if err := r.List(ctx, hcpList, &client.ListOptions{Namespace: req.Namespace}); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to get resource: %w", err) - } - if len(hcpList.Items) == 0 { - // Return early if HostedControlPlane is deleted + if hcp == nil { return ctrl.Result{}, nil } - if len(hcpList.Items) > 1 { - return ctrl.Result{}, fmt.Errorf("unexpected number of HostedControlPlanes in namespace, expected: 1, actual: %d", len(hcpList.Items)) - } - hcp := &hcpList.Items[0] if isPaused, duration := util.IsReconciliationPaused(log, hcp.Spec.PausedUntil); isPaused { log.Info("Reconciliation paused", "pausedUntil", *hcp.Spec.PausedUntil) @@ -561,6 +560,136 @@ func (r *AWSEndpointServiceReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil } +func (r *AWSEndpointServiceReconciler) reconcileEndpointServiceDeletion(ctx context.Context, awsEndpointService *hyperv1.AWSEndpointService, log logr.Logger) (ctrl.Result, error) { + if !controllerutil.ContainsFinalizer(awsEndpointService, finalizer) { + // If we previously removed our finalizer, don't delete again and return early + return ctrl.Result{}, nil + } + + // Best-effort initialization for deletion reconciles: after a controller restart + // the clientBuilder is uninitialized because initializeWithHCP is only called in + // the non-deletion path. If the HCP still exists, initialize from it so that + // getClients can succeed and deletion can proceed. If the HCP is already gone, + // use the role ARNs persisted on the AWSEndpointService status. + hcpList := &hyperv1.HostedControlPlaneList{} + if err := r.List(ctx, hcpList, &client.ListOptions{Namespace: awsEndpointService.Namespace}); err == nil && len(hcpList.Items) == 1 { + r.awsClientBuilder.initializeWithHCP(log, &hcpList.Items[0]) + } else if awsEndpointService.Status.SharedVPCEndpointRoleARN != "" || awsEndpointService.Status.SharedVPCRoute53RoleARN != "" { + r.awsClientBuilder.initializeWithSharedVPCRoleARNs( + awsEndpointService.Status.SharedVPCEndpointRoleARN, + awsEndpointService.Status.SharedVPCRoute53RoleARN, + ) + } + + ec2Client, route53Client, err := r.awsClientBuilder.getClients(ctx) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to get AWS clients for endpoint service cleanup: %w", err) + } + completed, err := r.delete(ctx, awsEndpointService, ec2Client, route53Client) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to delete resource: %w", err) + } + if !completed { + return ctrl.Result{RequeueAfter: endpointServiceDeletionRequeueDuration}, nil + } + if controllerutil.ContainsFinalizer(awsEndpointService, finalizer) { + controllerutil.RemoveFinalizer(awsEndpointService, finalizer) + if err := r.Update(ctx, awsEndpointService); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to remove finalizer: %w", err) + } + } + return ctrl.Result{}, nil +} + +func (r *AWSEndpointServiceReconciler) getHostedControlPlane(ctx context.Context, namespace string) (*hyperv1.HostedControlPlane, error) { + hcpList := &hyperv1.HostedControlPlaneList{} + if err := r.List(ctx, hcpList, &client.ListOptions{Namespace: namespace}); err != nil { + return nil, fmt.Errorf("failed to list HostedControlPlanes: %w", err) + } + if len(hcpList.Items) == 0 { + return nil, nil + } + if len(hcpList.Items) > 1 { + return nil, fmt.Errorf("unexpected number of HostedControlPlanes in namespace, expected: 1, actual: %d", len(hcpList.Items)) + } + return &hcpList.Items[0], nil +} + +func (r *AWSEndpointServiceReconciler) persistSharedVPCRoleARNs(ctx context.Context, awsEndpointService *hyperv1.AWSEndpointService, hcp *hyperv1.HostedControlPlane) error { + endpointRoleARN, route53RoleARN := sharedVPCRoleARNs(hcp) + return statuspatching.PatchStatus(ctx, r.Client, awsEndpointService, func() error { + awsEndpointService.Status.SharedVPCEndpointRoleARN = endpointRoleARN + awsEndpointService.Status.SharedVPCRoute53RoleARN = route53RoleARN + return nil + }) +} + +func sharedVPCRoleARNs(hcp *hyperv1.HostedControlPlane) (string, string) { + if hcp == nil || hcp.Spec.Platform.AWS == nil || hcp.Spec.Platform.AWS.SharedVPC == nil { + return "", "" + } + return hcp.Spec.Platform.AWS.SharedVPC.RolesRef.ControlPlaneARN, hcp.Spec.Platform.AWS.SharedVPC.RolesRef.IngressARN +} + +func (r *AWSEndpointServiceReconciler) reconcileHCPDeletion(ctx context.Context, awsEndpointService *hyperv1.AWSEndpointService, hcp *hyperv1.HostedControlPlane, log logr.Logger) (ctrl.Result, error) { + if !awsEndpointService.DeletionTimestamp.IsZero() { + return ctrl.Result{}, nil + } + + if controllerutil.ContainsFinalizer(awsEndpointService, finalizer) { + r.awsClientBuilder.initializeWithHCP(log, hcp) + ec2Client, route53Client, err := r.awsClientBuilder.getClients(ctx) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to get AWS clients for HCP deletion cleanup: %w", err) + } + completed, err := r.delete(ctx, awsEndpointService, ec2Client, route53Client) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to delete AWS resources: %w", err) + } + if !completed { + return ctrl.Result{RequeueAfter: endpointServiceDeletionRequeueDuration}, nil + } + controllerutil.RemoveFinalizer(awsEndpointService, finalizer) + if err := r.Update(ctx, awsEndpointService); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to remove finalizer: %w", err) + } + } + + allCleanedUp, err := r.allEndpointServicesCleanedUp(ctx, awsEndpointService.Namespace, awsEndpointService.Name) + if err != nil { + return ctrl.Result{}, err + } + if !allCleanedUp { + return ctrl.Result{RequeueAfter: endpointServiceDeletionRequeueDuration}, nil + } + + if err := conditions.PatchPrivateConnectivityCleanupCondition(ctx, r.Client, hcp, metav1.ConditionTrue, hyperv1.PrivateConnectivityCleanupCompleteReason, "All AWS PrivateLink resources have been cleaned up"); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to set PrivateConnectivityCleanedUp condition: %w", err) + } + log.Info("Set PrivateConnectivityCleanedUp condition on HCP") + + return ctrl.Result{}, nil +} + +func (r *AWSEndpointServiceReconciler) allEndpointServicesCleanedUp(ctx context.Context, namespace, selfName string) (bool, error) { + list := &hyperv1.AWSEndpointServiceList{} + if err := r.List(ctx, list, &client.ListOptions{Namespace: namespace}); err != nil { + return false, fmt.Errorf("failed to list AWSEndpointServices: %w", err) + } + for i := range list.Items { + if list.Items[i].Name == selfName { + continue + } + if controllerutil.ContainsFinalizer(&list.Items[i], finalizer) { + return false, nil + } + } + return true, nil +} + func isAWSThrottleError(err error) bool { switch supportawsutil.AWSErrorCode(err) { case "Throttling", "ThrottlingException", "RequestLimitExceeded", "TooManyRequestsException": diff --git a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go b/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go index 068a7686b06f..60e160fdb52c 100644 --- a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go +++ b/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller_test.go @@ -21,6 +21,7 @@ import ( "github.com/aws/smithy-go" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -897,24 +898,48 @@ func TestDeleteSecurityGroup(t *testing.T) { } } -// TestReconcileDeletionSharedVPC documents the remaining SharedVPC leak scenario. -// -// In SharedVPC clusters, the clientBuilder needs role ARNs from the HostedControlPlane -// (hcp.Spec.Platform.AWS.SharedVPC.RolesRef) to assume cross-account roles for EC2 -// and Route53 operations. These ARNs are only stored in-memory in the clientBuilder -// after initializeWithHCP is called. -// -// The deletion path now attempts best-effort initialization by listing HCPs in the -// namespace. However, when the operator restarts during deletion and the HCP has -// already been deleted: -// - The best-effort List finds no HCP, so initializeWithHCP is not called -// - getClients fails with "clients not initialized" -// - The fix preserves the finalizer, but retries will never succeed -// - After 10 minutes, the hypershift-operator force-removes the CPO finalizer, -// orphaning the security group, VPC endpoint, and DNS records -// -// A proper fix requires persisting the SharedVPC role ARNs in the AWSEndpointService -// status so the deletion path can authenticate independently of the HCP. +func TestPersistSharedVPCRoleARNs(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + + awsEndpointService := &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{Name: "private-router", Namespace: "clusters-sharedvpc"}, + } + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: "test-hcp", Namespace: "clusters-sharedvpc"}, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{ + SharedVPC: &hyperv1.AWSSharedVPC{ + RolesRef: hyperv1.AWSSharedVPCRolesRef{ + ControlPlaneARN: "arn:aws:iam::123456789012:role/shared-vpc-endpoint-role", + IngressARN: "arn:aws:iam::123456789012:role/shared-vpc-route53-role", + }, + }, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(awsEndpointService, hcp). + WithStatusSubresource(awsEndpointService). + Build() + + r := &AWSEndpointServiceReconciler{Client: fakeClient} + err := r.persistSharedVPCRoleARNs(t.Context(), awsEndpointService, hcp) + g.Expect(err).ToNot(HaveOccurred()) + + updated := &hyperv1.AWSEndpointService{} + g.Expect(fakeClient.Get(t.Context(), crclient.ObjectKeyFromObject(awsEndpointService), updated)).To(Succeed()) + g.Expect(updated.Status.SharedVPCEndpointRoleARN).To(Equal("arn:aws:iam::123456789012:role/shared-vpc-endpoint-role")) + g.Expect(updated.Status.SharedVPCRoute53RoleARN).To(Equal("arn:aws:iam::123456789012:role/shared-vpc-route53-role")) +} + func TestHasAWSConfig(t *testing.T) { tests := []struct { name string @@ -1631,6 +1656,7 @@ func TestReconcileDeletionSharedVPC(t *testing.T) { testCases := []struct { name string hasHCP bool + persistedRoleARNs bool setupMocks func(ctrl *gomock.Controller) *MockawsClientProvider expectError bool expectErrorContains string @@ -1654,6 +1680,23 @@ func TestReconcileDeletionSharedVPC(t *testing.T) { expectErrorContains: "clients not initialized", expectFinalizer: true, }, + { + name: "When HCP is gone but SharedVPC role ARNs are persisted, it should initialize clients from status", + hasHCP: false, + persistedRoleARNs: true, + setupMocks: func(mockCtrl *gomock.Controller) *MockawsClientProvider { + mockBuilder := NewMockawsClientProvider(mockCtrl) + mockBuilder.EXPECT().initializeWithSharedVPCRoleARNs( + "arn:aws:iam::123456789012:role/shared-vpc-endpoint-role", + "arn:aws:iam::123456789012:role/shared-vpc-route53-role", + ) + mockBuilder.EXPECT().getClients(gomock.Any()).Return(nil, nil, fmt.Errorf("clients unavailable for test")) + return mockBuilder + }, + expectError: true, + expectErrorContains: "clients unavailable for test", + expectFinalizer: true, + }, { // This scenario shows what happens if the clientBuilder is re-initialized // without the SharedVPC role ARNs (e.g. a naive fix that initializes without @@ -1702,6 +1745,10 @@ func TestReconcileDeletionSharedVPC(t *testing.T) { DNSZoneID: "Z1234567890", }, } + if tc.persistedRoleARNs { + awsEndpointService.Status.SharedVPCEndpointRoleARN = "arn:aws:iam::123456789012:role/shared-vpc-endpoint-role" + awsEndpointService.Status.SharedVPCRoute53RoleARN = "arn:aws:iam::123456789012:role/shared-vpc-route53-role" + } objects := []crclient.Object{awsEndpointService} if tc.hasHCP { @@ -2118,3 +2165,570 @@ func TestIsAWSThrottleError(t *testing.T) { }) } } + +func TestMapHCPToAWSEndpointServices(t *testing.T) { + now := metav1.NewTime(time.Now()) + + tests := []struct { + name string + obj crclient.Object + existingObjects []crclient.Object + expectedLen int + }{ + { + name: "When object is not an HCP, it should return nil", + obj: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-service", + Namespace: "test-ns", + }, + }, + expectedLen: 0, + }, + { + name: "When HCP has no deletion timestamp, it should return nil", + obj: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + }, + }, + expectedLen: 0, + }, + { + name: "When HCP is being deleted, it should return requests for all endpoint services", + obj: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + DeletionTimestamp: &now, + Finalizers: []string{"some-finalizer"}, + }, + }, + existingObjects: []crclient.Object{ + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-apiserver-private", + Namespace: "test-ns", + }, + }, + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "private-router", + Namespace: "test-ns", + }, + }, + }, + expectedLen: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tt.existingObjects...). + Build() + + r := &AWSEndpointServiceReconciler{ + Client: fakeClient, + } + + mapFn := r.mapHCPToAWSEndpointServices() + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + requests := mapFn(ctx, tt.obj) + + g.Expect(requests).To(HaveLen(tt.expectedLen)) + }) + } +} + +func TestGetHostedControlPlane(t *testing.T) { + tests := []struct { + name string + objects []crclient.Object + expectHCP bool + expectError bool + }{ + { + name: "When no HCPs exist, it should return nil", + objects: nil, + expectHCP: false, + expectError: false, + }, + { + name: "When one HCP exists, it should return it", + objects: []crclient.Object{ + &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + }, + }, + }, + expectHCP: true, + expectError: false, + }, + { + name: "When multiple HCPs exist, it should return error", + objects: []crclient.Object{ + &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hcp-1", + Namespace: "test-ns", + }, + }, + &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hcp-2", + Namespace: "test-ns", + }, + }, + }, + expectHCP: false, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tt.objects...). + Build() + + r := &AWSEndpointServiceReconciler{ + Client: fakeClient, + } + + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + hcp, err := r.getHostedControlPlane(ctx, "test-ns") + + if tt.expectError { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("unexpected number of HostedControlPlanes")) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + + if tt.expectHCP { + g.Expect(hcp).ToNot(BeNil()) + g.Expect(hcp.Name).To(Equal("test-hcp")) + } else if !tt.expectError { + g.Expect(hcp).To(BeNil()) + } + }) + } +} + +func TestAllEndpointServicesCleanedUp(t *testing.T) { + tests := []struct { + name string + selfName string + objects []crclient.Object + expected bool + expectError bool + }{ + { + name: "When no other CRs exist, it should return true", + selfName: "my-service", + objects: []crclient.Object{ + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + }, + }, + }, + expected: true, + }, + { + name: "When other CRs have no finalizer, it should return true", + selfName: "my-service", + objects: []crclient.Object{ + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + Namespace: "test-ns", + }, + }, + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-service", + Namespace: "test-ns", + }, + }, + }, + expected: true, + }, + { + name: "When other CR has finalizer, it should return false", + selfName: "my-service", + objects: []crclient.Object{ + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + Namespace: "test-ns", + }, + }, + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-service", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + }, + }, + }, + expected: false, + }, + { + name: "When self has finalizer, it should be skipped and return true", + selfName: "my-service", + objects: []crclient.Object{ + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + }, + }, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tt.objects...). + Build() + + r := &AWSEndpointServiceReconciler{ + Client: fakeClient, + } + + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + result, err := r.allEndpointServicesCleanedUp(ctx, "test-ns", tt.selfName) + + if tt.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(tt.expected)) + } + }) + } +} + +func TestReconcileHCPDeletion_WhenCRIsBeingDeleted_ItShouldReturnImmediately(t *testing.T) { + g := NewGomegaWithT(t) + + now := metav1.NewTime(time.Now()) + + awsEndpointService := &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "private-router", + Namespace: "test-ns", + DeletionTimestamp: &now, + Finalizers: []string{finalizer}, + }, + } + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + DeletionTimestamp: &now, + Finalizers: []string{"some-finalizer"}, + }, + } + + r := &AWSEndpointServiceReconciler{} + + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + result, err := r.reconcileHCPDeletion(ctx, awsEndpointService, hcp, ctrl.Log.WithName("test")) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(ctrl.Result{})) +} + +func TestReconcileHCPDeletion_WhenSingleCRCleanupCompletes_ItShouldSetConditionOnHCP(t *testing.T) { + g := NewGomegaWithT(t) + + now := metav1.NewTime(time.Now()) + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + + awsEndpointService := &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "private-router", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + }, + Status: hyperv1.AWSEndpointServiceStatus{}, + } + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + DeletionTimestamp: &now, + Finalizers: []string{"some-finalizer"}, + }, + } + + mockCtrl := gomock.NewController(t) + mockBuilder := NewMockawsClientProvider(mockCtrl) + mockEC2 := awsapi.NewMockEC2API(mockCtrl) + mockRoute53 := awsapi.NewMockROUTE53API(mockCtrl) + mockBuilder.EXPECT().initializeWithHCP(gomock.Any(), gomock.Any()) + mockBuilder.EXPECT().getClients(gomock.Any()).Return(mockEC2, mockRoute53, nil) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(awsEndpointService, hcp). + WithStatusSubresource(&hyperv1.HostedControlPlane{}). + Build() + + r := &AWSEndpointServiceReconciler{ + Client: fakeClient, + awsClientBuilder: mockBuilder, + } + + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + + // Re-read so objects have ResourceVersion for OptimisticLock + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(hcp), hcp)).To(Succeed()) + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(awsEndpointService), awsEndpointService)).To(Succeed()) + + result, err := r.reconcileHCPDeletion(ctx, awsEndpointService, hcp, ctrl.Log.WithName("test")) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(ctrl.Result{})) + + // Verify CR finalizer was removed + updatedAES := &hyperv1.AWSEndpointService{} + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(awsEndpointService), updatedAES)).To(Succeed()) + g.Expect(controllerutil.ContainsFinalizer(updatedAES, finalizer)).To(BeFalse()) + + // Verify PrivateConnectivityCleanedUp condition was set on HCP + updatedHCP := &hyperv1.HostedControlPlane{} + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(hcp), updatedHCP)).To(Succeed()) + cond := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(hyperv1.PrivateConnectivityCleanupCompleteReason)) +} + +func TestReconcileHCPDeletion_WhenOtherCRsStillHaveFinalizers_ItShouldNotSetCondition(t *testing.T) { + g := NewGomegaWithT(t) + + now := metav1.NewTime(time.Now()) + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + + // This CR's cleanup completes + awsEndpointService := &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "private-router", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + }, + Status: hyperv1.AWSEndpointServiceStatus{}, + } + + // Another CR still has its finalizer (cleanup not done) + otherAES := &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-apiserver-private", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + }, + } + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + DeletionTimestamp: &now, + Finalizers: []string{"some-finalizer"}, + }, + } + + mockCtrl := gomock.NewController(t) + mockBuilder := NewMockawsClientProvider(mockCtrl) + mockEC2 := awsapi.NewMockEC2API(mockCtrl) + mockRoute53 := awsapi.NewMockROUTE53API(mockCtrl) + mockBuilder.EXPECT().initializeWithHCP(gomock.Any(), gomock.Any()) + mockBuilder.EXPECT().getClients(gomock.Any()).Return(mockEC2, mockRoute53, nil) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(awsEndpointService, otherAES, hcp). + WithStatusSubresource(&hyperv1.HostedControlPlane{}). + Build() + + r := &AWSEndpointServiceReconciler{ + Client: fakeClient, + awsClientBuilder: mockBuilder, + } + + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(hcp), hcp)).To(Succeed()) + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(awsEndpointService), awsEndpointService)).To(Succeed()) + + result, err := r.reconcileHCPDeletion(ctx, awsEndpointService, hcp, ctrl.Log.WithName("test")) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.RequeueAfter).To(Equal(endpointServiceDeletionRequeueDuration)) + + // Verify CR finalizer was removed from this CR + updatedAES := &hyperv1.AWSEndpointService{} + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(awsEndpointService), updatedAES)).To(Succeed()) + g.Expect(controllerutil.ContainsFinalizer(updatedAES, finalizer)).To(BeFalse()) + + // Verify condition was NOT set on HCP (other CR still pending) + updatedHCP := &hyperv1.HostedControlPlane{} + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(hcp), updatedHCP)).To(Succeed()) + cond := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(cond).To(BeNil()) +} + +func TestReconcileHCPDeletion_WhenDeleteReturnsNotCompleted_ItShouldRequeue(t *testing.T) { + g := NewGomegaWithT(t) + + now := metav1.NewTime(time.Now()) + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + + awsEndpointService := &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "private-router", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + }, + Status: hyperv1.AWSEndpointServiceStatus{ + SecurityGroupID: "sg-12345", + }, + } + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + DeletionTimestamp: &now, + Finalizers: []string{"some-finalizer"}, + }, + } + + mockCtrl := gomock.NewController(t) + mockBuilder := NewMockawsClientProvider(mockCtrl) + mockEC2 := awsapi.NewMockEC2API(mockCtrl) + mockRoute53 := awsapi.NewMockROUTE53API(mockCtrl) + mockBuilder.EXPECT().initializeWithHCP(gomock.Any(), gomock.Any()) + mockBuilder.EXPECT().getClients(gomock.Any()).Return(mockEC2, mockRoute53, nil) + // SG cleanup returns DependencyViolation — delete() returns (false, nil) + mockEC2.EXPECT().DescribeSecurityGroups(gomock.Any(), gomock.Any()).Return(&ec2v2.DescribeSecurityGroupsOutput{ + SecurityGroups: []ec2types.SecurityGroup{{ + GroupId: aws.String("sg-12345"), + }}, + }, nil) + mockEC2.EXPECT().DeleteSecurityGroup(gomock.Any(), gomock.Any()).Return(nil, + &smithy.GenericAPIError{Code: "DependencyViolation", Message: "resource has a dependent object"}) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(awsEndpointService, hcp). + WithStatusSubresource(&hyperv1.HostedControlPlane{}). + Build() + + r := &AWSEndpointServiceReconciler{ + Client: fakeClient, + awsClientBuilder: mockBuilder, + } + + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(awsEndpointService), awsEndpointService)).To(Succeed()) + + result, err := r.reconcileHCPDeletion(ctx, awsEndpointService, hcp, ctrl.Log.WithName("test")) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.RequeueAfter).To(Equal(endpointServiceDeletionRequeueDuration)) + + // Verify finalizer still present + updatedAES := &hyperv1.AWSEndpointService{} + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(awsEndpointService), updatedAES)).To(Succeed()) + g.Expect(controllerutil.ContainsFinalizer(updatedAES, finalizer)).To(BeTrue()) +} + +func TestReconcileHCPDeletion_WhenCRHasNoFinalizer_ItShouldSkipCleanupAndCheckAllCRs(t *testing.T) { + g := NewGomegaWithT(t) + + now := metav1.NewTime(time.Now()) + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + + // CR already cleaned up (no finalizer) + awsEndpointService := &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "private-router", + Namespace: "test-ns", + }, + } + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + DeletionTimestamp: &now, + Finalizers: []string{"some-finalizer"}, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(awsEndpointService, hcp). + WithStatusSubresource(&hyperv1.HostedControlPlane{}). + Build() + + // No mock needed — should skip AWS cleanup entirely + r := &AWSEndpointServiceReconciler{ + Client: fakeClient, + } + + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(hcp), hcp)).To(Succeed()) + + result, err := r.reconcileHCPDeletion(ctx, awsEndpointService, hcp, ctrl.Log.WithName("test")) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(ctrl.Result{})) + + // Verify condition was set (this is the only CR and it has no finalizer) + updatedHCP := &hyperv1.HostedControlPlane{} + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(hcp), updatedHCP)).To(Succeed()) + cond := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) +} diff --git a/control-plane-operator/controllers/azureprivatelinkservice/controller.go b/control-plane-operator/controllers/azureprivatelinkservice/controller.go index 466e263d3a66..ea6bfa9fb56a 100644 --- a/control-plane-operator/controllers/azureprivatelinkservice/controller.go +++ b/control-plane-operator/controllers/azureprivatelinkservice/controller.go @@ -9,6 +9,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" manifests "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" "github.com/openshift/hypershift/support/azureutil" + "github.com/openshift/hypershift/support/conditions" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/k8sutil" "github.com/openshift/hypershift/support/netutil" @@ -27,11 +28,13 @@ import ( "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/go-logr/logr" @@ -108,25 +111,10 @@ const ( // aRecordTTL is the TTL for A records in seconds. aRecordTTL int64 = 300 - // hcpAzurePLSFinalizerName is a finalizer placed on the HostedControlPlane to ensure - // Azure PE and DNS cleanup completes before the HCP and its credentials are torn down. - // - // Problem: During HCP deletion, credentials (workload identity tokens, service principal - // secrets) may become invalid before the CPO reconciler gets a chance to clean up Azure - // resources. Without this finalizer, the AzurePrivateLinkService CR's own finalizer - // runs during CR deletion, but by that point the HCP credentials may already be gone, - // causing Azure API calls to fail with authentication errors. This orphans PE and DNS - // resources in the customer's Azure subscription. - // - // Solution: This HCP finalizer blocks HCP deletion until the CPO reconciler confirms - // all Azure resources (PE, DNS zone, VNet link, A record) are deleted. Only then is - // the finalizer removed, allowing HCP deletion to proceed with credentials still valid. - // - // Flow: - // 1. During normal reconciliation, this finalizer is added to the HCP. - // 2. When HCP.DeletionTimestamp is set, the reconciler detects it via the HCP watch, - // performs Azure resource cleanup, and removes this finalizer from the HCP. - // 3. HCP deletion then proceeds, tearing down credentials and the namespace. + // hcpAzurePLSFinalizerName is the legacy HCP finalizer. It has been replaced by the + // PrivateConnectivityCleanedUp status condition. Kept for backward compatibility: + // the controller proactively removes this finalizer from existing HCPs during + // normal reconciliation and during HCP deletion. hcpAzurePLSFinalizerName = "hypershift.openshift.io/azure-pls-endpoint-cleanup" // externalPrivateServiceLabelAzure labels ExternalName Services created for external-dns @@ -185,11 +173,20 @@ type AzurePrivateLinkServiceReconciler struct { RecordSets RecordSetsAPI } +// legacyHCPFinalizerReconciler removes the legacy Azure Private Link finalizer +// when an HCP is deleted without any AzurePrivateLinkService resources. The +// primary reconciler cannot handle this case because it is keyed by +// AzurePrivateLinkService objects. +type legacyHCPFinalizerReconciler struct { + client.Client + apiReader client.Reader +} + // SetupWithManager sets up the controller with the Manager. // It watches AzurePrivateLinkService CRs as the primary resource, and also watches // HostedControlPlane objects so that deletion of the HCP triggers reconciliation of -// the associated AzurePrivateLinkService CR. This enables the HCP finalizer to block -// HCP deletion until Azure resource cleanup is complete. +// the associated AzurePrivateLinkService CR. A separate HCP deletion reconciler handles +// legacy HCP finalizers when no AzurePrivateLinkService CRs remain. func (r *AzurePrivateLinkServiceReconciler) SetupWithManager(mgr ctrl.Manager) error { _, err := ctrl.NewControllerManagedBy(mgr). For(&hyperv1.AzurePrivateLinkService{}). @@ -205,9 +202,74 @@ func (r *AzurePrivateLinkServiceReconciler) SetupWithManager(mgr ctrl.Manager) e return fmt.Errorf("failed setting up with a controller manager: %w", err) } + _, err = ctrl.NewControllerManagedBy(mgr). + For(&hyperv1.HostedControlPlane{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { + hcp, ok := obj.(*hyperv1.HostedControlPlane) + return ok && !hcp.DeletionTimestamp.IsZero() && controllerutil.ContainsFinalizer(hcp, hcpAzurePLSFinalizerName) + }))). + WithOptions(controller.Options{ + RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](3*time.Second, 30*time.Second), + MaxConcurrentReconciles: 1, + }). + Build(&legacyHCPFinalizerReconciler{ + Client: mgr.GetClient(), + apiReader: mgr.GetAPIReader(), + }) + if err != nil { + return fmt.Errorf("failed setting up the HCP deletion controller: %w", err) + } + return nil } +func (r *legacyHCPFinalizerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + reader := r.apiReader + if reader == nil { + reader = r.Client + } + + hcp := &hyperv1.HostedControlPlane{} + if err := reader.Get(ctx, req.NamespacedName, hcp); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to get HostedControlPlane: %w", err) + } + + if hcp.DeletionTimestamp.IsZero() || !controllerutil.ContainsFinalizer(hcp, hcpAzurePLSFinalizerName) { + return ctrl.Result{}, nil + } + + var azPLSList hyperv1.AzurePrivateLinkServiceList + if err := reader.List(ctx, &azPLSList, client.InNamespace(hcp.Namespace)); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to list AzurePrivateLinkService resources: %w", err) + } + if len(azPLSList.Items) > 0 { + return ctrl.Result{}, nil + } + + if err := conditions.PatchPrivateConnectivityCleanupCondition(ctx, r.Client, hcp, metav1.ConditionTrue, hyperv1.PrivateConnectivityCleanupCompleteReason, "All Azure Private Link Service resources have been cleaned up"); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to set PrivateConnectivityCleanedUp condition: %w", err) + } + + originalHCP := hcp.DeepCopy() + controllerutil.RemoveFinalizer(hcp, hcpAzurePLSFinalizerName) + if err := r.Patch(ctx, hcp, client.MergeFromWithOptions(originalHCP, client.MergeFromWithOptimisticLock{})); err != nil { + if apierrors.IsConflict(err) { + return ctrl.Result{RequeueAfter: time.Second}, nil + } + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to remove legacy HCP finalizer: %w", err) + } + + return ctrl.Result{}, nil +} + // mapHCPToAzurePLS maps HostedControlPlane events to the AzurePrivateLinkService CRs // in the same namespace. This ensures the reconciler is triggered when an HCP is being // deleted, allowing it to perform Azure resource cleanup and remove the HCP finalizer. @@ -218,9 +280,7 @@ func (r *AzurePrivateLinkServiceReconciler) mapHCPToAzurePLS() handler.MapFunc { return nil } - // Only trigger reconciliation when the HCP has our finalizer; this avoids - // unnecessary reconciliations for HCPs that are not related to Azure PLS. - if !controllerutil.ContainsFinalizer(hcp, hcpAzurePLSFinalizerName) { + if hcp.DeletionTimestamp.IsZero() { return nil } @@ -309,12 +369,17 @@ func (r *AzurePrivateLinkServiceReconciler) Reconcile(ctx context.Context, req c return ctrl.Result{RequeueAfter: azureutil.PLSRequeueInterval}, nil } - // 7. Add HCP finalizer to block HCP deletion until Azure cleanup is done. - // This is done after the PLS alias is available, which means Azure resources - // are about to be created or already exist. Adding the finalizer at this point - // ensures we only block HCP deletion when there are actually resources to clean up. - if result, err := r.ensureHCPFinalizer(ctx, hcp, log); err != nil || !result.IsZero() { - return result, err + // 7. Migration: remove legacy HCP finalizer if present (replaced by PrivateConnectivityCleanedUp condition) + if controllerutil.ContainsFinalizer(hcp, hcpAzurePLSFinalizerName) { + originalHCP := hcp.DeepCopy() + controllerutil.RemoveFinalizer(hcp, hcpAzurePLSFinalizerName) + if err := r.Patch(ctx, hcp, client.MergeFromWithOptions(originalHCP, client.MergeFromWithOptimisticLock{})); err != nil { + if apierrors.IsConflict(err) { + return ctrl.Result{RequeueAfter: time.Second}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to remove legacy HCP finalizer: %w", err) + } + log.Info("Removed legacy HCP finalizer during migration") } // 8. Reconcile Private Endpoint @@ -381,25 +446,6 @@ func (r *AzurePrivateLinkServiceReconciler) Reconcile(ctx context.Context, req c return ctrl.Result{RequeueAfter: azureutil.DriftDetectionRequeueInterval}, nil } -// ensureHCPFinalizer adds the HCP finalizer if not already present. -// Uses optimistic locking via MergeFromWithOptimisticLock to safely update the HCP. -func (r *AzurePrivateLinkServiceReconciler) ensureHCPFinalizer(ctx context.Context, hcp *hyperv1.HostedControlPlane, log logr.Logger) (ctrl.Result, error) { - if controllerutil.ContainsFinalizer(hcp, hcpAzurePLSFinalizerName) { - return ctrl.Result{}, nil - } - - log.Info("Adding HCP finalizer for Azure PLS cleanup") - originalHCP := hcp.DeepCopy() - controllerutil.AddFinalizer(hcp, hcpAzurePLSFinalizerName) - if err := r.Patch(ctx, hcp, client.MergeFromWithOptions(originalHCP, client.MergeFromWithOptimisticLock{})); err != nil { - if apierrors.IsConflict(err) { - return ctrl.Result{RequeueAfter: time.Second}, nil - } - return ctrl.Result{}, fmt.Errorf("failed to add HCP finalizer: %w", err) - } - return ctrl.Result{}, nil -} - // getHCPOrCleanupOrphan looks up the HostedControlPlane for the given CR. If the HCP // is gone (NotFound), it removes any orphaned per-CR finalizer so the CR can be // garbage-collected with the namespace. Returns (nil, nil) when the HCP is gone and @@ -423,28 +469,29 @@ func (r *AzurePrivateLinkServiceReconciler) getHCPOrCleanupOrphan(ctx context.Co } // reconcileHCPDeletion handles HCP deletion by cleaning up Azure resources, removing -// per-CR finalizers, and removing the shared HCP finalizer. Per-CR finalizers must be -// removed here because once HCP deletion completes and HO deletes the namespace, CPO -// is terminated and can no longer process them. Stuck per-CR finalizers block namespace -// deletion, which blocks HO from removing the HC finalizer, causing a 40-minute timeout. +// per-CR finalizers, and setting the PrivateConnectivityCleanedUp condition on the HCP. +// Per-CR finalizers must be removed here because once HCP deletion completes and HO +// deletes the namespace, CPO is terminated and can no longer process them. Stuck per-CR +// finalizers block namespace deletion, which blocks HO from removing the HC finalizer, +// causing a 40-minute timeout. // // The flow is: -// 1. If the HCP does not have our finalizer, nothing to do. -// 2. Perform Azure resource cleanup for ALL CRs (PE, DNS zone, VNet link, A record). +// 1. Perform Azure resource cleanup for ALL CRs (PE, DNS zone, VNet link, A record). +// 2. Delete the shared base domain DNS zone. // 3. Remove per-CR finalizers from ALL CRs so they can be garbage-collected with the namespace. -// 4. Remove the shared HCP finalizer to unblock HCP deletion. +// 4. Remove legacy HCP finalizer if present (replaced by PrivateConnectivityCleanedUp condition). +// 5. Set PrivateConnectivityCleanedUp condition on HCP. func (r *AzurePrivateLinkServiceReconciler) reconcileHCPDeletion(ctx context.Context, azPLS *hyperv1.AzurePrivateLinkService, hcp *hyperv1.HostedControlPlane, log logr.Logger) (ctrl.Result, error) { - if !controllerutil.ContainsFinalizer(hcp, hcpAzurePLSFinalizerName) { + cleanupCondition := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + if cleanupCondition != nil && + cleanupCondition.Status == metav1.ConditionTrue && + !controllerutil.ContainsFinalizer(hcp, hcpAzurePLSFinalizerName) { + log.Info("Private connectivity cleanup already completed, skipping Azure cleanup") return ctrl.Result{}, nil } - log.Info("HCP is being deleted, cleaning up Azure resources before removing HCP finalizer") + log.Info("HCP is being deleted, cleaning up Azure resources before setting cleanup condition") - // List all AzurePrivateLinkService CRs in the namespace to ensure all are cleaned up - // before removing the shared HCP finalizer. When multiple CRs exist (e.g., private-router - // and oauth-openshift), each must complete Azure resource cleanup while HCP credentials - // are still valid. With MaxConcurrentReconciles: 1, the first CR to reconcile handles - // cleanup for all siblings in a single pass. var allPLS hyperv1.AzurePrivateLinkServiceList if err := r.List(ctx, &allPLS, client.InNamespace(azPLS.Namespace)); err != nil { return ctrl.Result{}, fmt.Errorf("failed to list AzurePrivateLinkService resources: %w", err) @@ -454,11 +501,6 @@ func (r *AzurePrivateLinkServiceReconciler) reconcileHCPDeletion(ctx context.Con return ctrl.Result{}, fmt.Errorf("failed to clean up Azure resources during HCP deletion: %w", err) } - // Delete the base domain DNS zone explicitly. When multiple CRs share the same - // base domain zone, each CR's reconcileDelete skips zone deletion because - // hasSiblingCR sees the other CR as still active (neither has DeletionTimestamp - // during HCP deletion). The per-CR cleanup above already removed all A records - // and VNet links from the zone, so it is safe to delete here. if err := r.deleteBaseDomainDNSZone(ctx, allPLS.Items, log); err != nil { return ctrl.Result{}, fmt.Errorf("failed to delete base domain DNS zone during HCP deletion: %w", err) } @@ -467,16 +509,26 @@ func (r *AzurePrivateLinkServiceReconciler) reconcileHCPDeletion(ctx context.Con return ctrl.Result{}, fmt.Errorf("failed to remove per-CR finalizers during HCP deletion: %w", err) } - // Remove the HCP finalizer to unblock HCP deletion - log.Info("Azure resource cleanup complete for all AzurePrivateLinkService CRs, removing HCP finalizer") - originalHCP := hcp.DeepCopy() - controllerutil.RemoveFinalizer(hcp, hcpAzurePLSFinalizerName) - if err := r.Patch(ctx, hcp, client.MergeFromWithOptions(originalHCP, client.MergeFromWithOptimisticLock{})); err != nil { - if apierrors.IsConflict(err) { - return ctrl.Result{RequeueAfter: time.Second}, nil + // Remove legacy HCP finalizer if present (replaced by PrivateConnectivityCleanedUp condition) + if controllerutil.ContainsFinalizer(hcp, hcpAzurePLSFinalizerName) { + originalHCP := hcp.DeepCopy() + controllerutil.RemoveFinalizer(hcp, hcpAzurePLSFinalizerName) + if err := r.Patch(ctx, hcp, client.MergeFromWithOptions(originalHCP, client.MergeFromWithOptimisticLock{})); err != nil { + if apierrors.IsConflict(err) { + return ctrl.Result{RequeueAfter: time.Second}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to remove legacy HCP finalizer: %w", err) + } + log.Info("Removed legacy HCP finalizer during deletion") + } + + if err := conditions.PatchPrivateConnectivityCleanupCondition(ctx, r.Client, hcp, metav1.ConditionTrue, hyperv1.PrivateConnectivityCleanupCompleteReason, "All Azure Private Link Service resources have been cleaned up"); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil } - return ctrl.Result{}, fmt.Errorf("failed to remove HCP finalizer: %w", err) + return ctrl.Result{}, fmt.Errorf("failed to set PrivateConnectivityCleanedUp condition: %w", err) } + log.Info("Set PrivateConnectivityCleanedUp condition on HCP") return ctrl.Result{}, nil } @@ -506,8 +558,6 @@ func (r *AzurePrivateLinkServiceReconciler) cleanupAllAzureResources(ctx context return utilerrors.NewAggregate(errs) } -// removeAllCRFinalizers removes per-CR finalizers so the CRs can be deleted -// during namespace cleanup without requiring CPO to still be running. func (r *AzurePrivateLinkServiceReconciler) removeAllCRFinalizers(ctx context.Context, items []hyperv1.AzurePrivateLinkService, log logr.Logger) error { var errs []error for i := range items { diff --git a/control-plane-operator/controllers/azureprivatelinkservice/controller_test.go b/control-plane-operator/controllers/azureprivatelinkservice/controller_test.go index 31c86720bcc7..c70c995c3d5d 100644 --- a/control-plane-operator/controllers/azureprivatelinkservice/controller_test.go +++ b/control-plane-operator/controllers/azureprivatelinkservice/controller_test.go @@ -1063,105 +1063,87 @@ func TestExtractPrivateEndpointConnectionState(t *testing.T) { } } -func TestEnsureHCPFinalizer_WhenNotPresent_ItShouldAddFinalizer(t *testing.T) { +func TestReconcile_WhenPLSAliasIsAvailable_ItShouldRemoveLegacyHCPFinalizer(t *testing.T) { t.Parallel() g := NewGomegaWithT(t) scheme := newTestScheme(t, g) + azPLS := newTestAzurePLS(t, "test-pls", "test-ns") + azPLS.Finalizers = []string{azurePrivateLinkServiceFinalizer} + azPLS.Status.PrivateLinkServiceAlias = "test-pls-alias.guid.eastus.azure.privatelinkservice" + hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") - // No finalizer set on HCP + hcp.Finalizers = []string{hcpAzurePLSFinalizerName} // Legacy finalizer present fakeClient := fake.NewClientBuilder(). WithScheme(scheme). - WithObjects(hcp). + WithObjects(azPLS, hcp). + WithStatusSubresource(azPLS). Build() - r := &AzurePrivateLinkServiceReconciler{ - Client: fakeClient, + mockPE := &mockPrivateEndpoints{ + getErr: &azcore.ResponseError{StatusCode: 404}, } - result, err := r.ensureHCPFinalizer(t.Context(), hcp, testr.New(t)) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.IsZero()).To(BeTrue(), "should return zero result after adding HCP finalizer") - - // Verify finalizer was added to HCP - updatedHCP := &hyperv1.HostedControlPlane{} - err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(updatedHCP.Finalizers).To(ContainElement(hcpAzurePLSFinalizerName), "HCP should have the Azure PLS finalizer") -} - -func TestEnsureHCPFinalizer_WhenAlreadyPresent_ItShouldNotModify(t *testing.T) { - t.Parallel() - g := NewGomegaWithT(t) - scheme := newTestScheme(t, g) - - hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") - hcp.Finalizers = []string{hcpAzurePLSFinalizerName} - - fakeClient := fake.NewClientBuilder(). - WithScheme(scheme). - WithObjects(hcp). - Build() - r := &AzurePrivateLinkServiceReconciler{ - Client: fakeClient, + Client: fakeClient, + PrivateEndpoints: mockPE, + PrivateDNSZones: &mockPrivateDNSZones{}, + VirtualNetworkLinks: &mockVirtualNetworkLinks{}, + RecordSets: &mockRecordSets{}, } - result, err := r.ensureHCPFinalizer(t.Context(), hcp, testr.New(t)) + _, err := r.Reconcile(log.IntoContext(t.Context(), testr.New(t)), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: "test-pls", Namespace: "test-ns"}, + }) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.IsZero()).To(BeTrue()) - // Verify finalizer is still present and only once + // Verify legacy HCP finalizer was removed during migration updatedHCP := &hyperv1.HostedControlPlane{} err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(updatedHCP.Finalizers).To(Equal([]string{hcpAzurePLSFinalizerName})) + g.Expect(updatedHCP.Finalizers).ToNot(ContainElement(hcpAzurePLSFinalizerName), "legacy HCP finalizer should be removed during reconciliation") } -func TestReconcile_WhenPLSAliasIsAvailable_ItShouldAddHCPFinalizer(t *testing.T) { +func TestLegacyHCPFinalizerReconcile_WhenHCPIsDeletedWithoutAzurePLSCR_ItShouldRemoveFinalizer(t *testing.T) { t.Parallel() g := NewGomegaWithT(t) scheme := newTestScheme(t, g) - azPLS := newTestAzurePLS(t, "test-pls", "test-ns") - azPLS.Finalizers = []string{azurePrivateLinkServiceFinalizer} - azPLS.Status.PrivateLinkServiceAlias = "test-pls-alias.guid.eastus.azure.privatelinkservice" - + now := metav1.Now() hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") - // No HCP finalizer yet + hcp.DeletionTimestamp = &now + hcp.Finalizers = []string{hcpAzurePLSFinalizerName, "some-finalizer-to-keep"} fakeClient := fake.NewClientBuilder(). WithScheme(scheme). - WithObjects(azPLS, hcp). - WithStatusSubresource(azPLS). + WithObjects(hcp). + WithStatusSubresource(&hyperv1.HostedControlPlane{}). Build() - mockPE := &mockPrivateEndpoints{ - getErr: &azcore.ResponseError{StatusCode: 404}, - } - - r := &AzurePrivateLinkServiceReconciler{ - Client: fakeClient, - PrivateEndpoints: mockPE, - PrivateDNSZones: &mockPrivateDNSZones{}, - VirtualNetworkLinks: &mockVirtualNetworkLinks{}, - RecordSets: &mockRecordSets{}, + r := &legacyHCPFinalizerReconciler{ + Client: fakeClient, + apiReader: fakeClient, } - _, err := r.Reconcile(log.IntoContext(t.Context(), testr.New(t)), ctrl.Request{ - NamespacedName: types.NamespacedName{Name: "test-pls", Namespace: "test-ns"}, + result, err := r.Reconcile(t.Context(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, }) g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.IsZero()).To(BeTrue()) - // Verify HCP finalizer was added updatedHCP := &hyperv1.HostedControlPlane{} err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(updatedHCP.Finalizers).To(ContainElement(hcpAzurePLSFinalizerName), "HCP should have the Azure PLS finalizer after reconciliation with PLS alias available") + g.Expect(updatedHCP.Finalizers).ToNot(ContainElement(hcpAzurePLSFinalizerName)) + g.Expect(updatedHCP.Finalizers).To(ContainElement("some-finalizer-to-keep")) + + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(condition).ToNot(BeNil()) + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) } -func TestReconcileHCPDeletion_WhenHCPIsBeingDeleted_ItShouldCleanUpAndRemoveFinalizer(t *testing.T) { +func TestReconcileHCPDeletion_WhenHCPIsBeingDeleted_ItShouldCleanUpAndRemoveCRFinalizer(t *testing.T) { t.Parallel() g := NewGomegaWithT(t) scheme := newTestScheme(t, g) @@ -1177,12 +1159,12 @@ func TestReconcileHCPDeletion_WhenHCPIsBeingDeleted_ItShouldCleanUpAndRemoveFina now := metav1.Now() hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") hcp.DeletionTimestamp = &now - hcp.Finalizers = []string{hcpAzurePLSFinalizerName} // Required for DeletionTimestamp to be respected by fake client + hcp.Finalizers = []string{"some-finalizer-to-keep"} // Required for DeletionTimestamp to be respected by fake client fakeClient := fake.NewClientBuilder(). WithScheme(scheme). WithObjects(azPLS, hcp). - WithStatusSubresource(azPLS). + WithStatusSubresource(azPLS, &hyperv1.HostedControlPlane{}). Build() mockPE := &mockPrivateEndpoints{} @@ -1211,59 +1193,73 @@ func TestReconcileHCPDeletion_WhenHCPIsBeingDeleted_ItShouldCleanUpAndRemoveFina g.Expect(mockDNS.deleteCalled).To(BeTrue(), "should attempt to delete Private DNS Zone") g.Expect(mockPE.deleteCalled).To(BeTrue(), "should attempt to delete Private Endpoint") - // Verify the HCP finalizer was removed. The fake client garbage-collects - // objects whose DeletionTimestamp is set and all finalizers are removed, - // so a NotFound error confirms the finalizer was successfully removed. + // Verify CR finalizer was removed from azPLS + updatedAzPLS := &hyperv1.AzurePrivateLinkService{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-pls", Namespace: "test-ns"}, updatedAzPLS) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(updatedAzPLS.Finalizers).ToNot(ContainElement(azurePrivateLinkServiceFinalizer), "CR finalizer should be removed after cleanup") + + // Verify PrivateConnectivityCleanedUp condition was set on HCP (only CR, so all cleaned up) updatedHCP := &hyperv1.HostedControlPlane{} err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) - if apierrors.IsNotFound(err) { - // Expected: fake client deleted the HCP because all finalizers were removed - return - } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(updatedHCP.Finalizers).ToNot(ContainElement(hcpAzurePLSFinalizerName), "HCP finalizer should be removed after cleanup") + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(condition).ToNot(BeNil(), "PrivateConnectivityCleanedUp condition should be set") + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(condition.Reason).To(Equal("CleanupComplete")) + + // A subsequent HCP event must not repeat the full batch cleanup or Azure API calls. + updatedAzPLS = &hyperv1.AzurePrivateLinkService{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-pls", Namespace: "test-ns"}, updatedAzPLS) + g.Expect(err).ToNot(HaveOccurred()) + updatedHCP = &hyperv1.HostedControlPlane{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) + g.Expect(err).ToNot(HaveOccurred()) + + result, err = r.reconcileHCPDeletion(t.Context(), updatedAzPLS, updatedHCP, testr.New(t)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.IsZero()).To(BeTrue()) + g.Expect(mockRecords.deleteCallCount).To(Equal(2), "repeated reconciliation should not delete A records again") + g.Expect(mockLinks.deleteCalled).To(BeTrue()) + g.Expect(mockDNS.deleteCalled).To(BeTrue()) + g.Expect(mockPE.deleteCalled).To(BeTrue()) } -func TestReconcileHCPDeletion_WhenHCPDoesNotHaveFinalizer_ItShouldBeNoOp(t *testing.T) { +func TestReconcileHCPDeletion_WhenCRHasNoFinalizer_ItShouldStillRunCleanupAndSetCondition(t *testing.T) { t.Parallel() g := NewGomegaWithT(t) scheme := newTestScheme(t, g) azPLS := newTestAzurePLS(t, "test-pls", "test-ns") - azPLS.Finalizers = []string{azurePrivateLinkServiceFinalizer} + // CR has no finalizer — batch cleanup still runs reconcileDelete for all CRs now := metav1.Now() hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") hcp.DeletionTimestamp = &now - hcp.Finalizers = []string{"some-other-finalizer"} // Has a different finalizer, not ours + hcp.Finalizers = []string{"some-finalizer-to-keep"} fakeClient := fake.NewClientBuilder(). WithScheme(scheme). WithObjects(azPLS, hcp). - WithStatusSubresource(azPLS). + WithStatusSubresource(azPLS, &hyperv1.HostedControlPlane{}). Build() - mockPE := &mockPrivateEndpoints{} - mockDNS := &mockPrivateDNSZones{} - - r := &AzurePrivateLinkServiceReconciler{ - Client: fakeClient, - PrivateEndpoints: mockPE, - PrivateDNSZones: mockDNS, - VirtualNetworkLinks: &mockVirtualNetworkLinks{}, - RecordSets: &mockRecordSets{}, - } + r := newTestReconciler(fakeClient) result, err := r.reconcileHCPDeletion(t.Context(), azPLS, hcp, testr.New(t)) g.Expect(err).ToNot(HaveOccurred()) g.Expect(result.IsZero()).To(BeTrue()) - // Verify no Azure cleanup was attempted - g.Expect(mockPE.deleteCalled).To(BeFalse(), "should not attempt PE deletion when HCP finalizer not present") - g.Expect(mockDNS.deleteCalled).To(BeFalse(), "should not attempt DNS deletion when HCP finalizer not present") + // Verify PrivateConnectivityCleanedUp condition was set (all CRs are cleaned up) + updatedHCP := &hyperv1.HostedControlPlane{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) + g.Expect(err).ToNot(HaveOccurred()) + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(condition).ToNot(BeNil(), "PrivateConnectivityCleanedUp condition should be set when all CRs are cleaned up") + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) } -func TestReconcileHCPDeletion_WhenAzureCleanupFails_ItShouldReturnErrorAndPreserveFinalizer(t *testing.T) { +func TestReconcileHCPDeletion_WhenAzureCleanupFails_ItShouldReturnErrorAndPreserveCRFinalizer(t *testing.T) { t.Parallel() g := NewGomegaWithT(t) scheme := newTestScheme(t, g) @@ -1279,12 +1275,12 @@ func TestReconcileHCPDeletion_WhenAzureCleanupFails_ItShouldReturnErrorAndPreser now := metav1.Now() hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") hcp.DeletionTimestamp = &now - hcp.Finalizers = []string{hcpAzurePLSFinalizerName} + hcp.Finalizers = []string{"some-finalizer-to-keep"} fakeClient := fake.NewClientBuilder(). WithScheme(scheme). WithObjects(azPLS, hcp). - WithStatusSubresource(azPLS). + WithStatusSubresource(azPLS, &hyperv1.HostedControlPlane{}). Build() // Configure mockRecordSets with a non-404 error to make reconcileDelete fail @@ -1303,11 +1299,152 @@ func TestReconcileHCPDeletion_WhenAzureCleanupFails_ItShouldReturnErrorAndPreser g.Expect(err).To(HaveOccurred(), "should return error when Azure cleanup fails") g.Expect(err).To(MatchError(ContainSubstring("failed to clean up Azure resources during HCP deletion"))) - // Verify the HCP finalizer was NOT removed + // Verify the CR finalizer was NOT removed + updatedAzPLS := &hyperv1.AzurePrivateLinkService{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-pls", Namespace: "test-ns"}, updatedAzPLS) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(updatedAzPLS.Finalizers).To(ContainElement(azurePrivateLinkServiceFinalizer), "CR finalizer should be preserved when cleanup fails") + + // Verify PrivateConnectivityCleanedUp condition was NOT set on HCP updatedHCP := &hyperv1.HostedControlPlane{} err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(updatedHCP.Finalizers).To(ContainElement(hcpAzurePLSFinalizerName), "HCP finalizer should be preserved when cleanup fails") + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(condition).To(BeNil(), "PrivateConnectivityCleanedUp condition should not be set when cleanup fails") +} + +func TestReconcileHCPDeletion_WhenCRIsBeingDeleted_ItShouldStillRunBatchCleanup(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + scheme := newTestScheme(t, g) + + now := metav1.Now() + azPLS := newTestAzurePLS(t, "test-pls", "test-ns") + azPLS.Finalizers = []string{azurePrivateLinkServiceFinalizer} + azPLS.DeletionTimestamp = &now + + hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") + hcp.DeletionTimestamp = &now + hcp.Finalizers = []string{"some-finalizer-to-keep"} + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(azPLS, hcp). + WithStatusSubresource(azPLS, &hyperv1.HostedControlPlane{}). + Build() + + r := newTestReconciler(fakeClient) + + result, err := r.reconcileHCPDeletion(t.Context(), azPLS, hcp, testr.New(t)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.IsZero()).To(BeTrue()) + + // Verify PrivateConnectivityCleanedUp condition was set (batch cleanup handles all CRs) + updatedHCP := &hyperv1.HostedControlPlane{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) + g.Expect(err).ToNot(HaveOccurred()) + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(condition).ToNot(BeNil(), "PrivateConnectivityCleanedUp condition should be set after batch cleanup") + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) +} + +func TestReconcileHCPDeletion_WhenMultipleCRs_ItShouldCleanAllAndSetCondition(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + scheme := newTestScheme(t, g) + + azPLS1 := newTestAzurePLS(t, "test-pls-1", "test-ns") + azPLS1.Finalizers = []string{azurePrivateLinkServiceFinalizer} + azPLS1.Status.PrivateEndpointID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.Network/privateEndpoints/test-pls-1-pe" + azPLS1.Status.DNSZoneName = "test-hcp.hypershift.local" + azPLS1.Status.PrivateDNSZoneID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.Network/privateDnsZones/test-hcp.hypershift.local" + + azPLS2 := newTestAzurePLS(t, "test-pls-2", "test-ns") + azPLS2.Finalizers = []string{azurePrivateLinkServiceFinalizer} + + now := metav1.Now() + hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") + hcp.DeletionTimestamp = &now + hcp.Finalizers = []string{"some-finalizer-to-keep"} + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(azPLS1, azPLS2, hcp). + WithStatusSubresource(azPLS1, azPLS2, &hyperv1.HostedControlPlane{}). + Build() + + r := newTestReconciler(fakeClient) + + // Batch cleanup handles ALL CRs in a single call + result, err := r.reconcileHCPDeletion(t.Context(), azPLS1, hcp, testr.New(t)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.IsZero()).To(BeTrue()) + + // Verify both CRs' finalizers were removed + updatedAzPLS1 := &hyperv1.AzurePrivateLinkService{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-pls-1", Namespace: "test-ns"}, updatedAzPLS1) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(updatedAzPLS1.Finalizers).ToNot(ContainElement(azurePrivateLinkServiceFinalizer), "CR1 finalizer should be removed") + + updatedAzPLS2 := &hyperv1.AzurePrivateLinkService{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-pls-2", Namespace: "test-ns"}, updatedAzPLS2) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(updatedAzPLS2.Finalizers).ToNot(ContainElement(azurePrivateLinkServiceFinalizer), "CR2 finalizer should be removed") + + // Verify PrivateConnectivityCleanedUp condition was set (all CRs cleaned in batch) + updatedHCP := &hyperv1.HostedControlPlane{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) + g.Expect(err).ToNot(HaveOccurred()) + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(condition).ToNot(BeNil(), "PrivateConnectivityCleanedUp condition should be set after batch cleanup") + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) +} + +func TestReconcileHCPDeletion_WithLegacyHCPFinalizer_ItShouldRemoveItDuringCleanup(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + scheme := newTestScheme(t, g) + + azPLS := newTestAzurePLS(t, "test-pls", "test-ns") + azPLS.Finalizers = []string{azurePrivateLinkServiceFinalizer} + azPLS.Status.PrivateEndpointID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.Network/privateEndpoints/test-pls-pe" + azPLS.Status.DNSZoneName = "test-hcp.hypershift.local" + azPLS.Status.PrivateDNSZoneID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Microsoft.Network/privateDnsZones/test-hcp.hypershift.local" + + now := metav1.Now() + hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") + hcp.DeletionTimestamp = &now + hcp.Finalizers = []string{hcpAzurePLSFinalizerName, "some-finalizer-to-keep"} // Legacy finalizer present + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(azPLS, hcp). + WithStatusSubresource(azPLS, &hyperv1.HostedControlPlane{}). + Build() + + r := &AzurePrivateLinkServiceReconciler{ + Client: fakeClient, + PrivateEndpoints: &mockPrivateEndpoints{}, + PrivateDNSZones: &mockPrivateDNSZones{}, + VirtualNetworkLinks: &mockVirtualNetworkLinks{}, + RecordSets: &mockRecordSets{}, + } + + result, err := r.reconcileHCPDeletion(t.Context(), azPLS, hcp, testr.New(t)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.IsZero()).To(BeTrue()) + + // Verify legacy HCP finalizer was removed + updatedHCP := &hyperv1.HostedControlPlane{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(updatedHCP.Finalizers).ToNot(ContainElement(hcpAzurePLSFinalizerName), "legacy HCP finalizer should be removed during cleanup") + g.Expect(updatedHCP.Finalizers).To(ContainElement("some-finalizer-to-keep"), "other finalizers should be preserved") + + // Verify PrivateConnectivityCleanedUp condition was set + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(condition).ToNot(BeNil(), "PrivateConnectivityCleanedUp condition should be set") + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) } func TestReconcile_WhenHCPIsBeingDeleted_ItShouldTriggerCleanupInsteadOfCreation(t *testing.T) { @@ -1325,12 +1462,12 @@ func TestReconcile_WhenHCPIsBeingDeleted_ItShouldTriggerCleanupInsteadOfCreation now := metav1.Now() hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") hcp.DeletionTimestamp = &now - hcp.Finalizers = []string{hcpAzurePLSFinalizerName} + hcp.Finalizers = []string{"some-finalizer-to-keep"} fakeClient := fake.NewClientBuilder(). WithScheme(scheme). WithObjects(azPLS, hcp). - WithStatusSubresource(azPLS). + WithStatusSubresource(azPLS, &hyperv1.HostedControlPlane{}). Build() mockPE := &mockPrivateEndpoints{ @@ -1364,17 +1501,19 @@ func TestReconcile_WhenHCPIsBeingDeleted_ItShouldTriggerCleanupInsteadOfCreation g.Expect(mockDNS.deleteCalled).To(BeTrue(), "should clean up DNS zone during HCP deletion") g.Expect(mockPE.deleteCalled).To(BeTrue(), "should clean up PE during HCP deletion") - // Verify the HCP finalizer was removed. The fake client garbage-collects - // objects whose DeletionTimestamp is set and all finalizers are removed, - // so a NotFound error confirms the finalizer was successfully removed. + // Verify CR finalizer was removed from azPLS + updatedAzPLS := &hyperv1.AzurePrivateLinkService{} + err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-pls", Namespace: "test-ns"}, updatedAzPLS) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(updatedAzPLS.Finalizers).ToNot(ContainElement(azurePrivateLinkServiceFinalizer), "CR finalizer should be removed after cleanup") + + // Verify PrivateConnectivityCleanedUp condition was set on HCP updatedHCP := &hyperv1.HostedControlPlane{} err = fakeClient.Get(t.Context(), types.NamespacedName{Name: "test-hcp", Namespace: "test-ns"}, updatedHCP) - if apierrors.IsNotFound(err) { - // Expected: fake client deleted the HCP because all finalizers were removed - return - } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(updatedHCP.Finalizers).ToNot(ContainElement(hcpAzurePLSFinalizerName), "HCP finalizer should be removed") + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(condition).ToNot(BeNil(), "PrivateConnectivityCleanedUp condition should be set") + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) } func newTestReconciler(fakeClient client.Client) *AzurePrivateLinkServiceReconciler { @@ -2250,10 +2389,12 @@ func TestMapHCPToAzurePLS(t *testing.T) { expectRequests int }{ { - name: "When HCP has the Azure PLS finalizer and PLS CRs exist, it should return requests for all PLS CRs", + name: "When HCP is being deleted and PLS CRs exist, it should return requests for all PLS CRs", hcp: func() *hyperv1.HostedControlPlane { + now := metav1.Now() hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") - hcp.Finalizers = []string{hcpAzurePLSFinalizerName} + hcp.DeletionTimestamp = &now + hcp.Finalizers = []string{"some-finalizer-to-keep"} return hcp }(), plsCRs: []client.Object{ @@ -2263,7 +2404,7 @@ func TestMapHCPToAzurePLS(t *testing.T) { expectRequests: 2, }, { - name: "When HCP does not have the Azure PLS finalizer, it should return no requests", + name: "When HCP is not being deleted, it should return no requests", hcp: newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com"), plsCRs: []client.Object{ newTestAzurePLS(t, "private-router", "test-ns"), @@ -2271,10 +2412,12 @@ func TestMapHCPToAzurePLS(t *testing.T) { expectRequests: 0, }, { - name: "When HCP has the finalizer but no PLS CRs exist, it should return no requests", + name: "When HCP is being deleted but no PLS CRs exist, it should return no requests", hcp: func() *hyperv1.HostedControlPlane { + now := metav1.Now() hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") - hcp.Finalizers = []string{hcpAzurePLSFinalizerName} + hcp.DeletionTimestamp = &now + hcp.Finalizers = []string{"some-finalizer-to-keep"} return hcp }(), plsCRs: []client.Object{}, @@ -3754,37 +3897,6 @@ func TestReconcile_WhenFinalizerAddConflicts_ItShouldRequeue(t *testing.T) { g.Expect(result.RequeueAfter).To(Equal(time.Second), "should requeue on conflict") } -func TestEnsureHCPFinalizer_WhenPatchConflicts_ItShouldRequeue(t *testing.T) { - t.Parallel() - g := NewGomegaWithT(t) - scheme := newTestScheme(t, g) - - hcp := newTestHCP(t, "test-hcp", "test-ns", "api.test.example.com") - // No finalizer → will try to add one - - conflictErr := apierrors.NewConflict( - hyperv1.Resource("hostedcontrolplanes"), "test-hcp", fmt.Errorf("conflict")) - - fakeClient := fake.NewClientBuilder(). - WithScheme(scheme). - WithObjects(hcp). - WithInterceptorFuncs(interceptor.Funcs{ - Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { - if _, ok := obj.(*hyperv1.HostedControlPlane); ok { - return conflictErr - } - return c.Patch(ctx, obj, patch, opts...) - }, - }). - Build() - - r := &AzurePrivateLinkServiceReconciler{Client: fakeClient} - - result, err := r.ensureHCPFinalizer(t.Context(), hcp, testr.New(t)) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.RequeueAfter).To(Equal(time.Second), "should requeue on conflict") -} - func TestReconcileHCPDeletion_WhenPatchConflicts_ItShouldRequeue(t *testing.T) { t.Parallel() g := NewGomegaWithT(t) diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 996bae9875e5..1d8a4a1b73e9 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -152,6 +152,11 @@ const ( resourceDeletionTimeout = 10 * time.Minute + // DefaultPrivateConnectivityCleanupTimeout is the default grace period for + // platform controllers to clean up private connectivity resources before CPO + // removes the HCP finalizer. + DefaultPrivateConnectivityCleanupTimeout = 10 * time.Minute + hcpReadyRequeueInterval = 1 * time.Minute hcpNotReadyRequeueInterval = 15 * time.Second @@ -161,6 +166,12 @@ const ( type HostedControlPlaneReconciler struct { client.Client + apiReader client.Reader + + // PrivateConnectivityCleanupTimeout controls how long HCP deletion waits for + // AWS PrivateLink or Azure Private Link cleanup to complete. A zero value uses + // DefaultPrivateConnectivityCleanupTimeout. + PrivateConnectivityCleanupTimeout time.Duration GVKAccessChecker component.GVKAccessChecker @@ -200,6 +211,9 @@ func (r *HostedControlPlaneReconciler) SetupWithManager(mgr ctrl.Manager, create if r.clock == nil { r.clock = clock.RealClock{} } + if r.apiReader == nil { + r.apiReader = mgr.GetAPIReader() + } r.setup(createOrUpdate) b := ctrl.NewControllerManagedBy(mgr). For(&hyperv1.HostedControlPlane{}). @@ -384,6 +398,8 @@ func (r *HostedControlPlaneReconciler) eventHandlers(scheme *runtime.Scheme, res } func (r *HostedControlPlaneReconciler) reconcileDeletion(ctx context.Context, hostedControlPlane *hyperv1.HostedControlPlane) (ctrl.Result, error) { + allCleanupDone := true + condition := metav1.Condition{ Type: string(hyperv1.AWSDefaultSecurityGroupDeleted), } @@ -423,10 +439,26 @@ func (r *HostedControlPlaneReconciler) reconcileDeletion(ctx context.Context, ho return ctrl.Result{}, fmt.Errorf("failed to ensure cloud resources are removed: %w", err) } if !done { - return ctrl.Result{RequeueAfter: time.Minute}, nil + allCleanupDone = false } } + if netutil.IsPrivateHCP(hostedControlPlane) && + (hostedControlPlane.Spec.Platform.Type == hyperv1.AWSPlatform || + hostedControlPlane.Spec.Platform.Type == hyperv1.AzurePlatform) { + done, err := r.waitForPrivateConnectivityCleanup(ctx, hostedControlPlane) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed waiting for private connectivity cleanup: %w", err) + } + if !done { + allCleanupDone = false + } + } + + if !allCleanupDone { + return ctrl.Result{RequeueAfter: time.Minute}, nil + } + if controllerutil.ContainsFinalizer(hostedControlPlane, finalizer) { originalHCP := hostedControlPlane.DeepCopy() controllerutil.RemoveFinalizer(hostedControlPlane, finalizer) @@ -437,6 +469,86 @@ func (r *HostedControlPlaneReconciler) reconcileDeletion(ctx context.Context, ho return ctrl.Result{}, nil } +// waitForPrivateConnectivityCleanup checks whether platform controllers have finished +// cleaning up private connectivity resources (AWS PrivateLink, Azure PLS). Returns done=true +// when the condition is set or the timeout has elapsed. +func (r *HostedControlPlaneReconciler) waitForPrivateConnectivityCleanup(ctx context.Context, hcp *hyperv1.HostedControlPlane) (bool, error) { + log := ctrl.LoggerFrom(ctx).WithValues("hcp", hcp.Name) + cleanupTimeout := r.privateConnectivityCleanupTimeout() + + cond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + if cond != nil { + if cond.Status == metav1.ConditionTrue { + return true, nil + } + if cond.Reason == hyperv1.PrivateConnectivityCleanupTimedOutReason { + return true, nil + } + } + + if hcp.DeletionTimestamp == nil { + return true, nil + } + + resourcesExist, err := r.privateConnectivityResourcesExist(ctx, hcp) + if err != nil { + return false, fmt.Errorf("failed to list private connectivity resources: %w", err) + } + if !resourcesExist { + log.Info("No private connectivity resources found, marking cleanup complete") + if err := conditions.PatchPrivateConnectivityCleanupCondition(ctx, r.Client, hcp, metav1.ConditionTrue, hyperv1.PrivateConnectivityCleanupCompleteReason, "No private connectivity resources found"); err != nil { + if apierrors.IsNotFound(err) { + return true, nil + } + return false, fmt.Errorf("failed to set private connectivity cleanup condition: %w", err) + } + return true, nil + } + + elapsed := r.clock.Since(hcp.DeletionTimestamp.Time) + if elapsed > cleanupTimeout { + log.Info("Private connectivity cleanup timed out, proceeding with deletion", "elapsed", elapsed, "timeout", cleanupTimeout) + if err := conditions.PatchPrivateConnectivityCleanupCondition(ctx, r.Client, hcp, metav1.ConditionFalse, hyperv1.PrivateConnectivityCleanupTimedOutReason, fmt.Sprintf("Platform controller did not signal cleanup completion within %s", cleanupTimeout)); err != nil { + return false, fmt.Errorf("failed to set private connectivity cleanup timeout condition: %w", err) + } + return true, nil + } + + log.Info("Waiting for private connectivity cleanup", "elapsed", elapsed, "timeout", cleanupTimeout) + return false, nil +} + +func (r *HostedControlPlaneReconciler) privateConnectivityCleanupTimeout() time.Duration { + if r.PrivateConnectivityCleanupTimeout > 0 { + return r.PrivateConnectivityCleanupTimeout + } + return DefaultPrivateConnectivityCleanupTimeout +} + +func (r *HostedControlPlaneReconciler) privateConnectivityResourcesExist(ctx context.Context, hcp *hyperv1.HostedControlPlane) (bool, error) { + reader := r.apiReader + if reader == nil { + reader = r.Client + } + + switch hcp.Spec.Platform.Type { + case hyperv1.AWSPlatform: + resources := &hyperv1.AWSEndpointServiceList{} + if err := reader.List(ctx, resources, client.InNamespace(hcp.Namespace)); err != nil { + return false, err + } + return len(resources.Items) > 0, nil + case hyperv1.AzurePlatform: + resources := &hyperv1.AzurePrivateLinkServiceList{} + if err := reader.List(ctx, resources, client.InNamespace(hcp.Namespace)); err != nil { + return false, err + } + return len(resources.Items) > 0, nil + default: + return true, nil + } +} + func (r *HostedControlPlaneReconciler) reconcileEtcdStatus(ctx context.Context, hostedControlPlane *hyperv1.HostedControlPlane) error { newCondition := metav1.Condition{ Type: string(hyperv1.EtcdAvailable), diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index 4ea3806ab6db..cf7bc2f6ad35 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -5237,6 +5237,267 @@ func TestValidateAzureKMSConfig(t *testing.T) { } } +func TestWaitForPrivateConnectivityCleanup(t *testing.T) { + now := metav1.Now() + tests := []struct { + name string + hcp *hyperv1.HostedControlPlane + objects []client.Object + cleanupTimeout time.Duration + wantDone bool + wantErr bool + interceptorFunc *interceptor.Funcs + // verifyCondition, when non-nil, is called after the function returns to + // verify that the expected condition was set on the HCP. + verifyCondition func(g Gomega, hcp *hyperv1.HostedControlPlane) + }{ + { + name: "When condition is True, it should return done", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + Status: hyperv1.HostedControlPlaneStatus{ + Conditions: []metav1.Condition{ + { + Type: string(hyperv1.PrivateConnectivityCleanedUp), + Status: metav1.ConditionTrue, + Reason: hyperv1.PrivateConnectivityCleanupCompleteReason, + LastTransitionTime: now, + }, + }, + }, + }, + wantDone: true, + wantErr: false, + }, + { + name: "When condition is False with CleanupTimedOut reason, it should return done", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + Status: hyperv1.HostedControlPlaneStatus{ + Conditions: []metav1.Condition{ + { + Type: string(hyperv1.PrivateConnectivityCleanedUp), + Status: metav1.ConditionFalse, + Reason: hyperv1.PrivateConnectivityCleanupTimedOutReason, + LastTransitionTime: now, + }, + }, + }, + }, + wantDone: true, + wantErr: false, + }, + { + name: "When DeletionTimestamp is nil, it should return done", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + }, + }, + wantDone: true, + wantErr: false, + }, + { + name: "When private AWS HCP has no endpoint CRs, it should return done", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{EndpointAccess: hyperv1.Private}, + }, + }, + }, + wantDone: true, + wantErr: false, + verifyCondition: func(g Gomega, hcp *hyperv1.HostedControlPlane) { + cond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(hyperv1.PrivateConnectivityCleanupCompleteReason)) + }, + }, + { + name: "When private Azure HCP has no endpoint CRs, it should return done", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AzurePlatform, + Azure: &hyperv1.AzurePlatformSpec{Topology: hyperv1.AzureTopologyPrivate}, + }, + }, + }, + wantDone: true, + wantErr: false, + verifyCondition: func(g Gomega, hcp *hyperv1.HostedControlPlane) { + cond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(cond.Reason).To(Equal(hyperv1.PrivateConnectivityCleanupCompleteReason)) + }, + }, + { + name: "When private AWS HCP has an endpoint CR, it should continue waiting", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{EndpointAccess: hyperv1.Private}, + }, + }, + }, + objects: []client.Object{ + &hyperv1.AWSEndpointService{ + ObjectMeta: metav1.ObjectMeta{Name: "private-router", Namespace: "test-ns"}, + }, + }, + wantDone: false, + wantErr: false, + }, + { + name: "When timeout has elapsed, it should set timed out condition and return done", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: ptr.To(metav1.NewTime(time.Now().Add(-15 * time.Minute))), + }, + }, + wantDone: true, + wantErr: false, + verifyCondition: func(g Gomega, hcp *hyperv1.HostedControlPlane) { + cond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(hyperv1.PrivateConnectivityCleanupTimedOutReason)) + }, + }, + { + name: "When configured timeout has elapsed, it should use the configured value", + cleanupTimeout: 2 * time.Minute, + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: ptr.To(metav1.NewTime(time.Now().Add(-3 * time.Minute))), + }, + }, + wantDone: true, + wantErr: false, + verifyCondition: func(g Gomega, hcp *hyperv1.HostedControlPlane) { + cond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.PrivateConnectivityCleanedUp)) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(hyperv1.PrivateConnectivityCleanupTimedOutReason)) + g.Expect(cond.Message).To(ContainSubstring("within 2m0s")) + }, + }, + { + name: "When waiting and not timed out, it should return not done", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: ptr.To(metav1.NewTime(time.Now().Add(-1 * time.Minute))), + }, + }, + wantDone: false, + wantErr: false, + }, + { + name: "When status patch fails during timeout handling, it should return error", + hcp: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + Finalizers: []string{finalizer}, + DeletionTimestamp: ptr.To(metav1.NewTime(time.Now().Add(-15 * time.Minute))), + }, + }, + interceptorFunc: &interceptor.Funcs{ + SubResourcePatch: func(_ context.Context, _ client.Client, _ string, _ client.Object, _ client.Patch, _ ...client.SubResourcePatchOption) error { + return fmt.Errorf("simulated status patch failure") + }, + }, + wantDone: false, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + objects := append([]client.Object{tt.hcp}, tt.objects...) + builder := fake.NewClientBuilder(). + WithScheme(api.Scheme). + WithObjects(objects...). + WithStatusSubresource(&hyperv1.HostedControlPlane{}) + if tt.interceptorFunc != nil { + builder = builder.WithInterceptorFuncs(*tt.interceptorFunc) + } + fakeClient := builder.Build() + + ctx := ctrl.LoggerInto(t.Context(), ctrl.Log.WithName("test")) + + // Re-read from fake client so the object has a ResourceVersion for OptimisticLock + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(tt.hcp), tt.hcp)).To(Succeed()) + + r := &HostedControlPlaneReconciler{ + Client: fakeClient, + Log: ctrl.Log.WithName("test"), + clock: clock.RealClock{}, + PrivateConnectivityCleanupTimeout: tt.cleanupTimeout, + } + + done, err := r.waitForPrivateConnectivityCleanup(ctx, tt.hcp) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + g.Expect(done).To(Equal(tt.wantDone)) + + if tt.verifyCondition != nil { + // Re-read from the fake client to see persisted status changes + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(tt.hcp), tt.hcp)).To(Succeed()) + tt.verifyCondition(g, tt.hcp) + } + }) + } +} + // Compile-time assertion that fakeVersionImageMetadataProvider satisfies the interface. var _ util.ImageMetadataProvider = &fakeVersionImageMetadataProvider{} diff --git a/control-plane-operator/main.go b/control-plane-operator/main.go index 6fde842dde16..cd5074f35e4a 100644 --- a/control-plane-operator/main.go +++ b/control-plane-operator/main.go @@ -323,20 +323,21 @@ func NewStartCommand() *cobra.Command { } var ( - namespace string - deploymentName string - metricsAddr string - healthProbeAddr string - cpoImage string - hostedClusterConfigOperatorImage string - socks5ProxyImage string - availabilityProberImage string - tokenMinterImage string - inCluster bool - enableCIDebugOutput bool - registryOverrides map[string]string - imageOverridesStr string - featureSet string + namespace string + deploymentName string + metricsAddr string + healthProbeAddr string + cpoImage string + hostedClusterConfigOperatorImage string + socks5ProxyImage string + availabilityProberImage string + tokenMinterImage string + inCluster bool + enableCIDebugOutput bool + registryOverrides map[string]string + imageOverridesStr string + featureSet string + privateConnectivityCleanupTimeout time.Duration ) cmd.Flags().StringVar(&namespace, "namespace", os.Getenv("MY_NAMESPACE"), "The namespace this operator lives in (required)") @@ -352,6 +353,7 @@ func NewStartCommand() *cobra.Command { "to avoid assuming access to the service network)") cmd.Flags().BoolVar(&enableCIDebugOutput, "enable-ci-debug-output", false, "If extra CI debug output should be enabled") cmd.Flags().StringToStringVar(®istryOverrides, "registry-overrides", map[string]string{}, "registry-overrides contains the source registry string as a key and the destination registry string as value. Images before being applied are scanned for the source registry string and if found the string is replaced with the destination registry string. Format is: sr1=dr1,sr2=dr2") + cmd.Flags().DurationVar(&privateConnectivityCleanupTimeout, "private-connectivity-cleanup-timeout", hostedcontrolplane.DefaultPrivateConnectivityCleanupTimeout, "How long to wait for platform private connectivity cleanup during HostedControlPlane deletion") cmd.Flags().StringVar(&imageOverridesStr, "image-overrides", "", "List of images that should be used for a hosted cluster control plane instead of images from OpenShift release specified in HostedCluster. "+ "Format is: name1=image1,name2=image2. \"nameX\" is name of an image in OpenShift release (e.g. \"cluster-network-operator\"). "+ @@ -361,6 +363,10 @@ func NewStartCommand() *cobra.Command { cmd.Run = func(cmd *cobra.Command, args []string) { setupLog.Info("Starting hypershift-controlplane-manager", "version", supportedversion.String()) ctx := ctrl.SetupSignalHandler() + if privateConnectivityCleanupTimeout <= 0 { + setupLog.Error(fmt.Errorf("timeout must be greater than zero"), "invalid private connectivity cleanup timeout", "timeout", privateConnectivityCleanupTimeout) + os.Exit(1) + } restConfig := ctrl.GetConfigOrDie() restConfig.UserAgent = "hypershift-controlplane-manager" @@ -522,6 +528,7 @@ func NewStartCommand() *cobra.Command { if err := (&hostedcontrolplane.HostedControlPlaneReconciler{ Client: mgr.GetClient(), + PrivateConnectivityCleanupTimeout: privateConnectivityCleanupTimeout, GVKAccessChecker: component.NewGVKAccessCache(mgr.GetAPIReader()), ManagementClusterCapabilities: mgmtClusterCaps, ReleaseProvider: cpReleaseProvider, diff --git a/docs/content/reference/aggregated-docs.md b/docs/content/reference/aggregated-docs.md index aceb728f2ab2..2b267db5d73e 100644 --- a/docs/content/reference/aggregated-docs.md +++ b/docs/content/reference/aggregated-docs.md @@ -46407,6 +46407,16 @@ hosted cluster can be live migrated without experiencing a node restart

desired platform are valid. A failure here is unlikely to resolve without the changing user input.

+

"PrivateConnectivityCleanedUp"

+

PrivateConnectivityCleanedUp signals whether platform private connectivity +resources (AWS PrivateLink endpoints, Azure Private Endpoints) have been +cleaned up during HostedControlPlane deletion. Set by platform controllers, +or by CPO when no matching private connectivity resources exist, and gated +with a 10-minute timeout in CPO’s deletion path. The timeout allows +HCP deletion to proceed even when cleanup is stuck (trading orphaned cloud +resources for avoiding indefinite deletion blocking), while the condition +provides visibility into whether cleanup completed successfully or timed out.

+

"PublicEndpointExposed"

PublicEndpointExposed indicates whether public API server endpoints are currently configured and exposed for this cluster via the management diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md index 3db7c7a984c8..1c6e31c8de09 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -6510,6 +6510,16 @@ hosted cluster can be live migrated without experiencing a node restart

desired platform are valid. A failure here is unlikely to resolve without the changing user input.

+

"PrivateConnectivityCleanedUp"

+

PrivateConnectivityCleanedUp signals whether platform private connectivity +resources (AWS PrivateLink endpoints, Azure Private Endpoints) have been +cleaned up during HostedControlPlane deletion. Set by platform controllers, +or by CPO when no matching private connectivity resources exist, and gated +with a 10-minute timeout in CPO’s deletion path. The timeout allows +HCP deletion to proceed even when cleanup is stuck (trading orphaned cloud +resources for avoiding indefinite deletion blocking), while the condition +provides visibility into whether cleanup completed successfully or timed out.

+

"PublicEndpointExposed"

PublicEndpointExposed indicates whether public API server endpoints are currently configured and exposed for this cluster via the management diff --git a/support/conditions/conditions.go b/support/conditions/conditions.go index c3426783c533..bc48888ca87c 100644 --- a/support/conditions/conditions.go +++ b/support/conditions/conditions.go @@ -1,14 +1,31 @@ package conditions import ( + "context" + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/support/netutil" + "github.com/openshift/hypershift/support/statuspatching" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "sigs.k8s.io/controller-runtime/pkg/client" ) +// PatchPrivateConnectivityCleanupCondition updates the HCP condition that gates +// private connectivity cleanup during HostedControlPlane deletion. +func PatchPrivateConnectivityCleanupCondition(ctx context.Context, c client.Client, hcp *hyperv1.HostedControlPlane, status metav1.ConditionStatus, reason, message string) error { + return statuspatching.PatchStatusCondition(ctx, c, hcp, &hcp.Status.Conditions, metav1.Condition{ + Type: string(hyperv1.PrivateConnectivityCleanedUp), + Status: status, + Reason: reason, + Message: message, + ObservedGeneration: hcp.Generation, + }) +} + func ExpectedHCConditions(hostedCluster *hyperv1.HostedCluster) map[hyperv1.ConditionType]metav1.ConditionStatus { conditions := map[hyperv1.ConditionType]metav1.ConditionStatus{ hyperv1.HostedClusterAvailable: metav1.ConditionTrue, diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/endpointservice_types.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/endpointservice_types.go index abb2df3893e3..0fee9824c3eb 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/endpointservice_types.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/endpointservice_types.go @@ -119,6 +119,26 @@ type AWSEndpointServiceStatus struct { // SecurityGroupID is the ID of the security group. // +optional SecurityGroupID string `json:"securityGroupID,omitempty"` + + // sharedVPCEndpointRoleARN is the persisted ARN of the SharedVPC role used + // for EC2 endpoint and security group operations during deletion. + // + // The role ARN is copied from the HostedControlPlane so cleanup can continue + // after the HCP is gone, for example after a controller restart. + // +optional + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=2048 + SharedVPCEndpointRoleARN string `json:"sharedVPCEndpointRoleARN,omitempty"` + + // sharedVPCRoute53RoleARN is the persisted ARN of the SharedVPC role used + // for Route53 operations during deletion. + // + // The role ARN is copied from the HostedControlPlane so cleanup can continue + // after the HCP is gone, for example after a controller restart. + // +optional + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=2048 + SharedVPCRoute53RoleARN string `json:"sharedVPCRoute53RoleARN,omitempty"` } // +kubebuilder:object:root=true diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.go index 9dc6fd0b1bc0..ed245ec6e767 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.go @@ -309,6 +309,26 @@ const ( HostedControlPlaneDegraded ConditionType = "Degraded" EtcdSnapshotRestored ConditionType = "EtcdSnapshotRestored" CVOScaledDown ConditionType = "CVOScaledDown" + + // PrivateConnectivityCleanedUp signals whether platform private connectivity + // resources (AWS PrivateLink endpoints, Azure Private Endpoints) have been + // cleaned up during HostedControlPlane deletion. Set by platform controllers, + // or by CPO when no matching private connectivity resources exist, and gated + // with a 10-minute timeout in CPO's deletion path. The timeout allows + // HCP deletion to proceed even when cleanup is stuck (trading orphaned cloud + // resources for avoiding indefinite deletion blocking), while the condition + // provides visibility into whether cleanup completed successfully or timed out. + PrivateConnectivityCleanedUp ConditionType = "PrivateConnectivityCleanedUp" + + // PrivateConnectivityCleanupCompleteReason is set when platform controllers + // have finished cleaning up all private connectivity resources. + PrivateConnectivityCleanupCompleteReason = "CleanupComplete" + + // PrivateConnectivityCleanupTimedOutReason is set when the cleanup timeout + // elapsed before the platform controller signaled completion. When this occurs, + // cloud resources (endpoints, DNS zones, security groups) may be orphaned and + // require manual cleanup. + PrivateConnectivityCleanupTimedOutReason = "CleanupTimedOut" ) // HostedControlPlaneStatus defines the observed state of HostedControlPlane diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go index 2a00effd35d0..9cbca182658e 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go @@ -70,6 +70,8 @@ const ( // ClusterVersionRetrievedUpdates bubbles up RetrievedUpdates from the CVO. ClusterVersionRetrievedUpdates ConditionType = "ClusterVersionRetrievedUpdates" + // HCP-only conditions (not propagated to HostedCluster). + // UnmanagedEtcdAvailable indicates whether a user-managed etcd cluster is // healthy. UnmanagedEtcdAvailable ConditionType = "UnmanagedEtcdAvailable"