Skip to content

Commit

Permalink
fix(controller): record event only when freight verification is perfo…
Browse files Browse the repository at this point in the history
…rmed (akuity#1819)

Signed-off-by: Sunghoon Kang <hoon@akuity.io>
  • Loading branch information
Sunghoon Kang authored Apr 11, 2024
1 parent 92eb0e7 commit b607082
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 48 deletions.
35 changes: 22 additions & 13 deletions internal/controller/stages/stages.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ type reconciler struct {
namespace string,
freightName string,
stageName string,
) error
) (bool, error)

patchFreightStatusFn func(
ctx context.Context,
Expand Down Expand Up @@ -716,6 +716,7 @@ func (r *reconciler) syncNormalStage(
)
} else {
freightLogger := logger.WithField("freight", status.CurrentFreight.Name)
shouldRecordFreightVerificationEvent := false

// Push the latest state of the current Freight to the history at the
// end of each reconciliation loop.
Expand Down Expand Up @@ -815,6 +816,7 @@ func (r *reconciler) syncNormalStage(

if status.CurrentFreight.VerificationInfo.Phase.IsTerminal() {
// Verification is complete
shouldRecordFreightVerificationEvent = true
status.Phase = kargoapi.StagePhaseSteady
log.Debug("verification is complete")
}
Expand All @@ -834,12 +836,13 @@ func (r *reconciler) syncNormalStage(
(stage.Spec.Verification == nil ||
(status.CurrentFreight.VerificationInfo != nil &&
status.CurrentFreight.VerificationInfo.Phase == kargoapi.VerificationPhaseSuccessful)) {
if err := r.verifyFreightInStageFn(
updated, err := r.verifyFreightInStageFn(
ctx,
stage.Namespace,
status.CurrentFreight.Name,
stage.Name,
); err != nil {
)
if err != nil {
return status, fmt.Errorf(
"error marking Freight %q in namespace %q as verified in Stage %q: %w",
status.CurrentFreight.Name,
Expand All @@ -848,12 +851,15 @@ func (r *reconciler) syncNormalStage(
err,
)
}

// Always record verification event when the Freight is marked as verified
if updated {
shouldRecordFreightVerificationEvent = true
}
}

// Record an event if the verification process has completed.
if stage.Spec.Verification == nil ||
(status.CurrentFreight.VerificationInfo != nil &&
status.CurrentFreight.VerificationInfo.Phase.IsTerminal()) {
// Record freight verification event only if the freight is newly verified
if shouldRecordFreightVerificationEvent {
vi := status.CurrentFreight.VerificationInfo
if stage.Spec.Verification == nil {
vi = &kargoapi.VerificationInfo{
Expand Down Expand Up @@ -1162,12 +1168,15 @@ func (r *reconciler) hasNonTerminalPromotions(
return len(promos.Items) > 0, nil
}

// verifyFreightInStage marks the given Freight as verified in the given Stage.
// It returns true if succeeded to mark Freight as verified in the Stage,
// or false if it was already marked as verified in the Stage.
func (r *reconciler) verifyFreightInStage(
ctx context.Context,
namespace string,
freightName string,
stageName string,
) error {
) (bool, error) {
logger := logging.LoggerFromContext(ctx).WithField("freight", freightName)

// Find the Freight
Expand All @@ -1180,15 +1189,15 @@ func (r *reconciler) verifyFreightInStage(
},
)
if err != nil {
return fmt.Errorf(
return false, fmt.Errorf(
"error finding Freight %q in namespace %q: %w",
freightName,
namespace,
err,
)
}
if freight == nil {
return fmt.Errorf(
return false, fmt.Errorf(
"found no Freight %q in namespace %q",
freightName,
namespace,
Expand All @@ -1203,16 +1212,16 @@ func (r *reconciler) verifyFreightInStage(
// Only try to mark as verified in this Stage if not already the case.
if _, ok := newStatus.VerifiedIn[stageName]; ok {
logger.Debug("Freight already marked as verified in Stage")
return nil
return false, nil
}

newStatus.VerifiedIn[stageName] = kargoapi.VerifiedStage{}
if err = r.patchFreightStatusFn(ctx, freight, newStatus); err != nil {
return err
return false, err
}

logger.Debug("marked Freight as verified in Stage")
return nil
return true, nil
}

func (r *reconciler) patchFreightStatus(
Expand Down
Loading

0 comments on commit b607082

Please sign in to comment.