From fd91988831e2e66189071853571c3cfaa247fc72 Mon Sep 17 00:00:00 2001 From: Ruinan Liu Date: Fri, 15 May 2026 18:16:25 +0000 Subject: [PATCH 1/3] Eno, logs which input is missing --- .../controllers/composition/controller.go | 9 + internal/inputs/inputs.go | 68 +++-- internal/inputs/inputs_test.go | 242 ++++++++++++++++++ 3 files changed, 298 insertions(+), 21 deletions(-) diff --git a/internal/controllers/composition/controller.go b/internal/controllers/composition/controller.go index 6f496dbf..d9767737 100644 --- a/internal/controllers/composition/controller.go +++ b/internal/controllers/composition/controller.go @@ -262,6 +262,15 @@ func (c *compositionController) reconcileSimplifiedStatus(ctx context.Context, s return false, nil } + if next != nil && synth != nil { + switch next.Status { + case "MissingInputs": + logger.Info("composition is missing required inputs", "missingInputs", inputs.Missing(synth, comp)) + case "MismatchedInputs": + logger.Info("composition has inputs that are out of lockstep", "mismatchedInputs", inputs.Mismatched(synth, comp, comp.Status.InputRevisions), "synthesizerGeneration", synth.Generation, "compositionGeneration", comp.Generation) + } + } + logger.Info("composition status changed", "previousStatus", comp.Status.Simplified, "currentStatus", next) copy := comp.DeepCopy() copy.Status.Simplified = next diff --git a/internal/inputs/inputs.go b/internal/inputs/inputs.go index f5b0992e..4e2ad345 100644 --- a/internal/inputs/inputs.go +++ b/internal/inputs/inputs.go @@ -9,6 +9,13 @@ import ( // Exist returns true when all of the inputs required by a synthesizer are represented by the given composition's status. // Optional refs are not required to exist and will not cause this function to return false. func Exist(syn *apiv1.Synthesizer, c *apiv1.Composition) bool { + return len(Missing(syn, c)) == 0 +} + +// Missing returns the keys of any non-optional inputs required by the synthesizer that +// are not represented in the given composition's status. +func Missing(syn *apiv1.Synthesizer, c *apiv1.Composition) []string { + var missing []string for _, ref := range syn.Spec.Refs { // Skip optional refs - they are not required to exist if ref.Optional { @@ -19,44 +26,63 @@ func Exist(syn *apiv1.Synthesizer, c *apiv1.Composition) bool { return ref.Key == current.Key }) if !found { - return false + missing = append(missing, ref.Key) } } - return true + return missing } // OutOfLockstep returns true when one or more inputs that specify a revision do not match the others. // It also returns true if any revision is derived from a synthesizer/composition generation older than the ones provided. func OutOfLockstep(synth *apiv1.Synthesizer, comp *apiv1.Composition, revs []apiv1.InputRevisions) bool { - // First, the the max revision across all bindings + return len(Mismatched(synth, comp, revs)) > 0 +} + +// MismatchedInput describes a single input revision that is out of lockstep with the others +// (or with the current synthesizer/composition generation). It is intended for logging/telemetry. +type MismatchedInput struct { + Key string + Revision *int + MaxRevision *int + SynthesizerGeneration *int64 + CompositionGeneration *int64 +} + +// Mismatched returns the set of input revisions that are out of lockstep with the others, or +// derived from a synthesizer/composition generation older than the current ones. +func Mismatched(synth *apiv1.Synthesizer, comp *apiv1.Composition, revs []apiv1.InputRevisions) []MismatchedInput { + var mismatched []MismatchedInput + + // First, find the max revision across all bindings var maxRevision *int for _, rev := range revs { - if rev.SynthesizerGeneration != nil && *rev.SynthesizerGeneration < synth.Generation { - return true - } - if rev.CompositionGeneration != nil && *rev.CompositionGeneration < comp.Generation { - return true - } if rev.Revision == nil { continue } - if maxRevision == nil { - maxRevision = rev.Revision - continue - } - if *rev.Revision > *maxRevision { + if maxRevision == nil || *rev.Revision > *maxRevision { maxRevision = rev.Revision } } - if maxRevision == nil { - return false // no inputs declare a revision, so we should assume they're in sync - } - // Now given the max, make sure all inputs with a revision match it for _, rev := range revs { - if rev.Revision != nil && *maxRevision != *rev.Revision { - return true + stale := false + if rev.SynthesizerGeneration != nil && *rev.SynthesizerGeneration < synth.Generation { + stale = true } + if rev.CompositionGeneration != nil && *rev.CompositionGeneration < comp.Generation { + stale = true + } + revMismatch := maxRevision != nil && rev.Revision != nil && *rev.Revision != *maxRevision + if !stale && !revMismatch { + continue + } + mismatched = append(mismatched, MismatchedInput{ + Key: rev.Key, + Revision: rev.Revision, + MaxRevision: maxRevision, + SynthesizerGeneration: rev.SynthesizerGeneration, + CompositionGeneration: rev.CompositionGeneration, + }) } - return false + return mismatched } diff --git a/internal/inputs/inputs_test.go b/internal/inputs/inputs_test.go index 38a06ef6..06972d32 100644 --- a/internal/inputs/inputs_test.go +++ b/internal/inputs/inputs_test.go @@ -491,3 +491,245 @@ func TestOutOfLockstep(t *testing.T) { }) } } + +func TestMissing(t *testing.T) { + tests := []struct { + Name string + Composition apiv1.Composition + Synthesizer apiv1.Synthesizer + Expectation []string + }{ + { + Name: "All required refs present", + Composition: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "key1"}, + {Key: "key2"}, + }, + }, + }, + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "key1"}, + {Key: "key2"}, + }, + }, + }, + Expectation: nil, + }, + { + Name: "One required ref missing", + Composition: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "key1"}, + }, + }, + }, + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "key1"}, + {Key: "key2"}, + }, + }, + }, + Expectation: []string{"key2"}, + }, + { + Name: "Multiple required refs missing reported in spec order", + Composition: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "key2"}, + }, + }, + }, + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "key1"}, + {Key: "key2"}, + {Key: "key3"}, + }, + }, + }, + Expectation: []string{"key1", "key3"}, + }, + { + Name: "Optional missing ref is not reported", + Composition: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "key1"}, + }, + }, + }, + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "key1"}, + {Key: "key2", Optional: true}, + }, + }, + }, + Expectation: nil, + }, + { + Name: "Required missing while optional missing - only required reported", + Composition: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{}, + }, + }, + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "key1"}, + {Key: "key2", Optional: true}, + }, + }, + }, + Expectation: []string{"key1"}, + }, + { + Name: "No refs declared", + Composition: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{}, + }, + }, + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{Refs: []apiv1.Ref{}}, + }, + Expectation: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + result := Missing(&tt.Synthesizer, &tt.Composition) + assert.Equal(t, tt.Expectation, result) + // Consistency check: Exist iff Missing is empty. + assert.Equal(t, len(tt.Expectation) == 0, Exist(&tt.Synthesizer, &tt.Composition)) + }) + } +} + +func TestMismatched(t *testing.T) { + revision1 := 1 + revision2 := 2 + + tests := []struct { + Name string + Input apiv1.Composition + Synth apiv1.Synthesizer + Expectation []MismatchedInput + }{ + { + Name: "No revisions", + Input: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{}, + }, + }, + Expectation: nil, + }, + { + Name: "All revisions match", + Input: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "a", Revision: &revision1}, + {Key: "b", Revision: &revision1}, + }, + }, + }, + Expectation: nil, + }, + { + Name: "One lagging behind", + Input: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "a", Revision: &revision1}, + {Key: "b", Revision: &revision2}, + {Key: "c", Revision: &revision1}, + }, + }, + }, + Expectation: []MismatchedInput{ + {Key: "a", Revision: &revision1, MaxRevision: &revision2}, + {Key: "c", Revision: &revision1, MaxRevision: &revision2}, + }, + }, + { + Name: "Nil revision is not reported as mismatch", + Input: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "a", Revision: &revision1}, + {Key: "b", Revision: nil}, + }, + }, + }, + Expectation: nil, + }, + { + Name: "Stale synthesizer generation", + Input: apiv1.Composition{ + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "a", SynthesizerGeneration: ptr.To(int64(122))}, + }, + }, + }, + Synth: apiv1.Synthesizer{ + ObjectMeta: metav1.ObjectMeta{Generation: 123}, + }, + Expectation: []MismatchedInput{ + {Key: "a", SynthesizerGeneration: ptr.To(int64(122))}, + }, + }, + { + Name: "Stale composition generation", + Input: apiv1.Composition{ + ObjectMeta: metav1.ObjectMeta{Generation: 5}, + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "a", CompositionGeneration: ptr.To(int64(4))}, + }, + }, + }, + Expectation: []MismatchedInput{ + {Key: "a", CompositionGeneration: ptr.To(int64(4))}, + }, + }, + { + Name: "Stale gens and revision mismatch combined", + Input: apiv1.Composition{ + ObjectMeta: metav1.ObjectMeta{Generation: 5}, + Status: apiv1.CompositionStatus{ + InputRevisions: []apiv1.InputRevisions{ + {Key: "a", Revision: &revision1, CompositionGeneration: ptr.To(int64(4))}, + {Key: "b", Revision: &revision2, CompositionGeneration: ptr.To(int64(5))}, + }, + }, + }, + Expectation: []MismatchedInput{ + {Key: "a", Revision: &revision1, MaxRevision: &revision2, CompositionGeneration: ptr.To(int64(4))}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + result := Mismatched(&tt.Synth, &tt.Input, tt.Input.Status.InputRevisions) + assert.Equal(t, tt.Expectation, result) + // Consistency check: OutOfLockstep iff Mismatched is non-empty. + assert.Equal(t, len(tt.Expectation) > 0, OutOfLockstep(&tt.Synth, &tt.Input, tt.Input.Status.InputRevisions)) + }) + } +} From fcb64b4372882b2a00e02783bb906b4f93824098 Mon Sep 17 00:00:00 2001 From: Ruinan Liu Date: Fri, 15 May 2026 18:25:14 +0000 Subject: [PATCH 2/3] Update more log line --- .../controllers/composition/controller.go | 2 +- internal/inputs/inputs.go | 12 +++ internal/inputs/inputs_test.go | 73 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/internal/controllers/composition/controller.go b/internal/controllers/composition/controller.go index d9767737..422e4c0b 100644 --- a/internal/controllers/composition/controller.go +++ b/internal/controllers/composition/controller.go @@ -265,7 +265,7 @@ func (c *compositionController) reconcileSimplifiedStatus(ctx context.Context, s if next != nil && synth != nil { switch next.Status { case "MissingInputs": - logger.Info("composition is missing required inputs", "missingInputs", inputs.Missing(synth, comp)) + logger.Info("composition is missing required inputs", "missingInputs", inputs.Missing(synth, comp), "expectedInputs", inputs.Expected(synth)) case "MismatchedInputs": logger.Info("composition has inputs that are out of lockstep", "mismatchedInputs", inputs.Mismatched(synth, comp, comp.Status.InputRevisions), "synthesizerGeneration", synth.Generation, "compositionGeneration", comp.Generation) } diff --git a/internal/inputs/inputs.go b/internal/inputs/inputs.go index 4e2ad345..f07c8160 100644 --- a/internal/inputs/inputs.go +++ b/internal/inputs/inputs.go @@ -32,6 +32,18 @@ func Missing(syn *apiv1.Synthesizer, c *apiv1.Composition) []string { return missing } +// Expected returns the keys of all non-optional inputs declared by the synthesizer. +func Expected(syn *apiv1.Synthesizer) []string { + var expected []string + for _, ref := range syn.Spec.Refs { + if ref.Optional { + continue + } + expected = append(expected, ref.Key) + } + return expected +} + // OutOfLockstep returns true when one or more inputs that specify a revision do not match the others. // It also returns true if any revision is derived from a synthesizer/composition generation older than the ones provided. func OutOfLockstep(synth *apiv1.Synthesizer, comp *apiv1.Composition, revs []apiv1.InputRevisions) bool { diff --git a/internal/inputs/inputs_test.go b/internal/inputs/inputs_test.go index 06972d32..6cb64678 100644 --- a/internal/inputs/inputs_test.go +++ b/internal/inputs/inputs_test.go @@ -618,6 +618,79 @@ func TestMissing(t *testing.T) { } } +func TestExpected(t *testing.T) { + tests := []struct { + Name string + Synthesizer apiv1.Synthesizer + Expectation []string + }{ + { + Name: "All required refs", + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "key1"}, + {Key: "key2"}, + }, + }, + }, + Expectation: []string{"key1", "key2"}, + }, + { + Name: "Optional refs excluded", + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "key1"}, + {Key: "key2", Optional: true}, + {Key: "key3"}, + }, + }, + }, + Expectation: []string{"key1", "key3"}, + }, + { + Name: "All optional refs", + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "key1", Optional: true}, + {Key: "key2", Optional: true}, + }, + }, + }, + Expectation: nil, + }, + { + Name: "No refs declared", + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{Refs: []apiv1.Ref{}}, + }, + Expectation: nil, + }, + { + Name: "Spec order is preserved", + Synthesizer: apiv1.Synthesizer{ + Spec: apiv1.SynthesizerSpec{ + Refs: []apiv1.Ref{ + {Key: "c"}, + {Key: "a"}, + {Key: "b"}, + }, + }, + }, + Expectation: []string{"c", "a", "b"}, + }, + } + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + result := Expected(&tt.Synthesizer) + assert.Equal(t, tt.Expectation, result) + }) + } +} + func TestMismatched(t *testing.T) { revision1 := 1 revision2 := 2 From b187606b65075625e4c93980cb8a73c174e1e3e4 Mon Sep 17 00:00:00 2001 From: Ruinan Liu Date: Fri, 15 May 2026 20:03:52 +0000 Subject: [PATCH 3/3] resolve comments --- .../controllers/composition/controller.go | 12 ++--- internal/inputs/inputs.go | 44 ++++++++++++------- internal/inputs/inputs_test.go | 10 ++--- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/internal/controllers/composition/controller.go b/internal/controllers/composition/controller.go index 422e4c0b..36166508 100644 --- a/internal/controllers/composition/controller.go +++ b/internal/controllers/composition/controller.go @@ -33,6 +33,8 @@ const ( AKSComponentLabel = "aks.azure.com/component-type" // TODO(ruinanliu): Temp workaround remove after 14802391 is released addOnLabelValue = "addon" // TODO(ruinanliu): Temp workaround remove after 14802391 is released EnoCleanupFinalizer = "eno.azure.io/cleanup" + MissingInputStatus = "MissingInputs" + MismatchedInputsStatus = "MismatchedInputs" ) type compositionController struct { @@ -262,11 +264,11 @@ func (c *compositionController) reconcileSimplifiedStatus(ctx context.Context, s return false, nil } - if next != nil && synth != nil { + if synth != nil { switch next.Status { - case "MissingInputs": + case MissingInputStatus: logger.Info("composition is missing required inputs", "missingInputs", inputs.Missing(synth, comp), "expectedInputs", inputs.Expected(synth)) - case "MismatchedInputs": + case MismatchedInputsStatus: logger.Info("composition has inputs that are out of lockstep", "mismatchedInputs", inputs.Mismatched(synth, comp, comp.Status.InputRevisions), "synthesizerGeneration", synth.Generation, "compositionGeneration", comp.Generation) } } @@ -392,11 +394,11 @@ func buildSimplifiedStatus(synth *apiv1.Synthesizer, comp *apiv1.Composition) *a } if !inputs.Exist(synth, comp) { - status.Status = "MissingInputs" + status.Status = MissingInputStatus return status } if inputs.OutOfLockstep(synth, comp, comp.Status.InputRevisions) { - status.Status = "MismatchedInputs" + status.Status = MismatchedInputsStatus } if comp.Status.CurrentSynthesis == nil && comp.Status.InFlightSynthesis == nil { diff --git a/internal/inputs/inputs.go b/internal/inputs/inputs.go index f07c8160..6aa0b37a 100644 --- a/internal/inputs/inputs.go +++ b/internal/inputs/inputs.go @@ -50,14 +50,15 @@ func OutOfLockstep(synth *apiv1.Synthesizer, comp *apiv1.Composition, revs []api return len(Mismatched(synth, comp, revs)) > 0 } -// MismatchedInput describes a single input revision that is out of lockstep with the others -// (or with the current synthesizer/composition generation). It is intended for logging/telemetry. +// MismatchedInput describes a single input revision that is out of lockstep, +// either with peer revisions or with the current synthesizer/composition +// generation. Intended for logging only. Numeric fields are 0 when unknown. type MismatchedInput struct { Key string - Revision *int - MaxRevision *int - SynthesizerGeneration *int64 - CompositionGeneration *int64 + Revision int // 0 if unset + MaxRevision int // 0 if no peer revision was observed + SynthesizerGeneration int64 // 0 if unset + CompositionGeneration int64 // 0 if unset } // Mismatched returns the set of input revisions that are out of lockstep with the others, or @@ -66,13 +67,15 @@ func Mismatched(synth *apiv1.Synthesizer, comp *apiv1.Composition, revs []apiv1. var mismatched []MismatchedInput // First, find the max revision across all bindings - var maxRevision *int + maxRevision := 0 + maxSet := false for _, rev := range revs { if rev.Revision == nil { continue } - if maxRevision == nil || *rev.Revision > *maxRevision { - maxRevision = rev.Revision + if !maxSet || *rev.Revision > maxRevision { + maxRevision = *rev.Revision + maxSet = true } } @@ -84,17 +87,24 @@ func Mismatched(synth *apiv1.Synthesizer, comp *apiv1.Composition, revs []apiv1. if rev.CompositionGeneration != nil && *rev.CompositionGeneration < comp.Generation { stale = true } - revMismatch := maxRevision != nil && rev.Revision != nil && *rev.Revision != *maxRevision + revMismatch := maxSet && rev.Revision != nil && *rev.Revision != maxRevision if !stale && !revMismatch { continue } - mismatched = append(mismatched, MismatchedInput{ - Key: rev.Key, - Revision: rev.Revision, - MaxRevision: maxRevision, - SynthesizerGeneration: rev.SynthesizerGeneration, - CompositionGeneration: rev.CompositionGeneration, - }) + entry := MismatchedInput{Key: rev.Key} + if maxSet { + entry.MaxRevision = maxRevision + } + if rev.Revision != nil { + entry.Revision = *rev.Revision + } + if rev.SynthesizerGeneration != nil { + entry.SynthesizerGeneration = *rev.SynthesizerGeneration + } + if rev.CompositionGeneration != nil { + entry.CompositionGeneration = *rev.CompositionGeneration + } + mismatched = append(mismatched, entry) } return mismatched } diff --git a/internal/inputs/inputs_test.go b/internal/inputs/inputs_test.go index 6cb64678..c733b2cf 100644 --- a/internal/inputs/inputs_test.go +++ b/internal/inputs/inputs_test.go @@ -734,8 +734,8 @@ func TestMismatched(t *testing.T) { }, }, Expectation: []MismatchedInput{ - {Key: "a", Revision: &revision1, MaxRevision: &revision2}, - {Key: "c", Revision: &revision1, MaxRevision: &revision2}, + {Key: "a", Revision: revision1, MaxRevision: revision2}, + {Key: "c", Revision: revision1, MaxRevision: revision2}, }, }, { @@ -763,7 +763,7 @@ func TestMismatched(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Generation: 123}, }, Expectation: []MismatchedInput{ - {Key: "a", SynthesizerGeneration: ptr.To(int64(122))}, + {Key: "a", SynthesizerGeneration: 122}, }, }, { @@ -777,7 +777,7 @@ func TestMismatched(t *testing.T) { }, }, Expectation: []MismatchedInput{ - {Key: "a", CompositionGeneration: ptr.To(int64(4))}, + {Key: "a", CompositionGeneration: 4}, }, }, { @@ -792,7 +792,7 @@ func TestMismatched(t *testing.T) { }, }, Expectation: []MismatchedInput{ - {Key: "a", Revision: &revision1, MaxRevision: &revision2, CompositionGeneration: ptr.To(int64(4))}, + {Key: "a", Revision: revision1, MaxRevision: revision2, CompositionGeneration: 4}, }, }, }