Skip to content

Commit 416a487

Browse files
Fix Boxcutter to show Failed (not Absent) for config errors
Boxcutter now consistently shows 'Installed: False, Reason: Failed' for configuration validation errors, matching Helm behavior. Detects "invalid configuration:" errors in rolling revisions and uses Failed reason instead of Absent for semantic correctness. Assisted-by: Cursor
1 parent 7a60e71 commit 416a487

3 files changed

Lines changed: 152 additions & 5 deletions

File tree

internal/operator-controller/controllers/common_controller.go

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controllers
1919
import (
2020
"errors"
2121
"fmt"
22+
"strings"
2223

2324
apimeta "k8s.io/apimachinery/pkg/api/meta"
2425
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -56,11 +57,8 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt
5657
// Nothing is installed
5758
if revisionStates.Installed == nil {
5859
setInstallStatus(ext, nil)
59-
if len(revisionStates.RollingOut) == 0 {
60-
setInstalledStatusConditionFalse(ext, ocv1.ReasonFailed, "No bundle installed")
61-
} else {
62-
setInstalledStatusConditionFalse(ext, ocv1.ReasonAbsent, "No bundle installed")
63-
}
60+
reason := determineFailureReason(revisionStates.RollingOut)
61+
setInstalledStatusConditionFalse(ext, reason, "No bundle installed")
6462
return
6563
}
6664
// Something is installed
@@ -71,6 +69,23 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt
7169
setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", revisionStates.Installed.Image))
7270
}
7371

72+
// determineFailureReason determines the appropriate reason for installation failure.
73+
// Returns Failed if there are no rolling revisions or if rolling revisions have config validation errors.
74+
// Returns Absent if there are rolling revisions without config errors (e.g., normal rollout in progress).
75+
func determineFailureReason(rollingRevisions []*RevisionMetadata) string {
76+
if len(rollingRevisions) == 0 {
77+
return ocv1.ReasonFailed
78+
}
79+
80+
// Check if rolling revisions have config validation errors
81+
// If so, use Failed instead of Absent for consistency across runtimes
82+
if hasConfigValidationError(rollingRevisions) {
83+
return ocv1.ReasonFailed
84+
}
85+
86+
return ocv1.ReasonAbsent
87+
}
88+
7489
// setInstalledStatusConditionSuccess sets the installed status condition to success.
7590
func setInstalledStatusConditionSuccess(ext *ocv1.ClusterExtension, message string) {
7691
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
@@ -108,6 +123,25 @@ func setInstallStatus(ext *ocv1.ClusterExtension, installStatus *ocv1.ClusterExt
108123
ext.Status.Install = installStatus
109124
}
110125

126+
// hasConfigValidationError checks if any rolling revision has a config validation error.
127+
// Config validation errors contain "invalid configuration:" or "invalid ClusterExtension configuration:"
128+
// in their condition messages. This helps maintain consistency across runtimes:
129+
// - Helm validates config early (before creating resources), so no rolling revisions exist, resulting in Failed
130+
// - Boxcutter creates revision first then validates later, so rolling revision exists but should still be Failed
131+
func hasConfigValidationError(rollingRevisions []*RevisionMetadata) bool {
132+
for _, rev := range rollingRevisions {
133+
// Check Progressing condition for config validation errors
134+
progressingCond := apimeta.FindStatusCondition(rev.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)
135+
if progressingCond != nil && strings.Contains(progressingCond.Message, "invalid configuration:") {
136+
return true
137+
}
138+
if progressingCond != nil && strings.Contains(progressingCond.Message, "invalid ClusterExtension configuration:") {
139+
return true
140+
}
141+
}
142+
return false
143+
}
144+
111145
func setStatusProgressing(ext *ocv1.ClusterExtension, err error) {
112146
progressingCond := metav1.Condition{
113147
Type: ocv1.TypeProgressing,

internal/operator-controller/controllers/common_controller_test.go

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,3 +240,114 @@ func TestSetStatusConditionWrapper(t *testing.T) {
240240
})
241241
}
242242
}
243+
244+
func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T) {
245+
tests := []struct {
246+
name string
247+
revisionStates *RevisionStates
248+
expectedInstalledCond metav1.Condition
249+
}{
250+
{
251+
name: "no revisions at all - uses Failed",
252+
revisionStates: &RevisionStates{
253+
Installed: nil,
254+
RollingOut: nil,
255+
},
256+
expectedInstalledCond: metav1.Condition{
257+
Type: ocv1.TypeInstalled,
258+
Status: metav1.ConditionFalse,
259+
Reason: ocv1.ReasonFailed,
260+
},
261+
},
262+
{
263+
name: "rolling revision without config error - uses Absent",
264+
revisionStates: &RevisionStates{
265+
Installed: nil,
266+
RollingOut: []*RevisionMetadata{
267+
{
268+
RevisionName: "rev-1",
269+
Conditions: []metav1.Condition{
270+
{
271+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
272+
Status: metav1.ConditionTrue,
273+
Reason: ocv1.ClusterExtensionRevisionReasonRetrying,
274+
Message: "some other error",
275+
},
276+
},
277+
},
278+
},
279+
},
280+
expectedInstalledCond: metav1.Condition{
281+
Type: ocv1.TypeInstalled,
282+
Status: metav1.ConditionFalse,
283+
Reason: ocv1.ReasonAbsent,
284+
},
285+
},
286+
{
287+
name: "rolling revision with 'invalid configuration:' error - uses Failed",
288+
revisionStates: &RevisionStates{
289+
Installed: nil,
290+
RollingOut: []*RevisionMetadata{
291+
{
292+
RevisionName: "rev-1",
293+
Conditions: []metav1.Condition{
294+
{
295+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
296+
Status: metav1.ConditionTrue,
297+
Reason: ocv1.ClusterExtensionRevisionReasonRetrying,
298+
Message: "error for resolved bundle: invalid configuration: required field 'watchNamespace' is missing",
299+
},
300+
},
301+
},
302+
},
303+
},
304+
expectedInstalledCond: metav1.Condition{
305+
Type: ocv1.TypeInstalled,
306+
Status: metav1.ConditionFalse,
307+
Reason: ocv1.ReasonFailed,
308+
},
309+
},
310+
{
311+
name: "rolling revision with 'invalid ClusterExtension configuration:' error - uses Failed",
312+
revisionStates: &RevisionStates{
313+
Installed: nil,
314+
RollingOut: []*RevisionMetadata{
315+
{
316+
RevisionName: "rev-1",
317+
Conditions: []metav1.Condition{
318+
{
319+
Type: ocv1.ClusterExtensionRevisionTypeProgressing,
320+
Status: metav1.ConditionTrue,
321+
Reason: ocv1.ClusterExtensionRevisionReasonRetrying,
322+
Message: "invalid ClusterExtension configuration: watchNamespace must be 'test-ns'",
323+
},
324+
},
325+
},
326+
},
327+
},
328+
expectedInstalledCond: metav1.Condition{
329+
Type: ocv1.TypeInstalled,
330+
Status: metav1.ConditionFalse,
331+
Reason: ocv1.ReasonFailed,
332+
},
333+
},
334+
}
335+
336+
for _, tt := range tests {
337+
t.Run(tt.name, func(t *testing.T) {
338+
ext := &ocv1.ClusterExtension{
339+
ObjectMeta: metav1.ObjectMeta{
340+
Name: "test-ext",
341+
Generation: 1,
342+
},
343+
}
344+
345+
setInstalledStatusFromRevisionStates(ext, tt.revisionStates)
346+
347+
cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled)
348+
require.NotNil(t, cond)
349+
require.Equal(t, tt.expectedInstalledCond.Status, cond.Status)
350+
require.Equal(t, tt.expectedInstalledCond.Reason, cond.Reason)
351+
})
352+
}
353+
}

test/e2e/features/install.feature

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ Feature: Install ClusterExtension
122122
error for resolved bundle "single-namespace-operator.1.0.0" with version "1.0.0":
123123
invalid ClusterExtension configuration: invalid configuration: required field "watchNamespace" is missing
124124
"""
125+
And ClusterExtension reports Installed as False with Reason Failed
125126
When ClusterExtension is updated to set config.watchNamespace field
126127
"""
127128
apiVersion: olm.operatorframework.io/v1
@@ -205,6 +206,7 @@ Feature: Install ClusterExtension
205206
must be "${TEST_NAMESPACE}" (the namespace where the operator is installed) because this
206207
operator only supports OwnNamespace install mode
207208
"""
209+
And ClusterExtension reports Installed as False with Reason Failed
208210
When ClusterExtension is updated to set watchNamespace to own namespace value
209211
"""
210212
apiVersion: olm.operatorframework.io/v1

0 commit comments

Comments
 (0)