Skip to content

Commit aee7b51

Browse files
pedjakclaude
andcommitted
Address review feedback: delimiter in digest, wording fix, whitespace normalization
- Add newline delimiter between objects in computePhaseDigest to prevent ambiguous concatenation encodings - Fix API doc: "first successful reconciliation" → "first successful resolution" to match actual behavior - Normalize whitespace in actual condition messages in e2e message fragment comparisons to reduce flakiness - Revert predicate rename to skipProgressDeadlineExceededPredicate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b95d9ab commit aee7b51

File tree

7 files changed

+12
-9
lines changed

7 files changed

+12
-9
lines changed

api/v1/clusterobjectset_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ type ObservedPhase struct {
531531
Name string `json:"name"`
532532

533533
// digest is the hex-encoded SHA-256 digest of the phase's resolved
534-
// object content at first successful reconciliation.
534+
// object content at first successful resolution.
535535
//
536536
// +required
537537
Digest string `json:"digest"`

applyconfigurations/api/v1/observedphase.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ spec:
635635
digest:
636636
description: |-
637637
digest is the hex-encoded SHA-256 digest of the phase's resolved
638-
object content at first successful reconciliation.
638+
object content at first successful resolution.
639639
type: string
640640
name:
641641
description: name is the phase name matching a phase in spec.phases.

internal/operator-controller/controllers/clusterobjectset_controller.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ type Sourcoser interface {
370370
}
371371

372372
func (c *ClusterObjectSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
373-
skipTerminallyBlockedPredicate := predicate.Funcs{
373+
skipProgressDeadlineExceededPredicate := predicate.Funcs{
374374
UpdateFunc: func(e event.UpdateEvent) bool {
375375
rev, ok := e.ObjectNew.(*ocv1.ClusterObjectSet)
376376
if !ok {
@@ -394,7 +394,7 @@ func (c *ClusterObjectSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
394394
&ocv1.ClusterObjectSet{},
395395
builder.WithPredicates(
396396
predicate.ResourceVersionChangedPredicate{},
397-
skipTerminallyBlockedPredicate,
397+
skipProgressDeadlineExceededPredicate,
398398
),
399399
).
400400
WatchesRawSource(
@@ -780,6 +780,7 @@ func computePhaseDigest(phase boxcutter.Phase) (string, error) {
780780
return "", fmt.Errorf("marshaling object in phase %q: %w", phase.GetName(), err)
781781
}
782782
h.Write(data)
783+
h.Write([]byte{'\n'})
783784
}
784785
return fmt.Sprintf("%x", h.Sum(nil)), nil
785786
}

manifests/experimental-e2e.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1959,7 +1959,7 @@ spec:
19591959
digest:
19601960
description: |-
19611961
digest is the hex-encoded SHA-256 digest of the phase's resolved
1962-
object content at first successful reconciliation.
1962+
object content at first successful resolution.
19631963
type: string
19641964
name:
19651965
description: name is the phase name matching a phase in spec.phases.

manifests/experimental.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1920,7 +1920,7 @@ spec:
19201920
digest:
19211921
description: |-
19221922
digest is the hex-encoded SHA-256 digest of the phase's resolved
1923-
object content at first successful reconciliation.
1923+
object content at first successful resolution.
19241924
type: string
19251925
name:
19261926
description: name is the phase name matching a phase in spec.phases.

test/e2e/steps/steps.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,8 @@ func ClusterExtensionReportsConditionWithMessageFragment(ctx context.Context, co
511511
if msgFragment != nil {
512512
expectedMsgFragment := substituteScenarioVars(strings.Join(strings.Fields(msgFragment.Content), " "), scenarioCtx(ctx))
513513
msgCmp = func(actualMsg string) bool {
514-
return strings.Contains(actualMsg, expectedMsgFragment)
514+
normalizedActual := strings.Join(strings.Fields(actualMsg), " ")
515+
return strings.Contains(normalizedActual, expectedMsgFragment)
515516
}
516517
}
517518
return waitForExtensionCondition(ctx, conditionType, conditionStatus, &conditionReason, msgCmp)
@@ -607,7 +608,8 @@ func ClusterObjectSetReportsConditionWithMessageFragment(ctx context.Context, re
607608
if msgFragment != nil {
608609
expectedMsgFragment := substituteScenarioVars(strings.Join(strings.Fields(msgFragment.Content), " "), scenarioCtx(ctx))
609610
msgCmp = func(actualMsg string) bool {
610-
return strings.Contains(actualMsg, expectedMsgFragment)
611+
normalizedActual := strings.Join(strings.Fields(actualMsg), " ")
612+
return strings.Contains(normalizedActual, expectedMsgFragment)
611613
}
612614
}
613615
return waitForCondition(ctx, "clusterobjectset", substituteScenarioVars(revisionName, scenarioCtx(ctx)), conditionType, conditionStatus, &conditionReason, msgCmp)

0 commit comments

Comments
 (0)