Skip to content

feat(ci): add the weekly candidate-to-stable promotion workflow - #344

Draft
cbartz wants to merge 8 commits into
mainfrom
feat/promote-candidate-to-stable
Draft

feat(ci): add the weekly candidate-to-stable promotion workflow#344
cbartz wants to merge 8 commits into
mainfrom
feat/promote-candidate-to-stable

Conversation

@cbartz

@cbartz cbartz commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds .github/workflows/promote_candidate_to_stable.yaml: a weekly workflow that promotes
the garm and garm-configurator revisions sitting in latest/candidate to latest/stable
once they have soaked for SOAK_DAYS (7, set as a workflow-level variable).

The check job does the arithmetic and always writes a verdict table to the job summary.
The release job is gated on a charmhub-stable environment with required reviewers, so
the run parks for human approval before anything is released.

Why we need it

Part of ISD-5877. Promotion to stable is manual today. This is independent of the rest of
the series — it never invokes the end-to-end suite — so it can merge on its own.

Checklist

  • Changes comply with the project's coding standards and guidelines (see CONTRIBUTING.md and STYLE.md)
  • CONTRIBUTING.md has been updated upon changes to the contribution/development process (e.g. changes to the way tests are run)
  • Technical author has been assigned to review the PR in case of documentation changes (usually *.md files)
  • I updated docs/changelog.md with user-relevant changes
  • I used AI to assist with preparing this PR
  • I added or updated tests as needed (unit and integration)
  • If integration test modules are used: I updated the workflow configuration
  • If this PR involves a Grafana dashboard: I added a screenshot of the dashboard
  • If this PR involves Terraform: terraform fmt passes and tflint reports no errors
  • If this PR involves Rockcraft: I updated the version
  • If this PR adds/removes a charm, or changes a charm's base class, conventions, tooling, or repo structure: I updated the relevant AGENTS.md
  • If this PR changes .copilot-collections.yaml or .github/instructions/: I re-checked whether the AGENTS.md "12-factor divergences" guidance still matches the upstream copilot-collections guidance

Not applicable: no Terraform, Rockcraft, dashboard or charm-structure changes. Tests are
unticked deliberately — this adds a scheduled workflow with no code under test; its logic was
verified by simulation instead. The changelog entry and the process documentation ride with
the docs PR.

Test plan

The check job's script was extracted and executed directly:

  • Against the live Charmhub API: both charms currently have an empty candidate
    channel → eligible=false, held back with "no candidate release is available to promote".
    This is the real current state and the case most likely to crash.
  • Against a stubbed API: both soaked 9 days → eligible; both 3 days old → held back,
    reporting 4.0 days of soak remaining; one soaked and one young → held back, confirming
    the atomic rule; candidate already equal to stable → held back; candidate present with no
    stable at all → eligible.
  • The stub included decoy entries on another track and another architecture; the filter
    rejected both.
  • actionlint: clean, no findings.
  • YAML parse check with yaml.safe_load: ok.

Review focus

  • The charmhub-stable environment must be created manually in repository settings with
    required reviewers, and a CHARMHUB_STABLE_TOKEN secret scoped to that environment (not
    the repo-level CHARMHUB_TOKEN used by the edge publish workflow). An undefined
    environment does not fail
    — GitHub treats it as metadata and the job simply does not
    pause, so the approval gate would silently be a no-op until it exists. Using a
    dedicated environment secret, and failing fast if it is empty, closes the gap a Copilot
    review round caught: reusing the repo-level CHARMHUB_TOKEN would have let the job
    release to stable with valid credentials even with no environment configured. This is
    the one setup step that cannot be done from the repository.
  • The soak signal is indirect, and this is deliberate. It measures how long the revision
    has been in the candidate channel, which does not prove production ever ran it — the
    production bump is a separate PR in the internal gitops repository and may be unmerged or
    unapplied. The ticket proposes deriving the soak from that repository's commit history
    instead; that was traded away to avoid putting a private-repository read credential in a
    public repository. The reviewer at the approval gate is what closes the gap, and their
    job is precisely to confirm the revision really has been running in production. If those
    approvals become a rubber stamp, this should be revisited.
  • Atomic promotion: if either charm is not eligible, neither is promoted. A failure
    between the two charmcraft release calls is trapped and reported as leaving stable
    inconsistent and needing manual repair.
  • CHARMHUB_STABLE_TOKEN must hold exported credentials from charmcraft login --export (not a raw token), matching the repo's existing CHARMHUB_TOKEN convention, and must carry package-manage-releases for both charms.
  • This workflow must not be added as a required status check: it never runs on a pull
    request, so requiring it would block every merge.

A dispatch raised while the scheduled run is parked at the approval gate could
otherwise be approved alongside it and release the same revisions twice.
The step summary printed the "Released to stable" heading before the first
charmcraft release ran, so a failed release produced a summary that claimed a
release had happened and then contradicted itself. Emit the heading with the
per-charm lines once both releases have succeeded, matching the edge-to-candidate
workflow.
@cbartz
cbartz force-pushed the feat/promote-candidate-to-stable branch from 658bbb3 to 8ef93c4 Compare September 1, 2026 13:11
@cbartz
cbartz requested a lite review from Copilot September 4, 2026 14:22

Copilot AI 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.

🟡 Changes recommended

The stable promotion approval gate can be bypassed if the environment is missing/misconfigured because the workflow currently uses a repo-level Charmhub secret that already exists for other releases.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a new GitHub Actions workflow to automate weekly promotion of the garm and garm-configurator charms from latest/candidate to latest/stable after a soak window, aligning with the repo’s Charmhub publishing automation.

Changes:

  • Introduces a scheduled + manually-dispatchable workflow that queries Charmhub’s channel-map to compute soak age and promotion eligibility.
  • Emits a markdown verdict table in the job summary on every run, including held-back reasons.
  • Adds a gated release job intended to pause behind a charmhub-stable environment before running charmcraft release for both charms.
File summaries
File Description
.github/workflows/promote_candidate_to_stable.yaml New scheduled workflow to check candidate soak and (after approval) promote both charms to stable atomically.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/promote_candidate_to_stable.yaml
Comment thread .github/workflows/promote_candidate_to_stable.yaml
Use a dedicated CHARMHUB_STABLE_TOKEN secret scoped to the charmhub-stable
environment instead of the repo-level CHARMHUB_TOKEN, and fail fast if it
is empty, so an unconfigured/missing environment cannot silently bypass
the human approval gate. Also show n/a for stable revision in the summary
table when no stable release exists yet, instead of the misleading 0.

Copilot AI 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.

🔵 Needs a closer look

The workflow’s timestamp parsing is brittle for external API data and should be hardened to avoid scheduled-run failures with unclear tracebacks.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/promote_candidate_to_stable.yaml:90

  • age_days() assumes Charmhub’s released-at string is always directly accepted by datetime.fromisoformat(). If Charmhub returns RFC3339 timestamps with a trailing Z (or any unexpected format), this will raise and the job will fail with a traceback rather than a clear ::error:: message. Consider normalizing Z to +00:00, validating timezone presence, and failing via fail() on parse errors.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Normalize a trailing Z (RFC3339, which fromisoformat rejects) before
parsing, and fail with a clear ::error:: message instead of a bare
traceback if Charmhub ever returns an unparseable or timezone-naive
timestamp.

Copilot AI 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.

🔵 Needs a closer look

The eligibility script can incorrectly proceed (or crash with a traceback) when Charmhub responses omit expected fields, so it should validate required data and fail with clear ::error:: output before this workflow is relied on for stable promotions.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

.github/workflows/promote_candidate_to_stable.yaml:112

  • If Charmhub returns a candidate entry without a revision.revision field, candidate_revision becomes None and the eligibility check can still evaluate to true (None != stable_revision), causing the release job to run with an empty --revision value and fail later. Validate the candidate revision is present and fail fast with a clear error.
    .github/workflows/promote_candidate_to_stable.yaml:120
  • The code indexes candidate["channel"]["released-at"] directly. If Charmhub returns an entry missing channel or released-at (or changes the schema), this will crash with a KeyError and produce a traceback instead of the intended ::error:: messaging. Use .get() and fail with a clear message when the timestamp is missing.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Fail fast with a clear ::error:: if a candidate entry is missing its
revision number or release timestamp, instead of letting a None
revision slip through eligibility (leading to charmcraft release with
an empty --revision) or a raw KeyError traceback.

Copilot AI 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.

🔵 Needs a closer look

The eligibility check can mis-handle a malformed Charmhub “stable” entry (missing revision) and should fail fast to avoid unintended promotions.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/promote_candidate_to_stable.yaml:113

  • If Charmhub returns a stable entry without a revision number, stable_revision becomes None and the eligibility check can treat the candidate as different-from-stable and promote it anyway (and the summary table will show None). It’s safer to fail fast in this case, similar to how missing candidate revisions are handled.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Mirrors the existing candidate-side guard: a None stable_revision would
otherwise compare unequal to any candidate revision and let an
ineligible promotion through, plus show None in the summary table.

Copilot AI 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.

🔵 Needs a closer look

The workflow has a couple of concrete robustness/operability gaps (notably clearer auth-secret expectations and controlled handling of malformed Charmhub responses) that should be addressed to avoid confusing failures or misconfiguration.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

.github/workflows/promote_candidate_to_stable.yaml:68

  • json.load(response) can raise json.JSONDecodeError (e.g., if Charmhub returns a non-JSON body with a 200), which will currently surface as an unhandled Python traceback instead of a controlled ::error:: message. Catch this and fail via fail() for a clearer, more actionable failure mode.
    .github/workflows/promote_candidate_to_stable.yaml:214
  • The failure message for a missing CHARMHUB_STABLE_TOKEN doesn’t indicate that charmcraft expects this secret to contain the exported credentials format (same as the repo’s existing CHARMHUB_TOKEN usage in canonical/charm-ci, i.e. charmcraft login --export). Clarifying this in the ::error:: reduces the chance of someone providing a raw token and getting a confusing auth failure later.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Catch json.JSONDecodeError so a malformed 200 response fails with a
clear ::error:: instead of a bare traceback, and note in the missing-
CHARMHUB_STABLE_TOKEN error that it must hold exported charmcraft
credentials, not a raw API token.

Copilot AI 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.

🔵 Needs a closer look

It introduces automation that can publish releases to the stable channel, which is operationally high-impact and warrants final human approval despite the built-in environment gate.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants