Skip to content

Commit 39173f3

Browse files
michaelhtmmichaelhtm
authored
feat: Add Support for Ready Condition (#210)
Description of changes: Adding support for a Ready condition that is set after every reconciliation standardizes ACK's conditions. This condition will serve as a high level status check for users that want to use ACK alogside with other controllers (kro). The existing conditions will remain unchanged, and will still be set on the resource as they have been in the past. With this change, you'll be seeing a new condition that brings all our conditions together. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 829d005 commit 39173f3

File tree

5 files changed

+141
-13
lines changed

5 files changed

+141
-13
lines changed

‎apis/core/v1alpha1/conditions.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import (
2323
type ConditionType string
2424

2525
const (
26+
// ConditionTypeReady indicates the resource is ready to be consumed by other resources.
27+
// It is equivalent to the ResourceSynced condition type. Using Ready will standardize
28+
// ACK conditions.
29+
ConditionTypeReady ConditionType = "Ready"
2630
// ConditionTypeAdopted indicates that the adopted resource custom resource
2731
// has been successfully reconciled and the target has been created
2832
ConditionTypeAdopted ConditionType = "ACK.Adopted"

‎pkg/condition/condition.go‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,23 @@ var (
2929
NotManagedReason = "This resource already exists but is not managed by ACK. " +
3030
"To bring the resource under ACK management, you should explicitly adopt " +
3131
"the resource by enabling the ResourceAdoption feature gate and populating " +
32-
"the `services.k8s.aws/adoption-policy` and `services.k8s.aws/adoption-fields` " +
32+
"the `services.k8s.aws/adoption-policy` and `services.k8s.aws/adoption-fields` " +
3333
"annotations."
3434
UnknownSyncedMessage = "Unable to determine if desired resource state matches latest observed state"
3535
NotSyncedMessage = "Resource not synced"
36+
TerminalMessage = "Resource is "
3637
SyncedMessage = "Resource synced successfully"
3738
FailedReferenceResolutionMessage = "Reference resolution failed"
3839
UnavailableIAMRoleMessage = "IAM Role is not available"
3940
)
4041

42+
// Ready returns the Condition in the resource's Conditions collection that is
43+
// of type ConditionTypeResourceReady. If no such condition is found, returns
44+
// nil.
45+
func Ready(subject acktypes.ConditionManager) *ackv1alpha1.Condition {
46+
return FirstOfType(subject, ackv1alpha1.ConditionTypeReady)
47+
}
48+
4149
// Synced returns the Condition in the resource's Conditions collection that is
4250
// of type ConditionTypeResourceSynced. If no such condition is found, returns
4351
// nil.
@@ -126,6 +134,28 @@ func SetSynced(
126134
subject.ReplaceConditions(allConds)
127135
}
128136

137+
func SetReady(
138+
subject acktypes.ConditionManager,
139+
status corev1.ConditionStatus,
140+
message *string,
141+
reason *string,
142+
) {
143+
allConds := subject.Conditions()
144+
var c *ackv1alpha1.Condition
145+
if c = Ready(subject); c == nil {
146+
c = &ackv1alpha1.Condition{
147+
Type: ackv1alpha1.ConditionTypeReady,
148+
}
149+
allConds = append(allConds, c)
150+
}
151+
now := metav1.Now()
152+
c.LastTransitionTime = &now
153+
c.Status = status
154+
c.Message = message
155+
c.Reason = reason
156+
subject.ReplaceConditions(allConds)
157+
}
158+
129159
// SetTerminal sets the resource's Condition of type ConditionTypeTerminal to
130160
// the supplied status, optional message and reason.
131161
func SetTerminal(

‎pkg/condition/condition_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ func TestConditionGetters(t *testing.T) {
4646
got = ackcond.ReferencesResolved(r)
4747
assert.Nil(got)
4848

49+
got = ackcond.Ready(r)
50+
assert.Nil(got)
51+
4952
conds = append(conds, &ackv1alpha1.Condition{
5053
Type: ackv1alpha1.ConditionTypeResourceSynced,
5154
Status: corev1.ConditionFalse,
@@ -60,6 +63,9 @@ func TestConditionGetters(t *testing.T) {
6063
got = ackcond.Terminal(r)
6164
assert.Nil(got)
6265

66+
got = ackcond.Ready(r)
67+
assert.Nil(got)
68+
6369
conds = append(conds, &ackv1alpha1.Condition{
6470
Type: ackv1alpha1.ConditionTypeTerminal,
6571
Status: corev1.ConditionFalse,
@@ -74,6 +80,9 @@ func TestConditionGetters(t *testing.T) {
7480
got = ackcond.Terminal(r)
7581
assert.NotNil(got)
7682

83+
got = ackcond.Ready(r)
84+
assert.Nil(got)
85+
7786
gotAll := ackcond.AllOfType(r, ackv1alpha1.ConditionTypeAdvisory)
7887
assert.Empty(gotAll)
7988

@@ -106,6 +115,15 @@ func TestConditionGetters(t *testing.T) {
106115
r.On("Conditions").Return(conds)
107116
got = ackcond.ReferencesResolved(r)
108117
assert.NotNil(got)
118+
119+
conds = append(conds, &ackv1alpha1.Condition{
120+
Type: ackv1alpha1.ConditionTypeReady,
121+
Status: corev1.ConditionTrue,
122+
})
123+
r = &ackmocks.AWSResource{}
124+
r.On("Conditions").Return(conds)
125+
got = ackcond.Ready(r)
126+
assert.NotNil(got)
109127
}
110128

111129
func TestConditionSetters(t *testing.T) {
@@ -295,4 +313,20 @@ func TestConditionSetters(t *testing.T) {
295313
}),
296314
)
297315
ackcond.WithReferencesResolvedCondition(r, terminalError)
316+
317+
// Ready condition
318+
// SetReady
319+
r = &ackmocks.AWSResource{}
320+
r.On("Conditions").Return([]*ackv1alpha1.Condition{})
321+
r.On(
322+
"ReplaceConditions",
323+
mock.MatchedBy(func(subject []*ackv1alpha1.Condition) bool {
324+
if len(subject) != 1 {
325+
return false
326+
}
327+
return (subject[0].Type == ackv1alpha1.ConditionTypeReady &&
328+
subject[0].Status == corev1.ConditionTrue)
329+
}),
330+
)
331+
ackcond.SetReady(r, corev1.ConditionTrue, nil, nil)
298332
}

‎pkg/runtime/reconciler.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
apierrors "k8s.io/apimachinery/pkg/api/errors"
2929
"k8s.io/apimachinery/pkg/runtime/schema"
3030
"k8s.io/apimachinery/pkg/types"
31+
"k8s.io/utils/ptr"
3132
ctrlrt "sigs.k8s.io/controller-runtime"
3233
"sigs.k8s.io/controller-runtime/pkg/client"
3334
ctrlrtcontroller "sigs.k8s.io/controller-runtime/pkg/controller"
@@ -582,6 +583,20 @@ func (r *resourceReconciler) ensureConditions(
582583
}
583584
ackcondition.SetSynced(res, condStatus, &condMessage, &condReason)
584585
}
586+
587+
terminal := ackcondition.Terminal(res)
588+
recoverable := ackcondition.Recoverable(res)
589+
synced := ackcondition.Synced(res)
590+
591+
if terminal != nil && terminal.Status == corev1.ConditionTrue {
592+
ackcondition.SetReady(res, corev1.ConditionFalse, terminal.Message, ptr.To(string(ackv1alpha1.ConditionTypeTerminal)))
593+
} else if recoverable != nil && recoverable.Status == corev1.ConditionTrue {
594+
ackcondition.SetReady(res, corev1.ConditionFalse, recoverable.Message, ptr.To(string(ackv1alpha1.ConditionTypeRecoverable)))
595+
} else if synced != nil {
596+
ackcondition.SetReady(res, synced.Status, synced.Message, synced.Reason)
597+
} else {
598+
ackcondition.SetReady(res, corev1.ConditionUnknown, &ackcondition.UnknownSyncedMessage, nil)
599+
}
585600
}
586601

587602
// createResource marks the CR as managed by ACK, calls one or more AWS APIs to

‎pkg/runtime/reconciler_test.go‎

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ func TestReconcilerAdoptOrCreateResource_Adopt(t *testing.T) {
505505
latest, latestRTObj, latestMetaObj := resourceMocks()
506506
latest.On("Identifiers").Return(ids)
507507
latest.On("Conditions").Return([]*ackv1alpha1.Condition{})
508-
latest.On(
508+
latest.On(
509509
"ReplaceConditions",
510510
mock.AnythingOfType("[]*v1alpha1.Condition"),
511511
).Return().Run(func(args mock.Arguments) {
@@ -521,7 +521,12 @@ func TestReconcilerAdoptOrCreateResource_Adopt(t *testing.T) {
521521
assert.Equal(ackcondition.SyncedMessage, *condition.Message)
522522
}
523523
assert.True(hasSynced)
524-
})
524+
}).Once()
525+
latest.On(
526+
"ReplaceConditions",
527+
mock.AnythingOfType("[]*v1alpha1.Condition"),
528+
).Return()
529+
525530
latestMetaObj.SetAnnotations(map[string]string{
526531
ackv1alpha1.AnnotationAdoptionPolicy: "adopt-or-create",
527532
ackv1alpha1.AnnotationAdoptionFields: adoptionFieldsString,
@@ -617,7 +622,11 @@ func TestReconcilerCreate_UnManagedResource_CheckReferencesResolveOnce(t *testin
617622
cond := conditions[0]
618623
assert.Equal(t, ackv1alpha1.ConditionTypeResourceSynced, cond.Type)
619624
assert.Equal(t, corev1.ConditionTrue, cond.Status)
620-
})
625+
}).Once()
626+
latest.On(
627+
"ReplaceConditions",
628+
mock.AnythingOfType("[]*v1alpha1.Condition"),
629+
).Return()
621630

622631
rm := &ackmocks.AWSResourceManager{}
623632
rm.On("ResolveReferences", ctx, nil, desired).Return(
@@ -700,7 +709,11 @@ func TestReconcilerCreate_ManagedResource_CheckReferencesResolveOnce(t *testing.
700709
cond := conditions[0]
701710
assert.Equal(t, ackv1alpha1.ConditionTypeResourceSynced, cond.Type)
702711
assert.Equal(t, corev1.ConditionTrue, cond.Status)
703-
})
712+
}).Once()
713+
latest.On(
714+
"ReplaceConditions",
715+
mock.AnythingOfType("[]*v1alpha1.Condition"),
716+
).Return()
704717

705718
rm := &ackmocks.AWSResourceManager{}
706719
rm.On("ResolveReferences", ctx, nil, desired).Return(
@@ -783,7 +796,11 @@ func TestReconcilerUpdate(t *testing.T) {
783796
cond := conditions[0]
784797
assert.Equal(t, ackv1alpha1.ConditionTypeResourceSynced, cond.Type)
785798
assert.Equal(t, corev1.ConditionTrue, cond.Status)
786-
})
799+
}).Once()
800+
latest.On(
801+
"ReplaceConditions",
802+
mock.AnythingOfType("[]*v1alpha1.Condition"),
803+
).Return()
787804

788805
rm := &ackmocks.AWSResourceManager{}
789806
rm.On("ResolveReferences", ctx, nil, desired).Return(
@@ -870,7 +887,11 @@ func TestReconcilerUpdate_ResourceNotSynced(t *testing.T) {
870887
// False
871888
assert.Equal(t, corev1.ConditionFalse, cond.Status)
872889
assert.Equal(t, ackcondition.NotSyncedMessage, *cond.Message)
873-
})
890+
}).Once()
891+
latest.On(
892+
"ReplaceConditions",
893+
mock.AnythingOfType("[]*v1alpha1.Condition"),
894+
).Return()
874895

875896
rm := &ackmocks.AWSResourceManager{}
876897
rm.On("ResolveReferences", ctx, nil, desired).Return(
@@ -954,7 +975,11 @@ func TestReconcilerUpdate_NoDelta_ResourceNotSynced(t *testing.T) {
954975
// False
955976
assert.Equal(t, corev1.ConditionFalse, cond.Status)
956977
assert.Equal(t, ackcondition.NotSyncedMessage, *cond.Message)
957-
})
978+
}).Once()
979+
latest.On(
980+
"ReplaceConditions",
981+
mock.AnythingOfType("[]*v1alpha1.Condition"),
982+
).Return()
958983

959984
rm := &ackmocks.AWSResourceManager{}
960985
rm.On("ResolveReferences", ctx, nil, desired).Return(
@@ -1033,7 +1058,11 @@ func TestReconcilerUpdate_NoDelta_ResourceSynced(t *testing.T) {
10331058
// True
10341059
assert.Equal(t, corev1.ConditionTrue, cond.Status)
10351060
assert.Equal(t, ackcondition.SyncedMessage, *cond.Message)
1036-
})
1061+
}).Once()
1062+
latest.On(
1063+
"ReplaceConditions",
1064+
mock.AnythingOfType("[]*v1alpha1.Condition"),
1065+
).Return()
10371066

10381067
rm := &ackmocks.AWSResourceManager{}
10391068
rm.On("ResolveReferences", ctx, nil, desired).Return(
@@ -1116,7 +1145,11 @@ func TestReconcilerUpdate_IsSyncedError(t *testing.T) {
11161145
assert.Equal(t, corev1.ConditionFalse, cond.Status)
11171146
assert.Equal(t, ackcondition.NotSyncedMessage, *cond.Message)
11181147
assert.Equal(t, syncedError.Error(), *cond.Reason)
1119-
})
1148+
}).Once()
1149+
latest.On(
1150+
"ReplaceConditions",
1151+
mock.AnythingOfType("[]*v1alpha1.Condition"),
1152+
).Return()
11201153

11211154
rm := &ackmocks.AWSResourceManager{}
11221155
rm.On("ResolveReferences", ctx, nil, desired).Return(
@@ -1275,7 +1308,11 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInSpec(t *testing.T) {
12751308
assert.Equal(ackcondition.SyncedMessage, *condition.Message)
12761309
}
12771310
assert.True(hasSynced)
1278-
})
1311+
}).Once()
1312+
latest.On(
1313+
"ReplaceConditions",
1314+
mock.AnythingOfType("[]*v1alpha1.Condition"),
1315+
).Return()
12791316
// Note no change to metadata...
12801317

12811318
rmf, rd := managedResourceManagerFactoryMocks(desired, latest)
@@ -1425,7 +1462,11 @@ func TestReconcilerUpdate_ErrorInLateInitialization(t *testing.T) {
14251462
assert.Equal(requeueError.Error(), *condition.Reason)
14261463
}
14271464
assert.True(hasSynced)
1428-
})
1465+
}).Once()
1466+
latest.On(
1467+
"ReplaceConditions",
1468+
mock.AnythingOfType("[]*v1alpha1.Condition"),
1469+
).Return()
14291470

14301471
rm := &ackmocks.AWSResourceManager{}
14311472
rm.On("ResolveReferences", ctx, nil, desired).Return(
@@ -1545,7 +1586,11 @@ func TestReconcilerUpdate_ResourceNotManaged(t *testing.T) {
15451586
assert.Equal(ackcondition.NotSyncedMessage, *condition.Message)
15461587
}
15471588
assert.True(hasSynced)
1548-
})
1589+
}).Once()
1590+
latest.On(
1591+
"ReplaceConditions",
1592+
mock.AnythingOfType("[]*v1alpha1.Condition"),
1593+
).Return()
15491594

15501595
rm := &ackmocks.AWSResourceManager{}
15511596
rmf, rd := managerFactoryMocks(desired, latest, false)

0 commit comments

Comments
 (0)