Skip to content

Commit

Permalink
Fix another race
Browse files Browse the repository at this point in the history
  • Loading branch information
Jordan Olshevski committed Dec 13, 2023
1 parent b8d71a3 commit 4a327ad
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 9 deletions.
2 changes: 1 addition & 1 deletion internal/controllers/synthesis/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (c *podLifecycleController) shouldDeletePod(logger logr.Logger, comp *apiv1
for _, pod := range pods.Items {
pod := pod
reason, shouldDelete := c.podStatusTerminal(&pod)
isCurrent := podDerivedFrom(comp, syn, &pod)
isCurrent := podDerivedFrom(comp, syn, &pod) && len(comp.Status.CurrentState.ResourceSlices) != 0

// If the current pod is being deleted it's safe to create a new one if needed
// Avoid getting stuck by pods that fail to delete
Expand Down
16 changes: 8 additions & 8 deletions internal/controllers/synthesis/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@ func (c *statusController) Reconcile(ctx context.Context, req ctrl.Request) (ctr
synGen, _ = strconv.ParseInt(pod.Annotations["eno.azure.io/synthesizer-generation"], 10, 0)
)
logger.WithValues("synthesizerGeneration", synGen, "compositionGeneration", compGen)
if statusIsOutOfSync(comp, compGen, synGen) {
if shouldWriteStatus(comp, compGen, synGen) {
comp.Status.CurrentState.PodCreation = &pod.CreationTimestamp
comp.Status.CurrentState.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("wrote synthesizer pod metadata to composition")
return ctrl.Result{}, nil
return ctrl.Result{Requeue: true}, nil
}

// Remove the finalizer
Expand All @@ -89,15 +89,15 @@ func (c *statusController) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, fmt.Errorf("removing pod finalizer: %w", err)
}
logger.V(1).Info("synthesizer pod can safely be deleted now that its metadata has been captured")
return ctrl.Result{}, nil
return ctrl.Result{Requeue: true}, nil
}

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
// TODO: Do we need to also check against the previous state? Do we swap if the current state is still being synthesized? Should we?
return (comp.Status.CurrentState != nil && comp.Status.CurrentState.ObservedCompositionGeneration == podCompGen) &&
(comp.Status.CurrentState.PodCreation == nil || comp.Status.CurrentState.ObservedSynthesizerGeneration != podSynGen)
func shouldWriteStatus(comp *apiv1.Composition, podCompGen, podSynGen int64) bool {
current := comp.Status.CurrentState
isOldPod := current == nil || current.ObservedCompositionGeneration != podCompGen
isInSync := current != nil && current.PodCreation != nil
return !isOldPod && !isInSync
}

0 comments on commit 4a327ad

Please sign in to comment.