Skip to content

feat: rework autoRollback spec to optionally support Error phases#6442

Open
jessesuen wants to merge 1 commit into
akuity:mainfrom
jessesuen:feat/rework-rollback-spec
Open

feat: rework autoRollback spec to optionally support Error phases#6442
jessesuen wants to merge 1 commit into
akuity:mainfrom
jessesuen:feat/rework-rollback-spec

Conversation

@jessesuen

Copy link
Copy Markdown
Member

Description

This is a CRD spec change. The original autoRollback spec looked like the following

apiVersion: kargo.akuity.io/v1alpha1
kind: ProjectConfig
metadata:
  name: auto-rollback
  namespace: auto-rollback
spec:
  promotionPolicies:
  - stageSelector:
      name: staging
    autoRollback:
      onFailedPromotions: true # <<<--- optional. If omitted, will not rollback on failed Promotions, only failed verifications

The above has a limitation: we only consider Failed promotions/verifications, not Error. However, I think many users don't really distinguish between failures and errors, so I want to support Errors as an option. This change modifies it to:

apiVersion: kargo.akuity.io/v1alpha1
kind: ProjectConfig
metadata:
  name: auto-rollback
  namespace: auto-rollback
spec:
  promotionPolicies:
  - stageSelector:
      name: staging
    autoRollback:
      # list of unsuccessful terminal phases. Only `Failure` and `Errored` are accepted
      onPromotion: # defaults to []
      - Failure
      - Errored
      onVerification: # defaults to [Failure]
      - Failure
      - Error # < NOTE> this verification phases uses `Error` and not `Errored` used by promotion phases. Will call this out in docs later.

The default, if they just specify: autoRollback: {}  would be equivalent to:

apiVersion: kargo.akuity.io/v1alpha1
kind: ProjectConfig
metadata:
  name: auto-rollback
  namespace: auto-rollback
spec:
  promotionPolicies:
  - stageSelector:
      name: staging
    autoRollback:
      onPromotion: []              # default, no rollback
      onVerification: [Failed]     # default

Checklist

Eligibility

(types change only, not counting generated code)

  • Changes ten lines or fewer.

Quality

  • Adds or updates corresponding tests.
  • Adds or updates corresponding documentation.

AI Use Disclosure

This PR was written:

  • By an AI with human supervision. A human has reviewed every line prior to opening the PR.

Sign-Off

All commits:

  • Are signed off by their author (git commit -s) (required)
  • Are cryptographically signed (git commit -S) (encouraged)

@jessesuen jessesuen requested review from a team as code owners June 10, 2026 20:58
@kargo-governance-bot kargo-governance-bot Bot added needs/area Issue or PR needs to be labeled to indicate what parts of the code base are affected needs/kind Issue or PR needs to be labeled to clarify its nature needs/priority Priority has not yet been determined; a good signal that maintainers aren't fully committed labels Jun 10, 2026
@netlify

netlify Bot commented Jun 10, 2026

Copy link
Copy Markdown

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit f655204
🔍 Latest deploy log https://app.netlify.com/projects/docs-kargo-io/deploys/6a29eb558fb22900082677a7
😎 Deploy Preview https://deploy-preview-6442.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@jessesuen jessesuen force-pushed the feat/rework-rollback-spec branch 2 times, most recently from e76d53b to e30f2e9 Compare June 10, 2026 21:15
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.47%. Comparing base (8296b8e) to head (f655204).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6442   +/-   ##
=======================================
  Coverage   58.47%   58.47%           
=======================================
  Files         501      501           
  Lines       42141    42141           
=======================================
  Hits        24642    24642           
  Misses      16011    16011           
  Partials     1488     1488           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Jesse Suen <jesse@akuity.io>
@jessesuen jessesuen force-pushed the feat/rework-rollback-spec branch from e30f2e9 to f655204 Compare June 10, 2026 22:55

@EronWright EronWright left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The disparity between Error and Errored is pretty glaring, and assumedly comes from using preexisting enums PromotionPhase and VerificationPhase correct? Is it worth it?

I assume the logic changes would come in a followup.

@jessesuen

jessesuen commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

The disparity between Error and Errored is pretty glaring, and assumedly comes from using preexisting enums PromotionPhase and VerificationPhase correct?

Yes, it's unfortunate, and it's because of the historical difference in the enums from different projects (kargo vs. argo rollouts).

Is it worth it?

Perhaps. For example, Verifications support an Inconclusive phase, but Promotions do not. If we decide to standardize on promotion phase Errored instead of Error, but then later decide to support rollback if the verification is Inconclusive, we'll be in a very indefensible spot.

I assume the logic changes would come in a followup.

Correct, they are not in this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs/area Issue or PR needs to be labeled to indicate what parts of the code base are affected needs/kind Issue or PR needs to be labeled to clarify its nature needs/priority Priority has not yet been determined; a good signal that maintainers aren't fully committed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants