Skip to content

Commit 3fa8925

Browse files
committed
Add AsyncOperationInProgress to ExternalObservation
When AsyncOperationInProgress is true, the managed reconciler sets Synced=False with reason ReconcilePending instead of ReconcileSuccess. This prevents a false Synced=True signal during long-running async operations (e.g. MSK cluster instance type changes). Fixes #941
1 parent 3c37a9a commit 3fa8925

5 files changed

Lines changed: 128 additions & 2 deletions

File tree

apis/common/condition.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ const (
6363
ReasonReconcileSuccess ConditionReason = "ReconcileSuccess"
6464
ReasonReconcileError ConditionReason = "ReconcileError"
6565
ReasonReconcilePaused ConditionReason = "ReconcilePaused"
66+
ReasonReconcilePending ConditionReason = "ReconcilePending"
6667
)
6768

6869
// See https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties
@@ -312,3 +313,16 @@ func ReconcilePaused() Condition {
312313
Reason: ReasonReconcilePaused,
313314
}
314315
}
316+
317+
// ReconcilePending returns a condition indicating that reconciliation is
318+
// deferred pending an in-flight async operation. Unlike ReconcileError, this
319+
// does not trigger exponential backoff.
320+
func ReconcilePending(msg string) Condition {
321+
return Condition{
322+
Type: TypeSynced,
323+
Status: corev1.ConditionFalse,
324+
LastTransitionTime: metav1.Now(),
325+
Reason: ReasonReconcilePending,
326+
Message: msg,
327+
}
328+
}

apis/common/v1/condition.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const (
6060
ReasonReconcileSuccess = common.ReasonReconcileSuccess
6161
ReasonReconcileError = common.ReasonReconcileError
6262
ReasonReconcilePaused = common.ReasonReconcilePaused
63+
ReasonReconcilePending = common.ReasonReconcilePending
6364
)
6465

6566
// See https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties
@@ -134,3 +135,9 @@ func ReconcileError(err error) Condition {
134135
func ReconcilePaused() Condition {
135136
return common.ReconcilePaused()
136137
}
138+
139+
// ReconcilePending returns a condition indicating that reconciliation is
140+
// deferred pending an in-flight async operation.
141+
func ReconcilePending(msg string) Condition {
142+
return common.ReconcilePending(msg)
143+
}

pkg/reconciler/managed/reconciler.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,15 @@ type ExternalObservation struct {
526526
// finding where the observed diverges from the desired state.
527527
// The string should be a cmp.Diff that details the difference.
528528
Diff string
529+
530+
// AsyncOperationInProgress indicates that an asynchronous operation
531+
// (e.g. a long-running cloud API call) is currently in progress for
532+
// this resource. When true, the managed reconciler will set
533+
// Synced=False with reason ReconcilePending instead of
534+
// ReconcileSuccess, and will not call Update(). This allows providers
535+
// to suppress both Update() and the false ReconcileSuccess signal
536+
// during async operations.
537+
AsyncOperationInProgress bool
529538
}
530539

531540
// An ExternalCreation is the result of the creation of an external resource.
@@ -1436,8 +1445,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
14361445
// https://github.com/crossplane/crossplane/issues/289
14371446
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval)
14381447
log.Debug("External resource is up to date", "requeue-after", time.Now().Add(reconcileAfter))
1439-
status.MarkConditions(xpv1.ReconcileSuccess())
1440-
r.metricRecorder.recordFirstTimeReady(managed)
1448+
1449+
if observation.AsyncOperationInProgress {
1450+
log.Debug("Async operation in progress, setting ReconcilePending")
1451+
status.MarkConditions(xpv1.ReconcilePending("Async operation in progress"))
1452+
} else {
1453+
status.MarkConditions(xpv1.ReconcileSuccess())
1454+
r.metricRecorder.recordFirstTimeReady(managed)
1455+
}
14411456

14421457
// record that we intentionally did not update the managed resource
14431458
// because no drift was detected. We call this so late in the reconcile

pkg/reconciler/managed/reconciler_legacy_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,51 @@ func TestReconciler(t *testing.T) {
11211121
},
11221122
want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}},
11231123
},
1124+
"ExternalResourceUpToDateAsyncOperationInProgress": {
1125+
reason: "When an async operation is in progress, Synced should be set to ReconcilePending instead of ReconcileSuccess.",
1126+
args: args{
1127+
m: &fake.Manager{
1128+
Client: &test.MockClient{
1129+
MockGet: legacyManagedMockGetFn(nil, 42),
1130+
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
1131+
want := newLegacyManaged(42)
1132+
want.SetConditions(xpv1.ReconcilePending("Async operation in progress").WithObservedGeneration(42))
1133+
1134+
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
1135+
reason := "An async-in-progress reconcile should set ReconcilePending, not ReconcileSuccess."
1136+
t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff)
1137+
}
1138+
1139+
return nil
1140+
}),
1141+
},
1142+
Scheme: fake.SchemeWith(&fake.LegacyManaged{}),
1143+
},
1144+
mg: resource.ManagedKind(fake.GVK(&fake.LegacyManaged{})),
1145+
o: []ReconcilerOption{
1146+
WithInitializers(),
1147+
WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })),
1148+
WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) {
1149+
c := &ExternalClientFns{
1150+
ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) {
1151+
return ExternalObservation{
1152+
ResourceExists: true,
1153+
ResourceUpToDate: true,
1154+
AsyncOperationInProgress: true,
1155+
}, nil
1156+
},
1157+
DisconnectFn: func(_ context.Context) error {
1158+
return nil
1159+
},
1160+
}
1161+
1162+
return c, nil
1163+
})),
1164+
WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}),
1165+
},
1166+
},
1167+
want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}},
1168+
},
11241169
"ExternalResourceUpToDateWithJitter": {
11251170
reason: "When the external resource exists and is up to date a requeue should be triggered after a long wait with jitter added.",
11261171
args: args{

pkg/reconciler/managed/reconciler_modern_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,51 @@ func TestModernReconciler(t *testing.T) {
11271127
},
11281128
want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}},
11291129
},
1130+
"ExternalResourceUpToDateAsyncOperationInProgress": {
1131+
reason: "When an async operation is in progress, Synced should be set to ReconcilePending instead of ReconcileSuccess.",
1132+
args: args{
1133+
m: &fake.Manager{
1134+
Client: &test.MockClient{
1135+
MockGet: modernManagedMockGetFn(nil, 42),
1136+
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
1137+
want := newModernManaged(42)
1138+
want.SetConditions(xpv1.ReconcilePending("Async operation in progress").WithObservedGeneration(42))
1139+
1140+
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
1141+
reason := "An async-in-progress reconcile should set ReconcilePending, not ReconcileSuccess."
1142+
t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff)
1143+
}
1144+
1145+
return nil
1146+
}),
1147+
},
1148+
Scheme: fake.SchemeWith(&fake.ModernManaged{}),
1149+
},
1150+
mg: resource.ManagedKind(fake.GVK(&fake.ModernManaged{})),
1151+
o: []ReconcilerOption{
1152+
WithInitializers(),
1153+
WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })),
1154+
WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) {
1155+
c := &ExternalClientFns{
1156+
ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) {
1157+
return ExternalObservation{
1158+
ResourceExists: true,
1159+
ResourceUpToDate: true,
1160+
AsyncOperationInProgress: true,
1161+
}, nil
1162+
},
1163+
DisconnectFn: func(_ context.Context) error {
1164+
return nil
1165+
},
1166+
}
1167+
1168+
return c, nil
1169+
})),
1170+
WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}),
1171+
},
1172+
},
1173+
want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}},
1174+
},
11301175
"ExternalResourceUpToDateWithJitter": {
11311176
reason: "When the external resource exists and is up to date a requeue should be triggered after a long wait with jitter added.",
11321177
args: args{

0 commit comments

Comments
 (0)