Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion .github/workflows/_osdc-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ jobs:
if: ${{ inputs.run_smoke }}
runs-on: ubuntu-latest
environment: ${{ inputs.environment }}
timeout-minutes: 30
timeout-minutes: 60
defaults:
run:
working-directory: osdc
Expand All @@ -129,7 +129,17 @@ jobs:
aws-region: us-west-1
role-duration-seconds: 7200

# Smoke failures are usually transient cluster churn (the cluster needs
# time to converge after deploy), so retry once after a settle wait.
- name: Run smoke tests
id: smoke

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.

Why not use https://github.com/nick-fields/retry#retry_wait_seconds here? It looks nicer with that GHA

@jeanschmidt jeanschmidt Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Believe or not, this was the first implementation agents came up with. Then the full set of security review flagged that it uses internally a js script that is not pinned. So even if you pin the action to a specific hash, the code it actually runs is downloaded from a tag and is not safe from supply-chain attacks.

This leads me to drop the idea of using it for retry on context/workflows with critical infra tokens. I asked the agent to comment that, but it limited itself to mention in the previous PR "preferred over third party retry mechanisms that are unpinneable" - fail to mention the nick-fields and why it should not be used.

Do you believe I should document this?

continue-on-error: true
run: just smoke "${{ inputs.cluster }}"
- name: Wait for cluster to settle before smoke retry
if: ${{ steps.smoke.outcome == 'failure' }}
run: sleep 180
- name: Run smoke tests (retry)
if: ${{ steps.smoke.outcome == 'failure' }}
run: just smoke "${{ inputs.cluster }}"

integration:
Expand Down
2 changes: 1 addition & 1 deletion osdc/.claude/skills/osdc-deployment/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -356,4 +356,4 @@ Key workflows (all callable as reusable workflows via `workflow_call` from the o

When adding a new `just` recipe that CI should run, plumb it through the corresponding reusable workflow.

The OSDC-rooted CI jobs — the `deploy`/`smoke`/`integration` jobs in `_osdc-deploy.yml`, plus `osdc-drain.yml` and `osdc-undrain.yml` — set up their toolchain via the local composite action `./.github/actions/osdc-setup` (installs just + mise + `uv sync`). Its setup steps are retried on transient failures by **retry-by-duplication** (`continue-on-error` on the first attempt plus a copy gated on `steps.<id>.outcome == 'failure'`), chosen over a retry action deliberately: GitHub Actions has no native step retry, and a retry action would run unpinnable third-party code on the deploy / merge-queue path. `actions/checkout` and `aws-actions/configure-aws-credentials` are intentionally NOT wrapped because they already retry internally.
The OSDC-rooted CI jobs — the `deploy`/`smoke`/`integration` jobs in `_osdc-deploy.yml`, plus `osdc-drain.yml` and `osdc-undrain.yml` — set up their toolchain via the local composite action `./.github/actions/osdc-setup` (installs just + mise + `uv sync`). Its setup steps are retried on transient failures by **retry-by-duplication** (`continue-on-error` on the first attempt plus a copy gated on `steps.<id>.outcome == 'failure'`), chosen over a retry action deliberately: GitHub Actions has no native step retry, and a retry action would run unpinnable third-party code on the deploy / merge-queue path. `actions/checkout` and `aws-actions/configure-aws-credentials` are intentionally NOT wrapped because they already retry internally. On the deploy path, the `smoke` step is also retried once by duplication, with a 180s settle wait before the retry, because smoke failures are usually transient post-deploy cluster churn.
Loading