Skip to content

*: proposed follow-ups after cluster deletion investigation #8422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
stevekuznetsov wants to merge 3 commits into from
Closed

*: proposed follow-ups after cluster deletion investigation #8422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e22cc1
fix: control-plane-operator: signal that we're giving up on deletion
stevekuznetsov May 4, 2026
7931c9f
feat: hypershift-operator: emit deletion progress condition
stevekuznetsov May 5, 2026
debced8
chore: *: remove low-quality logs
stevekuznetsov May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions api/hypershift/v1beta1/hostedcluster_conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,12 @@ const (
// **False / AutoNodeProgressing** means AutoNode is being enabled or disabled — the operation is in progress.
// **False / AutoNodeNotConfigured** means AutoNode is not configured in the spec and all Karpenter components have been removed.
AutoNodeEnabled ConditionType = "AutoNodeEnabled"

// HostedClusterDeleting indicates whether the HostedCluster is being deleted and
// provides first-class visibility into which phase of deletion the cluster is in.
// **False / AsExpected** means the cluster is not being deleted.
// **True** means deletion is in progress; the Reason and Message indicate the current phase.
HostedClusterDeleting ConditionType = "HostedClusterDeleting"
)

// Reasons.
Expand Down Expand Up @@ -315,6 +321,8 @@ const (

CloudResourcesCleanupSkippedReason = "CloudResourcesCleanupSkipped"

CloudResourcesDeletionTimedOutReason = "CloudResourcesDeletionTimedOut"

DataPlaneConnectionNoKonnectivityAgentPodsNotFoundReason = "KonnectivityAgentPodsNotFound"

DataPlaneConnectionLogsAccessFailedReason = "LogsAccessFailed"
Expand All @@ -334,6 +342,15 @@ const (
AutoNodeNotConfiguredReason = "AutoNodeNotConfigured"
AutoNodeProgressingReason = "AutoNodeProgressing"
AutoNodeEvaluationFailedReason = "AutoNodeEvaluationFailed"

// HostedClusterDeleting reasons.
DeletionWaitingForNodePoolDeletionReason = "WaitingForNodePoolDeletion"
DeletionWaitingForCAPIClusterDeletionReason = "WaitingForCAPIClusterDeletion"
DeletionWaitingForEndpointServiceDeletionReason = "WaitingForEndpointServiceDeletion"
DeletionWaitingForPrivateConnectDeletionReason = "WaitingForPrivateConnectDeletion"
DeletionWaitingForControlPlaneDeletionReason = "WaitingForControlPlaneDeletion"
DeletionWaitingForNamespaceDeletionReason = "WaitingForNamespaceDeletion"
DeletionCompletedReason = "DeletionCompleted"
)

// Messages.
Expand Down
5 changes: 0 additions & 5 deletions control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ func (r *PrivateServiceObserver) SetupWithManager(ctx context.Context, mgr ctrl.
}

func (r *PrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
r.log.Info("reconciling")

// Fetch the Service
svc, err := r.clientset.CoreV1().Services(req.Namespace).Get(ctx, req.Name, metav1.GetOptions{})
Expand Down Expand Up @@ -181,7 +180,6 @@ func (r *PrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Request
}); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile AWSEndpointService: %w", err)
}
r.log.Info("reconcile complete", "request", req)
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -412,8 +410,6 @@ func (r *AWSEndpointServiceReconciler) Reconcile(ctx context.Context, req ctrl.R
return ctrl.Result{}, fmt.Errorf("logger not found: %w", err)
}

log.Info("reconciling")

// Fetch the AWSEndpointService
obj := &hyperv1.AWSEndpointService{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -548,7 +544,6 @@ func (r *AWSEndpointServiceReconciler) Reconcile(ctx context.Context, req ctrl.R
}
}

log.Info("reconciliation complete")
// always requeue to catch and report out of band changes in AWS
// NOTICE: if the RequeueAfter interval is short enough, it could result in hitting some AWS request limits.
return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil
Expand Down
3 changes: 0 additions & 3 deletions control-plane-operator/controllers/gcpprivateserviceconnect/observer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ func (r *GCPPrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, nil
}

r.log.Info("reconciling")

// Fetch the Service
svc := &corev1.Service{}
if err := r.Get(ctx, req.NamespacedName, svc); err != nil {
Expand Down Expand Up @@ -114,7 +112,6 @@ func (r *GCPPrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, fmt.Errorf("failed to reconcile GCPPrivateServiceConnect: %w", err)
}

r.log.Info("reconcile complete", "request", req, "loadBalancerIP", loadBalancerIP)
return ctrl.Result{}, nil
}

Expand Down
18 changes: 16 additions & 2 deletions control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2431,9 +2431,10 @@ func (r *HostedControlPlaneReconciler) removeCloudResources(ctx context.Context,
return true, nil
}

// check if cleanup has been skipped
// check if cleanup has been skipped or timed out
if resourcesDestroyedCond != nil && resourcesDestroyedCond.Status == metav1.ConditionFalse &&
resourcesDestroyedCond.Reason == string(hyperv1.CloudResourcesCleanupSkippedReason) {
(resourcesDestroyedCond.Reason == string(hyperv1.CloudResourcesCleanupSkippedReason) ||
resourcesDestroyedCond.Reason == string(hyperv1.CloudResourcesDeletionTimedOutReason)) {
log.Info("Cleanup has been skipped", "reason", resourcesDestroyedCond.Message)
return true, nil
Comment on lines +2434 to 2439
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make terminal-state log message accurate for timeout path.

This branch now handles both skipped and timed-out cleanup, but the log still says cleanup was skipped, which can mislead triage.

Suggested patch 
-		log.Info("Cleanup has been skipped", "reason", resourcesDestroyedCond.Message)
+		log.Info("Cloud resource cleanup reached terminal state",
+			"reason", resourcesDestroyedCond.Reason,
+			"message", resourcesDestroyedCond.Message)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go`
around lines 2434 - 2439, The log message in hostedcontrolplane_controller.go
inside the condition checking resourcesDestroyedCond (where Status ==
metav1.ConditionFalse and Reason is CloudResourcesCleanupSkippedReason or
CloudResourcesDeletionTimedOutReason) is always "Cleanup has been skipped" which
is incorrect for the timeout case; update the log in that block (the code around
resourcesDestroyedCond handling in the Reconcile function) to inspect
resourcesDestroyedCond.Reason and log a precise terminal message such as
"Cleanup has been skipped" when Reason == CloudResourcesCleanupSkippedReason and
"Cleanup timed out" (or similar) when Reason ==
CloudResourcesDeletionTimedOutReason, ensuring you still include
resourcesDestroyedCond.Message for context.

}
Expand All @@ -2453,6 +2454,19 @@ func (r *HostedControlPlaneReconciler) removeCloudResources(ctx context.Context,

if timeElapsed > resourceDeletionTimeout {
log.Info("Giving up on resource deletion after timeout", "timeElapsed", duration.ShortHumanDuration(timeElapsed))
message := fmt.Sprintf("Giving up on cloud resource deletion after %s", duration.ShortHumanDuration(timeElapsed))
if resourcesDestroyedCond != nil && resourcesDestroyedCond.Message != "" {
message = fmt.Sprintf("%s (last status: %s)", message, resourcesDestroyedCond.Message)
}
meta.SetStatusCondition(&hcp.Status.Conditions, metav1.Condition{
Type: string(hyperv1.CloudResourcesDestroyed),
Status: metav1.ConditionFalse,
Reason: string(hyperv1.CloudResourcesDeletionTimedOutReason),
Message: message,
})
if err := r.Status().Update(ctx, hcp); err != nil {
return false, fmt.Errorf("failed to update cloud resources destroyed condition: %w", err)
}
return true, nil
}
return false, nil
Expand Down
1 change: 0 additions & 1 deletion control-plane-operator/hostedclusterconfigoperator/controllers/drainer/drainer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ type Reconciler struct {

func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling")

node := &corev1.Node{}
err := r.guestClusterClient.Get(ctx, req.NamespacedName, node)
Expand Down
5 changes: 0 additions & 5 deletions control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ type Reconciler struct {
}

func (r *Reconciler) Reconcile(ctx context.Context, req crreconcile.Request) (crreconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("reconciling global pull secret")

// Reconcile GlobalPullSecret
if err := r.reconcileGlobalPullSecret(ctx); err != nil {
Expand Down Expand Up @@ -77,7 +75,6 @@ func (r *Reconciler) reconcileGlobalPullSecret(ctx context.Context) error {
ok bool
)
log := ctrl.LoggerFrom(ctx)
log.Info("reconciling global pull secret")

// Create ServiceAccount for global-pull-secret-syncer
serviceAccount := manifests.GlobalPullSecretServiceAccount()
Expand Down Expand Up @@ -180,8 +177,6 @@ func (r *Reconciler) reconcileGlobalPullSecret(ctx context.Context) error {
}

func reconcileDaemonSet(ctx context.Context, daemonSet *appsv1.DaemonSet, globalPullSecretName string, originalPullSecretName string, configSeed string, c crclient.Client, createOrUpdate upsert.CreateOrUpdateFN, hccoImage string) error {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling global pull secret daemon set")

if _, err := createOrUpdate(ctx, c, daemonSet, func() error {
daemonSet.Spec = appsv1.DaemonSetSpec{
Expand Down
1 change: 0 additions & 1 deletion control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ func findClusterOperatorStatusCondition(conditions []configv1.ClusterOperatorSta

func (h *hcpStatusReconciler) reconcile(ctx context.Context, hcp *hyperv1.HostedControlPlane) error {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling hosted cluster version conditions")

var clusterVersion configv1.ClusterVersion
err := h.hostedClusterClient.Get(ctx, crclient.ObjectKey{Name: "version"}, &clusterVersion)
Expand Down
1 change: 0 additions & 1 deletion ...plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ type Reconciler struct {

func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling")

// Fetch the MachineSet.
machineSet := &capiv1.MachineSet{}
Expand Down
1 change: 0 additions & 1 deletion control-plane-operator/hostedclusterconfigoperator/controllers/machine/machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const (

func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling")

hcp := &hyperv1.HostedControlPlane{}
if err := r.client.Get(ctx, r.hcpKey, hcp); err != nil {
Expand Down
1 change: 0 additions & 1 deletion control-plane-operator/hostedclusterconfigoperator/controllers/node/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type reconciler struct {

func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling")

node := &corev1.Node{}
if err := r.guestClusterClient.Get(ctx, req.NamespacedName, node); err != nil {
Expand Down
1 change: 0 additions & 1 deletion control-plane-operator/hostedclusterconfigoperator/controllers/nodecount/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ type reconciler struct {

func (r *reconciler) Reconcile(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling")

var hcp hypershiftv1beta1.HostedControlPlane
if err := r.lister.Get(ctx, client.ObjectKey{
Expand Down
4 changes: 0 additions & 4 deletions ...plane-operator/hostedclusterconfigoperator/controllers/resources/kas/admissionpolicies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand Down Expand Up @@ -61,9 +60,6 @@ var (
// ReconcileKASValidatingAdmissionPolicies will create ValidatingAdmissionPolicies which block certain resources
// from being updated/deleted from the DataPlane side.
func ReconcileKASValidatingAdmissionPolicies(ctx context.Context, hcp *hyperv1.HostedControlPlane, client client.Client, createOrUpdate upsert.CreateOrUpdateFN) error {
log := ctrl.LoggerFrom(ctx)
log.Info("reconciling validating admission policies")

if err := reconcileConfigValidatingAdmissionPolicy(ctx, hcp, client, createOrUpdate); err != nil {
return fmt.Errorf("failed to reconcile Config Validating Admission Policy: %v", err)
}
Expand Down
3 changes: 0 additions & 3 deletions ...-operator/hostedclusterconfigoperator/controllers/resources/registry/admissionpolicies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ var (
)

func ReconcileRegistryConfigValidatingAdmissionPolicies(ctx context.Context, hcp *hyperv1.HostedControlPlane, client client.Client, createOrUpdate upsert.CreateOrUpdateFN) error {
log := ctrl.LoggerFrom(ctx)
log.Info("reconciling image registry config validating admission policies")

if err := reconcileRegistryConfigManagementStateValidatingAdmissionPolicy(ctx, hcp, client, createOrUpdate); err != nil {
return fmt.Errorf("failed to reconcile ManagementState Validating Admission Policy: %v", err)
}
Expand Down
1 change: 0 additions & 1 deletion control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,6 @@ func Setup(ctx context.Context, opts *operator.HostedClusterConfigOperatorConfig

func (r *reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling")

hcp := manifests.HostedControlPlane(r.hcpNamespace, r.hcpName)
if err := r.cpClient.Get(ctx, client.ObjectKeyFromObject(hcp), hcp); err != nil {
Expand Down
6 changes: 6 additions & 0 deletions docs/content/reference/aggregated-docs.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions docs/content/reference/api.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading