diff --git a/api/v1/composition.go b/api/v1/composition.go index 03f00141..b94e4965 100644 --- a/api/v1/composition.go +++ b/api/v1/composition.go @@ -50,11 +50,10 @@ type ResourceInputRef struct { } type CompositionStatus struct { - CurrentState *Synthesis `json:"currentState,omitempty"` - PreviousState *Synthesis `json:"previousState,omitempty"` + Syntheses []*Synthesis `json:"syntheses,omitempty"` } -// Synthesis represents a Synthesizer's specific synthesis of a given Composition. +// Synthesis represents a Synthesizer's synthesis of a Composition. type Synthesis struct { ObservedCompositionGeneration int64 `json:"observedCompositionGeneration,omitempty"` ObservedSynthesizerGeneration int64 `json:"observedSynthesizerGeneration,omitempty"` diff --git a/api/v1/config/crd/eno.azure.io_compositions.yaml b/api/v1/config/crd/eno.azure.io_compositions.yaml index 962b8991..ba411d21 100644 --- a/api/v1/config/crd/eno.azure.io_compositions.yaml +++ b/api/v1/config/crd/eno.azure.io_compositions.yaml @@ -72,54 +72,32 @@ spec: type: object status: properties: - currentState: - description: Synthesis represents a Synthesizer's specific synthesis - of a given Composition. - properties: - observedCompositionGeneration: - format: int64 - type: integer - observedSynthesizerGeneration: - format: int64 - type: integer - podCreation: - format: date-time - type: string - ready: - type: boolean - resourceSliceCount: - description: Number of resulting resource slices. Since they are - immutable, this provides adequate timing signal to avoid stale - informer caches. - format: int64 - type: integer - synced: - type: boolean - type: object - previousState: - description: Synthesis represents a Synthesizer's specific synthesis - of a given Composition. - properties: - observedCompositionGeneration: - format: int64 - type: integer - observedSynthesizerGeneration: - format: int64 - type: integer - podCreation: - format: date-time - type: string - ready: - type: boolean - resourceSliceCount: - description: Number of resulting resource slices. Since they are - immutable, this provides adequate timing signal to avoid stale - informer caches. - format: int64 - type: integer - synced: - type: boolean - type: object + syntheses: + items: + description: Synthesis represents a Synthesizer's synthesis of a + Composition. + properties: + observedCompositionGeneration: + format: int64 + type: integer + observedSynthesizerGeneration: + format: int64 + type: integer + podCreation: + format: date-time + type: string + ready: + type: boolean + resourceSliceCount: + description: Number of resulting resource slices. Since they + are immutable, this provides adequate timing signal to avoid + stale informer caches. + format: int64 + type: integer + synced: + type: boolean + type: object + type: array type: object type: object served: true diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index b4426d9c..3a9a94cd 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -100,15 +100,16 @@ func (in *CompositionSpec) DeepCopy() *CompositionSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CompositionStatus) DeepCopyInto(out *CompositionStatus) { *out = *in - if in.CurrentState != nil { - in, out := &in.CurrentState, &out.CurrentState - *out = new(Synthesis) - (*in).DeepCopyInto(*out) - } - if in.PreviousState != nil { - in, out := &in.PreviousState, &out.PreviousState - *out = new(Synthesis) - (*in).DeepCopyInto(*out) + if in.Syntheses != nil { + in, out := &in.Syntheses, &out.Syntheses + *out = make([]*Synthesis, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Synthesis) + (*in).DeepCopyInto(*out) + } + } } } diff --git a/internal/controllers/synthesis/integration_test.go b/internal/controllers/synthesis/integration_test.go index a447a159..7d4ccef8 100644 --- a/internal/controllers/synthesis/integration_test.go +++ b/internal/controllers/synthesis/integration_test.go @@ -16,6 +16,8 @@ import ( "github.com/Azure/eno/internal/testutil" ) +// TODO: Document that we never block forward movement + var minimalTestConfig = &Config{ WrapperImage: "test-wrapper-image", MaxRestarts: 3, diff --git a/internal/controllers/synthesis/lifecycle.go b/internal/controllers/synthesis/lifecycle.go index ba1de5d6..54f82e9b 100644 --- a/internal/controllers/synthesis/lifecycle.go +++ b/internal/controllers/synthesis/lifecycle.go @@ -14,6 +14,11 @@ import ( "github.com/Azure/eno/internal/manager" ) +// TODO: some notes +// - We should be able to spawn more than one pod per composition concurrently +// - We should delete pods that are no longer needed +// - We should trim states that are no longer needed unless they've been generated + type Config struct { WrapperImage string JobSA string @@ -83,17 +88,19 @@ func (c *podLifecycleController) Reconcile(ctx context.Context, req ctrl.Request } // Swap the state to prepare for resynthesis if needed - if comp.Status.CurrentState == nil || comp.Status.CurrentState.ObservedCompositionGeneration != comp.Generation { - swapStates(syn, comp) + if len(comp.Status.Syntheses) == 0 || comp.Status.Syntheses[0].ObservedCompositionGeneration != comp.Generation { + // TODO: Should we stop re-synthesis at a certain slice length? + comp.Status.Syntheses = append([]*apiv1.Synthesis{{ObservedCompositionGeneration: comp.Generation}}, comp.Status.Syntheses...) + if err := c.client.Status().Update(ctx, comp); err != nil { return ctrl.Result{}, fmt.Errorf("swapping compisition state: %w", err) } - logger.Info("swapped composition state because composition was modified since last synthesis") + logger.Info("started a new synthesis because the composition was modified since last synthesis") return ctrl.Result{}, nil } // No need to create a pod if everything is in sync - if comp.Status.CurrentState != nil && comp.Status.CurrentState.ResourceSliceCount != nil { + if len(comp.Status.Syntheses) > 0 && comp.Status.Syntheses[0].ResourceSliceCount != nil { return ctrl.Result{}, nil } @@ -128,6 +135,7 @@ func (c *podLifecycleController) shouldDeletePod(logger logr.Logger, comp *apiv1 } // Only create pods when the previous one is deleting or non-existant + // TODO: Should we create pods for new generations? for _, pod := range pods.Items { pod := pod reason, shouldDelete := c.podStatusTerminal(&pod) @@ -183,10 +191,3 @@ func (c *podLifecycleController) podStatusTerminal(pod *corev1.Pod) (string, boo } return "", false // status not initialized yet } - -func swapStates(syn *apiv1.Synthesizer, comp *apiv1.Composition) { - comp.Status.PreviousState = comp.Status.CurrentState - comp.Status.CurrentState = &apiv1.Synthesis{ - ObservedCompositionGeneration: comp.Generation, - } -} diff --git a/internal/controllers/synthesis/status.go b/internal/controllers/synthesis/status.go index 983b0bc5..df650147 100644 --- a/internal/controllers/synthesis/status.go +++ b/internal/controllers/synthesis/status.go @@ -71,14 +71,18 @@ func (c *statusController) Reconcile(ctx context.Context, req ctrl.Request) (ctr compGen, _ = strconv.ParseInt(pod.Annotations["eno.azure.io/composition-generation"], 10, 0) synGen, _ = strconv.ParseInt(pod.Annotations["eno.azure.io/synthesizer-generation"], 10, 0) ) - if statusIsOutOfSync(comp, compGen, synGen) { - comp.Status.CurrentState.PodCreation = &pod.CreationTimestamp - comp.Status.CurrentState.ObservedSynthesizerGeneration = synGen + for _, synthesis := range comp.Status.Syntheses { + // TODO: Should we allow retry pods to be created before old ones terminate? + if synthesis.ObservedCompositionGeneration != compGen || synthesis.PodCreation != nil && synthesis.ObservedSynthesizerGeneration != nil { + continue + } + synthesis.PodCreation = &pod.CreationTimestamp + synthesis.ObservedSynthesizerGeneration = synGen if err := c.client.Status().Update(ctx, comp); err != nil { return ctrl.Result{}, fmt.Errorf("updating composition status: %w", err) } - logger.V(1).Info("added synthesizer pod status to its composition resource") + logger.V(1).Info("added synthesizer pod status to its composition resource", "compositionGeneration", compGen) return ctrl.Result{}, nil } @@ -93,9 +97,3 @@ func (c *statusController) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, nil } - -func statusIsOutOfSync(comp *apiv1.Composition, podCompGen, podSynGen int64) bool { - // TODO: Unit tests, make sure to cover the pod creation latching logic - return (comp.Status.CurrentState != nil && comp.Status.CurrentState.ObservedCompositionGeneration == podCompGen) && - (comp.Status.CurrentState.PodCreation == nil || comp.Status.CurrentState.ObservedSynthesizerGeneration != podSynGen) -}