Skip to content

Commit

Permalink
Log the reason why swap comp synthesis (#244)
Browse files Browse the repository at this point in the history
Resolve #225 

Log the reason why swap comp synthesis during reconciliation process
  • Loading branch information
chihshenghuang authored Nov 25, 2024
1 parent 3b7a52c commit 40cec4f
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 7 deletions.
47 changes: 41 additions & 6 deletions internal/controllers/synthesis/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ func (c *podLifecycleController) Reconcile(ctx context.Context, req ctrl.Request
}

// Swap the state to prepare for resynthesis if needed
if shouldSwapStates(syn, comp) {
reason, shouldSwap := shouldSwapStates(syn, comp)
if shouldSwap {
logger = logger.WithValues("reason", reason)
SwapStates(comp)
if err := c.client.Status().Update(ctx, comp); err != nil {
return ctrl.Result{}, fmt.Errorf("swapping compisition state: %w", err)
Expand Down Expand Up @@ -388,7 +390,7 @@ func SwapStates(comp *apiv1.Composition) {
}
}

func shouldSwapStates(synth *apiv1.Synthesizer, comp *apiv1.Composition) bool {
func shouldSwapStates(synth *apiv1.Synthesizer, comp *apiv1.Composition) (string, bool) {
// synthesize when (either):
// - synthesis has never occurred
// - the spec has changed
Expand All @@ -399,10 +401,43 @@ func shouldSwapStates(synth *apiv1.Synthesizer, comp *apiv1.Composition) bool {
// - synthesis is not already pending
// - all bound input resources exist and are in lockstep (or composition is being deleted)
syn := comp.Status.CurrentSynthesis
return (syn == nil ||
syn.ObservedCompositionGeneration != comp.Generation ||
(!inputRevisionsEqual(synth, comp.Status.InputRevisions, syn.InputRevisions) && syn.Synthesized != nil && !comp.ShouldIgnoreSideEffects())) &&
(comp.DeletionTimestamp != nil || (comp.InputsExist(synth) && !comp.InputsOutOfLockstep(synth)))

isSynNil := syn == nil
isCompGenDiff := syn != nil && syn.ObservedCompositionGeneration != comp.Generation
isInputRevisionsEqual := syn != nil && inputRevisionsEqual(synth, comp.Status.InputRevisions, syn.InputRevisions)
isSynthesizedNotNil := syn != nil && syn.Synthesized != nil
shouldIgnoreSideEffects := comp.ShouldIgnoreSideEffects()
isCompDeleted := comp.DeletionTimestamp != nil
isCompInputsExist := comp.InputsExist(synth)
isCompInputsOutOfLockstep := comp.InputsOutOfLockstep(synth)

reason := ""
// First condition
if isSynNil {
reason += "CurrentSynthesisEmpty"
} else {
if isCompGenDiff {
reason += "CompositionChanged"
} else if !isInputRevisionsEqual && isSynthesizedNotNil && !shouldIgnoreSideEffects {
reason += "InputsChanged"
}
}
if !(isSynNil || isCompGenDiff || (!isInputRevisionsEqual && isSynthesizedNotNil && !shouldIgnoreSideEffects)) {
return "", false
}

// Second condition
if isCompDeleted {
reason += " && CompositionDeleted"
} else if isCompInputsExist && !isCompInputsOutOfLockstep {
reason += " && InputsInLockstep"
}

if !(isCompDeleted || (isCompInputsExist && !isCompInputsOutOfLockstep)) {
return "", false
}

return reason, true
}

func shouldBackOffPodCreation(comp *apiv1.Composition) bool {
Expand Down
18 changes: 17 additions & 1 deletion internal/controllers/synthesis/lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,10 +675,12 @@ func TestShouldSwapStates(t *testing.T) {
Name string
Expectation bool
Composition apiv1.Composition
Reason string
}{
{
Name: "zero value",
Expectation: true,
Reason: "CurrentSynthesisEmpty && InputsInLockstep",
},
{
Name: "missing input",
Expand All @@ -688,6 +690,7 @@ func TestShouldSwapStates(t *testing.T) {
Bindings: []apiv1.Binding{{Key: "foo"}},
},
},
Reason: "",
},
{
Name: "matching input synthesis in progress",
Expand All @@ -707,6 +710,7 @@ func TestShouldSwapStates(t *testing.T) {
}},
},
},
Reason: "",
},
{
Name: "non-matching composition generation",
Expand All @@ -721,6 +725,7 @@ func TestShouldSwapStates(t *testing.T) {
},
},
},
Reason: "CompositionChanged && InputsInLockstep",
},
{
Name: "matching input synthesis terminal",
Expand All @@ -741,6 +746,7 @@ func TestShouldSwapStates(t *testing.T) {
}},
},
},
Reason: "",
},
{
Name: "non-matching input synthesis terminal",
Expand All @@ -762,6 +768,7 @@ func TestShouldSwapStates(t *testing.T) {
}},
},
},
Reason: "InputsChanged && InputsInLockstep",
},
{
Name: "non-matching input synthesis terminal ignore side effects",
Expand All @@ -788,6 +795,7 @@ func TestShouldSwapStates(t *testing.T) {
}},
},
},
Reason: "",
},
{
Name: "non-matching input synthesis non-terminal",
Expand All @@ -809,6 +817,7 @@ func TestShouldSwapStates(t *testing.T) {
}},
},
},
Reason: "",
},
{
Name: "non-matching input synthesis deleting",
Expand All @@ -833,6 +842,7 @@ func TestShouldSwapStates(t *testing.T) {
}},
},
},
Reason: "CompositionChanged && CompositionDeleted",
},
{
Name: "missing input synthesis deleting",
Expand All @@ -847,6 +857,7 @@ func TestShouldSwapStates(t *testing.T) {
},
Status: apiv1.CompositionStatus{},
},
Reason: "CurrentSynthesisEmpty && CompositionDeleted",
},
{
Name: "revision mismatch",
Expand All @@ -871,6 +882,7 @@ func TestShouldSwapStates(t *testing.T) {
}},
},
},
Reason: "",
},
{
Name: "revision match",
Expand All @@ -895,13 +907,17 @@ func TestShouldSwapStates(t *testing.T) {
}},
},
},
Reason: "CompositionChanged && InputsInLockstep",
},
}

for _, tc := range tests {
t.Run(tc.Name, func(t *testing.T) {
syn := &apiv1.Synthesizer{}
syn.Spec.Refs = []apiv1.Ref{{Key: "foo"}}
assert.Equal(t, tc.Expectation, shouldSwapStates(syn, &tc.Composition))
reason, shouldSwap := shouldSwapStates(syn, &tc.Composition)
assert.Equal(t, tc.Expectation, shouldSwap)
assert.Equal(t, tc.Reason, reason)
})
}
}
Expand Down

0 comments on commit 40cec4f

Please sign in to comment.