diff --git a/internal/controllers/composition/controller.go b/internal/controllers/composition/controller.go index 6f496dbf..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,6 +264,15 @@ func (c *compositionController) reconcileSimplifiedStatus(ctx context.Context, s return false, nil } + if synth != nil { + switch next.Status { + case MissingInputStatus: + logger.Info("composition is missing required inputs", "missingInputs", inputs.Missing(synth, comp), "expectedInputs", inputs.Expected(synth)) + 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) + } + } + logger.Info("composition status changed", "previousStatus", comp.Status.Simplified, "currentStatus", next) copy := comp.DeepCopy() copy.Status.Simplified = next @@ -383,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 f5b0992e..6aa0b37a 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,85 @@ func Exist(syn *apiv1.Synthesizer, c *apiv1.Composition) bool { return ref.Key == current.Key }) if !found { - return false + missing = append(missing, ref.Key) + } + } + 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 true + 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 { - // First, the the max revision across all bindings - var maxRevision *int + return len(Mismatched(synth, comp, revs)) > 0 +} + +// 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 // 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 +// 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 + maxRevision := 0 + maxSet := false for _, rev := range revs { + if rev.Revision == nil { + continue + } + if !maxSet || *rev.Revision > maxRevision { + maxRevision = *rev.Revision + maxSet = true + } + } + + for _, rev := range revs { + stale := false if rev.SynthesizerGeneration != nil && *rev.SynthesizerGeneration < synth.Generation { - return true + stale = true } if rev.CompositionGeneration != nil && *rev.CompositionGeneration < comp.Generation { - return true + stale = true } - if rev.Revision == nil { + revMismatch := maxSet && rev.Revision != nil && *rev.Revision != maxRevision + if !stale && !revMismatch { continue } - if maxRevision == nil { - maxRevision = rev.Revision - continue + entry := MismatchedInput{Key: rev.Key} + if maxSet { + entry.MaxRevision = maxRevision } - if *rev.Revision > *maxRevision { - maxRevision = rev.Revision + if rev.Revision != nil { + entry.Revision = *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 + if rev.SynthesizerGeneration != nil { + entry.SynthesizerGeneration = *rev.SynthesizerGeneration + } + if rev.CompositionGeneration != nil { + entry.CompositionGeneration = *rev.CompositionGeneration } + mismatched = append(mismatched, entry) } - return false + return mismatched } diff --git a/internal/inputs/inputs_test.go b/internal/inputs/inputs_test.go index 38a06ef6..c733b2cf 100644 --- a/internal/inputs/inputs_test.go +++ b/internal/inputs/inputs_test.go @@ -491,3 +491,318 @@ 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 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 + + 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: 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: 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: 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)) + }) + } +}