Skip to content

Commit a3f67f4

Browse files
tomkennedy513tomkennedy513
authored
Merge pull request buildpacks-community#1221 from evankanderson/i-got-a-message-for-you
Improve kpack conditions
2 parents 9864fc3 + 14dbe96 commit a3f67f4

3 files changed

Lines changed: 67 additions & 12 deletions

File tree

‎pkg/apis/build/v1alpha2/image_lifecycle.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
const (
1313
BuilderNotFound = "BuilderNotFound"
1414
BuilderNotReady = "BuilderNotReady"
15+
BuilderReady = "BuilderReady"
1516
)
1617

1718
func (im *Image) BuilderNotFound() corev1alpha1.Conditions {

‎pkg/reconciler/image/image_test.go‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -796,8 +796,10 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
796796
ObservedGeneration: originalGeneration,
797797
Conditions: corev1alpha1.Conditions{
798798
{
799-
Type: corev1alpha1.ConditionReady,
800-
Status: corev1.ConditionUnknown,
799+
Type: corev1alpha1.ConditionReady,
800+
Status: corev1.ConditionFalse,
801+
Reason: buildapi.BuilderNotReady,
802+
Message: "Builder builder-name is not ready: something went wrong",
801803
},
802804
{
803805
Type: buildapi.ConditionBuilderReady,
@@ -2106,6 +2108,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
21062108
{
21072109
Type: buildapi.ConditionBuilderReady,
21082110
Status: corev1.ConditionTrue,
2111+
Reason: buildapi.BuilderReady,
21092112
},
21102113
},
21112114
},
@@ -2168,10 +2171,12 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
21682171
{
21692172
Type: corev1alpha1.ConditionSucceeded,
21702173
Status: corev1.ConditionTrue,
2174+
Reason: image.UpToDateReason,
21712175
},
21722176
{
21732177
Type: buildapi.ConditionBuilderReady,
21742178
Status: corev1.ConditionTrue,
2179+
Reason: buildapi.BuilderReady,
21752180
},
21762181
},
21772182
},
@@ -2210,10 +2215,12 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
22102215
{
22112216
Type: corev1alpha1.ConditionReady,
22122217
Status: corev1.ConditionTrue,
2218+
Reason: image.UpToDateReason,
22132219
},
22142220
{
22152221
Type: buildapi.ConditionBuilderReady,
22162222
Status: corev1.ConditionTrue,
2223+
Reason: buildapi.BuilderReady,
22172224
},
22182225
},
22192226
},
@@ -2265,7 +2272,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
22652272
})
22662273
})
22672274

2268-
it("reports unknown when last build was successful and builder is not ready", func() {
2275+
it("reports false when last build was successful and builder is not ready", func() {
22692276
imageWithBuilder.Status.BuildCounter = 1
22702277
imageWithBuilder.Status.LatestBuildRef = "image-name-build-1"
22712278
imageWithBuilder.Status.LatestImage = "some/image@some-old-sha"
@@ -2291,8 +2298,10 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
22912298
ObservedGeneration: originalGeneration,
22922299
Conditions: corev1alpha1.Conditions{
22932300
{
2294-
Type: corev1alpha1.ConditionReady,
2295-
Status: corev1.ConditionUnknown,
2301+
Type: corev1alpha1.ConditionReady,
2302+
Status: corev1.ConditionFalse,
2303+
Reason: buildapi.BuilderNotReady,
2304+
Message: "Builder builder-name is not ready",
22962305
},
22972306
{
22982307
Type: buildapi.ConditionBuilderReady,
@@ -2377,11 +2386,13 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
23772386
{
23782387
Type: corev1alpha1.ConditionReady,
23792388
Status: corev1.ConditionFalse,
2389+
Reason: image.BuildFailedReason,
23802390
Message: failureMessage,
23812391
},
23822392
{
23832393
Type: buildapi.ConditionBuilderReady,
23842394
Status: corev1.ConditionTrue,
2395+
Reason: buildapi.BuilderReady,
23852396
},
23862397
},
23872398
},
@@ -2594,11 +2605,13 @@ func conditionReadyUnknown() corev1alpha1.Conditions {
25942605
{
25952606
Type: corev1alpha1.ConditionReady,
25962607
Status: corev1.ConditionUnknown,
2608+
Reason: image.ResolverNotReadyReason,
25972609
Message: "SourceResolver image-name-source is not ready",
25982610
},
25992611
{
26002612
Type: buildapi.ConditionBuilderReady,
26012613
Status: corev1.ConditionTrue,
2614+
Reason: buildapi.BuilderReady,
26022615
},
26032616
}
26042617
}
@@ -2608,11 +2621,13 @@ func conditionBuildExecuting(buildName string) corev1alpha1.Conditions {
26082621
{
26092622
Type: corev1alpha1.ConditionReady,
26102623
Status: corev1.ConditionUnknown,
2624+
Reason: image.BuildRunningReason,
26112625
Message: fmt.Sprintf("%s is executing", buildName),
26122626
},
26132627
{
26142628
Type: buildapi.ConditionBuilderReady,
26152629
Status: corev1.ConditionTrue,
2630+
Reason: buildapi.BuilderReady,
26162631
},
26172632
}
26182633
}
@@ -2622,10 +2637,12 @@ func conditionReady() corev1alpha1.Conditions {
26222637
{
26232638
Type: corev1alpha1.ConditionReady,
26242639
Status: corev1.ConditionTrue,
2640+
Reason: image.UpToDateReason,
26252641
},
26262642
{
26272643
Type: buildapi.ConditionBuilderReady,
26282644
Status: corev1.ConditionTrue,
2645+
Reason: buildapi.BuilderReady,
26292646
},
26302647
}
26312648
}
@@ -2635,10 +2652,12 @@ func conditionNotReady() corev1alpha1.Conditions {
26352652
{
26362653
Type: corev1alpha1.ConditionReady,
26372654
Status: corev1.ConditionFalse,
2655+
Reason: image.BuildFailedReason,
26382656
},
26392657
{
26402658
Type: buildapi.ConditionBuilderReady,
26412659
Status: corev1.ConditionTrue,
2660+
Reason: buildapi.BuilderReady,
26422661
},
26432662
}
26442663
}

‎pkg/reconciler/image/reconcile_build.go‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@ import (
1313
corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1"
1414
)
1515

16-
const BuildRunningReason = "BuildRunning"
16+
const (
17+
BuildRunningReason = "BuildRunning"
18+
ResolverNotReadyReason = "ResolverNotReady"
19+
BuildFailedReason = "BuildFailed"
20+
UpToDateReason = "UpToDate"
21+
)
1722

1823
func (c *Reconciler) reconcileBuild(ctx context.Context, image *buildapi.Image, latestBuild *buildapi.Build, sourceResolver *buildapi.SourceResolver, builder buildapi.BuilderResource, buildCacheName string) (buildapi.ImageStatus, error) {
1924
currentBuildNumber, err := buildCounter(latestBuild)
@@ -71,27 +76,49 @@ func (c *Reconciler) reconcileBuild(ctx context.Context, image *buildapi.Image,
7176
}
7277

7378
func noScheduledBuild(buildNeeded corev1.ConditionStatus, builder buildapi.BuilderResource, build *buildapi.Build, sourceResolver *buildapi.SourceResolver) corev1alpha1.Conditions {
79+
if !builder.Ready() {
80+
return corev1alpha1.Conditions{
81+
{
82+
Type: corev1alpha1.ConditionReady,
83+
Status: corev1.ConditionFalse,
84+
Reason: buildapi.BuilderNotReady,
85+
Message: builderError(builder),
86+
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
87+
},
88+
builderCondition(builder),
89+
}
90+
}
7491
if buildNeeded == corev1.ConditionUnknown {
75-
message := ""
92+
message := "Build status unknown"
7693
if !sourceResolver.Ready() {
7794
message = fmt.Sprintf("SourceResolver %s is not ready", sourceResolver.GetName())
7895
}
7996
return corev1alpha1.Conditions{
8097
{
8198
Type: corev1alpha1.ConditionReady,
8299
Status: corev1.ConditionUnknown,
100+
Reason: ResolverNotReadyReason,
83101
Message: message,
84102
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
85103
},
86104
builderCondition(builder),
87105
}
88106
}
89107

108+
buildReason := UpToDateReason
109+
buildMessage := "Last build succeeded"
110+
111+
if !build.Status.GetCondition(corev1alpha1.ConditionSucceeded).IsTrue() {
112+
buildReason = BuildFailedReason
113+
buildMessage = "Last build did not succeed"
114+
}
115+
90116
return corev1alpha1.Conditions{
91117
{
92118
Type: corev1alpha1.ConditionReady,
93119
Status: unknownStatusIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded)),
94-
Message: emptyMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded)),
120+
Reason: buildReason,
121+
Message: defaultMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded), buildMessage),
95122
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
96123
},
97124
builderCondition(builder),
@@ -106,9 +133,12 @@ func unknownStatusIfNil(condition *corev1alpha1.Condition) corev1.ConditionStatu
106133
return condition.Status
107134
}
108135

109-
func emptyMessageIfNil(condition *corev1alpha1.Condition) string {
136+
// Copies the message from the specified condition, or fills in a default message if nil.
137+
// We should always have a message for non-successful conditions, as that conveys
138+
// information to the user about what is expected.
139+
func defaultMessageIfNil(condition *corev1alpha1.Condition, defaultMessage string) string {
110140
if condition == nil {
111-
return ""
141+
return defaultMessage
112142
}
113143
return condition.Message
114144
}
@@ -126,6 +156,7 @@ func builderCondition(builder buildapi.BuilderResource) corev1alpha1.Condition {
126156
return corev1alpha1.Condition{
127157
Type: buildapi.ConditionBuilderReady,
128158
Status: corev1.ConditionTrue,
159+
Reason: buildapi.BuilderReady,
129160
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
130161
}
131162
}
@@ -145,12 +176,14 @@ func scheduledBuildCondition(build *buildapi.Build) corev1alpha1.Conditions {
145176
{
146177
Type: corev1alpha1.ConditionReady,
147178
Status: corev1.ConditionUnknown,
148-
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
179+
Reason: BuildRunningReason,
149180
Message: fmt.Sprintf("%s is executing", build.Name),
181+
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
150182
},
151183
{
152184
Type: buildapi.ConditionBuilderReady,
153185
Status: corev1.ConditionTrue,
186+
Reason: buildapi.BuilderReady,
154187
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
155188
},
156189
}
@@ -166,12 +199,14 @@ func buildCounter(build *buildapi.Build) (int64, error) {
166199
}
167200

168201
func buildRunningCondition(build *buildapi.Build, builder buildapi.BuilderResource) corev1alpha1.Conditions {
202+
message := defaultMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded),
203+
fmt.Sprintf("%s is executing", build.Name))
169204
return corev1alpha1.Conditions{
170205
{
171206
Type: corev1alpha1.ConditionReady,
172207
Status: corev1.ConditionUnknown,
173208
Reason: BuildRunningReason,
174-
Message: emptyMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded)),
209+
Message: message,
175210
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
176211
},
177212
builderCondition(builder),

0 commit comments

Comments
 (0)