Skip to content

fix(controllers): requeue on gate conditions instead of relying on watch events #129

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

Merged
openshift-merge-bot merged 3 commits into from
Aug 18, 2026
Merged

fix(controllers): requeue on gate conditions instead of relying on watch events #129

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
0160673
fix(nodepool): requeue on gate conditions instead of relying on watch…
cristianoveiga Aug 18, 2026
d749122
fix(hc): requeue on gate conditions instead of relying on watch events
cristianoveiga Aug 18, 2026
dd4e783
fix(controllers): move requeue log messages after status update
cristianoveiga Aug 18, 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
14 changes: 7 additions & 7 deletions controllers/hc/hc_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,38 +74,38 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco

// Check placement readiness.
if cluster.Status.PlacementResult == nil || cluster.Status.PlacementResult.ManagementClusterName == "" {
log.Infof(ctx, "placement not ready, waiting for next event")
if r.setWaitingConditions(&cluster, "PlacementNotReady", "Waiting for placement to select a management cluster") {
if err := r.client.Status().Update(ctx, &cluster); err != nil && !apierrors.IsConflict(err) {
return reconcile.Result{}, fmt.Errorf("%s: update cluster status: %w", adapterName, err)
}
}
return reconcile.Result{}, nil
log.Infof(ctx, "placement not ready, requeueing after %s", requeuePending)
return reconcile.Result{RequeueAfter: requeuePending}, nil
}

// Check version-resolution readiness.
if cluster.Status.VersionResolution == nil {
log.Infof(ctx, "version resolution not ready, waiting for next event")
if r.setWaitingConditions(&cluster, "VersionResolutionNotReady", "Waiting for version resolution") {
if err := r.client.Status().Update(ctx, &cluster); err != nil && !apierrors.IsConflict(err) {
return reconcile.Result{}, fmt.Errorf("%s: update cluster status: %w", adapterName, err)
}
}
return reconcile.Result{}, nil
log.Infof(ctx, "version resolution not ready, requeueing after %s", requeuePending)
return reconcile.Result{RequeueAfter: requeuePending}, nil
}

// Check version match.
if cluster.Status.VersionResolution.ReleaseVersion != cluster.Spec.Release.Version {
log.Infof(ctx, "vr version %q does not match spec version %q, waiting for next event",
cluster.Status.VersionResolution.ReleaseVersion, cluster.Spec.Release.Version)
msg := fmt.Sprintf("VR version %q does not match spec version %q",
cluster.Status.VersionResolution.ReleaseVersion, cluster.Spec.Release.Version)
if r.setWaitingConditions(&cluster, "VersionMismatch", msg) {
if err := r.client.Status().Update(ctx, &cluster); err != nil && !apierrors.IsConflict(err) {
return reconcile.Result{}, fmt.Errorf("%s: update cluster status: %w", adapterName, err)
}
}
return reconcile.Result{}, nil
log.Infof(ctx, "vr version %q does not match spec version %q, requeueing after %s",
cluster.Status.VersionResolution.ReleaseVersion, cluster.Spec.Release.Version, requeuePending)
return reconcile.Result{RequeueAfter: requeuePending}, nil
}

placement := cluster.Status.PlacementResult
Expand Down
12 changes: 6 additions & 6 deletions controllers/hc/hc_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func TestReconcile_DependenciesNotReady_NoPlacement(t *testing.T) {

result, err := r.Reconcile(context.Background(), clusterReq(clusterID))
require.NoError(t, err)
require.Equal(t, time.Duration(0), result.RequeueAfter)
require.Equal(t, 15*time.Second, result.RequeueAfter, "should requeue while placement is not ready")
require.Empty(t, tr.ApplyCalls)
}

Expand All @@ -267,7 +267,7 @@ func TestReconcile_PlacementNotReady_StatusUpdateConflict(t *testing.T) {

result, err := r.Reconcile(context.Background(), clusterReq(clusterID))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, 15*time.Second, result.RequeueAfter)
require.True(t, storeClient.statusWriter.called, "expected Status.Update to be called")
require.Empty(t, tr.ApplyCalls)
}
Expand Down Expand Up @@ -308,7 +308,7 @@ func TestReconcile_VRNil_SetsWaitingConditions(t *testing.T) {

result, err := r.Reconcile(context.Background(), clusterReq(clusterID))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, 15*time.Second, result.RequeueAfter, "should requeue while VR is not ready")
require.True(t, storeClient.statusWriter.called)
require.Empty(t, tr.ApplyCalls)
}
Expand All @@ -328,7 +328,7 @@ func TestReconcile_VRNil_StatusUpdateConflict(t *testing.T) {

result, err := r.Reconcile(context.Background(), clusterReq(clusterID))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, 15*time.Second, result.RequeueAfter)
require.True(t, storeClient.statusWriter.called)
}

Expand Down Expand Up @@ -363,7 +363,7 @@ func TestReconcile_DependenciesNotReady_VRVersionMismatch(t *testing.T) {

result, err := r.Reconcile(context.Background(), clusterReq(clusterID))
require.NoError(t, err)
require.Equal(t, time.Duration(0), result.RequeueAfter)
require.Equal(t, 15*time.Second, result.RequeueAfter, "should requeue while VR version mismatches")
require.Empty(t, tr.ApplyCalls)
}

Expand All @@ -379,7 +379,7 @@ func TestReconcile_VRVersionMismatch_StatusUpdateConflict(t *testing.T) {

result, err := r.Reconcile(context.Background(), clusterReq(clusterID))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, 15*time.Second, result.RequeueAfter)
require.True(t, storeClient.statusWriter.called)
}

Expand Down
18 changes: 9 additions & 9 deletions controllers/nodepool/nodepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,49 +95,49 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco

// Gate: cluster placement must be ready.
if cluster.Status.PlacementResult == nil || cluster.Status.PlacementResult.ManagementClusterName == "" {
log.Infof(ctx, "placement not ready for nodepool %s, waiting for next event", nodepoolID)
if setWaitingNPConditions(&np, "PlacementNotReady", "Waiting for cluster placement to select a management cluster") {
if err := r.client.Status().Update(ctx, &np); err != nil && !apierrors.IsConflict(err) {
return reconcile.Result{}, fmt.Errorf("nodepool reconciler: update nodepool status: %w", err)
}
}
return reconcile.Result{}, nil
log.Infof(ctx, "placement not ready for nodepool %s, requeueing after %s", nodepoolID, requeuePending)
return reconcile.Result{RequeueAfter: requeuePending}, nil
}

// Gate: HC must be Available (HostedClusterAvailable condition on the cluster).
if !meta.IsStatusConditionTrue(cluster.Status.Conditions, "HostedClusterAvailable") {
log.Infof(ctx, "hc not available for nodepool %s, waiting for next event", nodepoolID)
if setWaitingNPConditions(&np, "HostedClusterNotAvailable", "Waiting for HostedCluster to become available") {
if err := r.client.Status().Update(ctx, &np); err != nil && !apierrors.IsConflict(err) {
return reconcile.Result{}, fmt.Errorf("nodepool reconciler: update nodepool status: %w", err)
}
}
return reconcile.Result{}, nil
log.Infof(ctx, "hc not available for nodepool %s, requeueing after %s", nodepoolID, requeuePending)
return reconcile.Result{RequeueAfter: requeuePending}, nil
}

// Gate: nodepool VR must be ready.
if np.Status.VersionResolution == nil || np.Status.VersionResolution.ReleaseVersion == "" {
log.Infof(ctx, "nodepool VR not ready for nodepool %s, waiting for next event", nodepoolID)
if setWaitingNPConditions(&np, "VersionResolutionNotReady", "Waiting for nodepool version resolution") {
if err := r.client.Status().Update(ctx, &np); err != nil && !apierrors.IsConflict(err) {
return reconcile.Result{}, fmt.Errorf("nodepool reconciler: update nodepool status: %w", err)
}
}
return reconcile.Result{}, nil
log.Infof(ctx, "nodepool VR not ready for nodepool %s, requeueing after %s", nodepoolID, requeuePending)
return reconcile.Result{RequeueAfter: requeuePending}, nil
}

// Gate: VR version must match spec version.
if np.Status.VersionResolution.ReleaseVersion != np.Spec.Release.Version {
log.Infof(ctx, "nodepool VR version %q does not match spec version %q, waiting for next event",
np.Status.VersionResolution.ReleaseVersion, np.Spec.Release.Version)
msg := fmt.Sprintf("VR version %q does not match spec version %q",
np.Status.VersionResolution.ReleaseVersion, np.Spec.Release.Version)
if setWaitingNPConditions(&np, "VersionMismatch", msg) {
if err := r.client.Status().Update(ctx, &np); err != nil && !apierrors.IsConflict(err) {
return reconcile.Result{}, fmt.Errorf("nodepool reconciler: update nodepool status: %w", err)
}
}
return reconcile.Result{}, nil
log.Infof(ctx, "nodepool VR version %q does not match spec version %q, requeueing after %s",
np.Status.VersionResolution.ReleaseVersion, np.Spec.Release.Version, requeuePending)
return reconcile.Result{RequeueAfter: requeuePending}, nil
}

// Extract nodepool GCP platform fields.
Expand Down
16 changes: 8 additions & 8 deletions controllers/nodepool/nodepool_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func TestReconcile_NoPlacement(t *testing.T) {

result, err := r.Reconcile(context.Background(), npReq("cluster-test", "np-test"))
require.NoError(t, err)
require.Equal(t, time.Duration(0), result.RequeueAfter)
require.Equal(t, requeuePending, result.RequeueAfter, "should requeue while placement is not ready")
require.Empty(t, tr.ApplyCalls)
}

Expand All @@ -311,7 +311,7 @@ func TestReconcile_NoPlacement_StatusUpdateConflict(t *testing.T) {

result, err := r.Reconcile(context.Background(), npReq("cluster-test", "np-test"))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, requeuePending, result.RequeueAfter)
require.True(t, storeClient.statusWriter.called)
}

Expand Down Expand Up @@ -340,7 +340,7 @@ func TestReconcile_HCNotAvailable(t *testing.T) {

result, err := r.Reconcile(context.Background(), npReq("cluster-test", "np-test"))
require.NoError(t, err)
require.Equal(t, time.Duration(0), result.RequeueAfter)
require.Equal(t, requeuePending, result.RequeueAfter, "should requeue while HC is not available")
require.Empty(t, tr.ApplyCalls)
}

Expand All @@ -353,7 +353,7 @@ func TestReconcile_HCNotAvailable_StatusUpdateConflict(t *testing.T) {

result, err := r.Reconcile(context.Background(), npReq("cluster-test", "np-test"))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, requeuePending, result.RequeueAfter)
require.True(t, storeClient.statusWriter.called)
}

Expand Down Expand Up @@ -382,7 +382,7 @@ func TestReconcile_NodePoolVRNotReady(t *testing.T) {

result, err := r.Reconcile(context.Background(), npReq("cluster-test", "np-test"))
require.NoError(t, err)
require.Equal(t, time.Duration(0), result.RequeueAfter)
require.Equal(t, requeuePending, result.RequeueAfter, "should requeue while VR is not ready")
require.Empty(t, tr.ApplyCalls)
}

Expand All @@ -395,7 +395,7 @@ func TestReconcile_VRNotReady_StatusUpdateConflict(t *testing.T) {

result, err := r.Reconcile(context.Background(), npReq("cluster-test", "np-test"))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, requeuePending, result.RequeueAfter)
require.True(t, storeClient.statusWriter.called)
}

Expand All @@ -421,7 +421,7 @@ func TestReconcile_VRVersionMismatch(t *testing.T) {

result, err := r.Reconcile(context.Background(), npReq("cluster-test", "np-test"))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, requeuePending, result.RequeueAfter, "should requeue while VR version mismatches")
require.Empty(t, tr.ApplyCalls)
}

Expand All @@ -435,7 +435,7 @@ func TestReconcile_VRVersionMismatch_StatusUpdateConflict(t *testing.T) {

result, err := r.Reconcile(context.Background(), npReq("cluster-test", "np-test"))
require.NoError(t, err)
require.Zero(t, result.RequeueAfter)
require.Equal(t, requeuePending, result.RequeueAfter)
require.True(t, storeClient.statusWriter.called)
}

Expand Down