Skip to content

Commit df81fe4

Browse files
committed
Address PR review feedback and dry-run findings
- Deduplicate Instructions.md (remove sections that duplicate release.agent.md) - Strengthen core rules in release.agent.md with NEVER/ALWAYS/STOP emphasis - Name release-all.sh as the central tool in agent file and plan intro - Add .github/CODEOWNERS with 4 reviewers - Read CODEOWNERS in setup-release-env.sh instead of hardcoded pool - Add make clean-bin loop before rm -rf in Step 0 - Add gh auth status pre-flight check (prereq 3) - Add retry guidance for nnf_deploy GitHub Release wait - Add empty RELEASE_REVIEWERS warning guard Signed-off-by: Anthony Floeder <anthony.floeder@hpe.com>
1 parent d98a807 commit df81fe4

5 files changed

Lines changed: 45 additions & 119 deletions

File tree

.github/CODEOWNERS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# NNF release reviewers — used by the release agent to assign PR reviewers.
2+
# All code owners are assigned as reviewers except the person creating the release.
3+
* @matthew-richerson @bdevcich @roehrich-hpe @ajfloeder

.github/agents/release.agent.md

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,29 @@ argument-hint: "Specify the release type (patch, minor, or major) — defaults t
66

77
You are the NNF Release Agent. Your job is to execute NNF software releases by following the documented release process precisely, step by step, with user approval at each gate.
88

9+
The entire release process is orchestrated by `release-all.sh` (located in `tools/release-all/`). It handles cloning, branching, PR creation, merging, and tagging for all NNF repos. The helper function `nnf_cmd` wraps `release-all.sh` with the correct flags.
10+
911
## Required Reading
1012

1113
Before starting any release work, read these files completely:
1214

1315
1. [Release Plan](../../tools/release-all/RELEASE-PLAN.md) — The authoritative, self-contained guide for releases (repo table, dependency chain, all phases)
14-
2. [Agent Instructions](../../tools/release-all/Instructions.md)Behavioral guidelines: execution model, approval gates, error handling, key gotchas
16+
2. [Agent Instructions](../../tools/release-all/Instructions.md)Error handling specifics, key gotchas, and guidance for updating the plan
1517

1618
## Core Rules
1719

18-
- **Wait for user approval** after every step before proceeding.
19-
- After each step, state: what you did, whether it succeeded, and the evidence.
20-
- Run all commands in a terminal so the user can see them.
21-
- Append `2>&1 | cat` to all `release-all.sh` and `gh` commands.
22-
- Execute one phase per repo at a time — never parallelize across repos.
23-
- If a command fails, analyze the error and propose a fix. Do not retry blindly.
24-
- Never use `--force`, `--no-verify`, or destructive operations without asking.
25-
- If you discover gaps or errors in the plan, update RELEASE-PLAN.md and note the change.
20+
- **ALWAYS wait for explicit user approval before proceeding to the next step.** After every step, state what you did, whether it succeeded, and the evidence. **WAIT for the user.**
21+
- **ALWAYS run commands in a terminal** so the user can see them.
22+
- **ALWAYS append `2>&1 | cat`** to every `release-all.sh` and `gh` command — no exceptions.
23+
- **ONE repo at a time.** Execute one phase per repo sequentially. **NEVER parallelize** across repos.
24+
- **STOP on failure.** If a command fails, analyze the error and propose a fix. **Do NOT retry blindly.**
25+
- **NEVER use `--force`, `--no-verify`, or destructive operations** without asking the user first.
26+
- If you discover gaps or errors in the plan, **update RELEASE-PLAN.md** and tell the user what you changed.
2627

2728
## Workflow
2829

2930
1. Ask the user: What is the release type? (`patch`, `minor`, or `major` — default `patch`)
3031
2. Read the Release Plan and Agent Instructions completely.
31-
3. Follow the plan step by step, using the todo tool to track progress. The plan starts with cloning into a fresh working directory, then sources `setup-release-env.sh -B <type>` to set up environment variables and helper functions. Present the computed values to the user for confirmation.
32+
3. Follow the plan step by step, using the todo tool to track progress.
3233
4. At each approval gate, present your evidence and wait.
3334
5. After all repos are released, run `final-release-notes.sh` and `compare-releases.sh`.

tools/release-all/Instructions.md

Lines changed: 2 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,11 @@
11
# Agent Instructions for NNF Software Release
22

3-
> **Agent entry point:** In VS Code, invoke the `@release` agent from the chat picker (defined in `.github/agents/release.agent.md`). The agent automatically loads this file and `RELEASE-PLAN.md` as context.
3+
> This file supplements `RELEASE-PLAN.md` (the authoritative reference) with agent-specific guidance. **All procedural steps and gotchas are in the plan — do not duplicate them here.**
44
5-
## Overview
6-
7-
Execute the release plan defined in `RELEASE-PLAN.md` (in this same directory). That document is the authoritative, self-contained reference for the release process. Read it completely before starting.
8-
9-
## Goal
10-
11-
Create a release of the NNF software while ensuring the release process documented in `RELEASE-PLAN.md` is correct and repeatable. If you discover gaps, errors, or missing details in the plan, update it.
12-
13-
## Agent Behavior
14-
15-
### Execution Model
16-
17-
- Run all commands in a terminal window so the user can see them.
18-
- Append `2>&1 | cat` to all `release-all.sh` and `gh` commands to avoid pager issues.
19-
- Execute one phase per repo at a time. Do not parallelize across repos.
20-
- Complete steps 4a–4d for each repo before moving to the next.
21-
22-
### Approval Gates
23-
24-
- **Wait for the user to approve** after each step before proceeding.
25-
- After each step, clearly state:
26-
1. What you did
27-
2. Whether you believe it succeeded
28-
3. The evidence (command output, exit codes, API responses)
29-
- The user will respond with "approved", "approved. continue", or will coach you if something is wrong.
30-
31-
### Error Handling
32-
33-
- If a command fails, do **not** retry blindly. Analyze the error, explain it, and propose a fix.
34-
- If `merge-pr` produces no output, verify via: `gh api repos/<owner>/<repo>/pulls/<pr_number> 2>&1 | grep -o '"merged":[^,]*'`
35-
- If `tag-release` shows "Bypassed rule violations for refs/tags/..." — this is expected (tag protection rulesets allow admin/maintain bypass).
36-
- If a vendoring check fails, investigate the submodule pointer or vendor directory before proceeding.
37-
- Never use `--force`, `--no-verify`, or destructive operations without asking the user first.
38-
39-
### Release Process
40-
41-
The release process is the same regardless of how many repos have changes. The `release` phase automatically detects which repos have new commits since the last release. Repos with no changes report "No new changes to release" and are skipped in subsequent phases. Simply run all phases on all repos — the script handles the rest.
42-
43-
### Key Gotchas (Learned from Experience)
44-
45-
- `nnf_sos` requires the `-M` flag on ALL phases (not just `master`).
46-
- `nnf_mfu` and `nnf_ec` often have no changes — "No new changes to release" is normal, skip them.
47-
- `nnf_doc` must wait until after `nnf_deploy`'s GitHub Release is published (~60s after tagging). Verify with: `gh release view $NNF_RELEASE -R NearNodeFlash/nnf-deploy`
48-
- `nnf_doc` uses the `main` branch (not `master`). Its repo is `NearNodeFlash/NearNodeFlash.github.io`.
49-
- When a dependency changes (e.g., `nnf-sos`), all downstream repos that vendor it (`nnf-dm`, `nnf-integration-test`) must be revendored and their PRs merged **before** starting the release. If the vendoring check (Step 2) fails with "Peer modules are behind", follow Step 2a in RELEASE-PLAN.md to fix it. The check output prints the exact `go get` commands needed.
50-
- Always run vendoring checks on ALL repos — this catches stale submodule pointers.
51-
- After completing all releases, finalize release notes with `final-release-notes.sh` and run `compare-releases.sh` for verification.
52-
53-
### Updating the Plan
5+
## Updating the Plan
546

557
If you encounter an issue not covered by RELEASE-PLAN.md, or if a step's success criteria are unclear:
568

579
1. Resolve the issue with the user's guidance.
5810
2. Update RELEASE-PLAN.md with what you learned.
5911
3. Note the update to the user.
60-
61-
## Quick Start
62-
63-
```sh
64-
# 1. Read the plan
65-
cat RELEASE-PLAN.md
66-
67-
# 2. Ask the user: release type? (patch/minor/major, default patch)
68-
69-
# 3. Source the helper: source ./setup-release-env.sh -B patch
70-
# Confirm the printed summary with the user
71-
72-
# 4. Follow RELEASE-PLAN.md step by step, waiting for approval at each gate.
73-
```

tools/release-all/RELEASE-PLAN.md

Lines changed: 18 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# NNF Software Release Plan
22

3-
This document is the authoritative, self-contained guide for executing an NNF software release. The release process is the same regardless of how many repos have changes — the `release` phase automatically detects which repos have new commits since the last release and skips those that don't. An agent or human following this document should be able to execute the entire release without external context.
3+
This document is the authoritative, self-contained guide for executing an NNF software release. All release phases are driven by `release-all.sh` in this directory, wrapped by the `nnf_cmd` helper function. The release process is the same regardless of how many repos have changes — the `release` phase automatically detects which repos have new commits since the last release and skips those that don't. An agent or human following this document should be able to execute the entire release without external context.
44

55
---
66

@@ -19,14 +19,7 @@ This document is the authoritative, self-contained guide for executing an NNF so
1919
| `nnf_deploy` | `NearNodeFlash/nnf-deploy` | NearNodeFlash | `master` | Has submodules for most repos |
2020
| `nnf_doc` | `NearNodeFlash/NearNodeFlash.github.io` | NearNodeFlash | **`main`** (not master) | Released last; tracks nnf-deploy version |
2121

22-
**PR Reviewers:** Assign all members of the reviewer pool **except** the person creating the release:
23-
24-
- `matthew-richerson`
25-
- `bdevcich`
26-
- `roehrich-hpe`
27-
- `ajfloeder`
28-
29-
For example, if `ajfloeder` is running the release, the reviewers are `matthew-richerson`, `bdevcich`, `roehrich-hpe`.
22+
**PR Reviewers:** Defined in `.github/CODEOWNERS`. Assign all code owners **except** the person creating the release. The `setup-release-env.sh` helper reads CODEOWNERS automatically and sets `$RELEASE_REVIEWERS`.
3023

3124
**Dependency chain (release order):**
3225

@@ -51,9 +44,10 @@ If an upstream repo changes, **all downstream repos that vendor it** may need re
5144

5245
1. Ensure these tools are installed: `gh`, `yq` (Go version v4.x), `jq`, `perl`, `git`, `make`, `tput`, `sed`
5346
2. Set `GH_TOKEN` env var with a GitHub **classic** token (not fine-grained) with `repo` scope. The token is 40 characters, starting with `ghp_`.
54-
3. Verify SSH access to all repo URLs (`ssh -T git@github.com`)
55-
4. Decide the release type (`-B major|minor|patch`). **Always pass `-B` explicitly** — do not rely on the script default, which could change. Most releases use `-B patch`.
56-
5. **Pre-release vendoring check:** If any upstream repo has had changes merged to master since the last release that affect downstream vendoring, ensure downstream repos have been revendored **before** starting the release. For example, if `nnf-sos` changed, then `nnf-dm` and `nnf-integration-test` (which vendor `nnf-sos`) must have their vendor directories updated via PRs merged to master. Phase 1's vendoring checks will catch any mismatches.
47+
3. Verify `gh` authentication: `gh auth status`. Confirm it shows the expected user and token scopes.
48+
4. Verify SSH access to all repo URLs (`ssh -T git@github.com`)
49+
5. Decide the release type (`-B major|minor|patch`). **Always pass `-B` explicitly** — do not rely on the script default, which could change. Most releases use `-B patch`.
50+
6. **Pre-release vendoring check:** If any upstream repo has had changes merged to master since the last release that affect downstream vendoring, ensure downstream repos have been revendored **before** starting the release. For example, if `nnf-sos` changed, then `nnf-dm` and `nnf-integration-test` (which vendor `nnf-sos`) must have their vendor directories updated via PRs merged to master. Phase 1's vendoring checks will catch any mismatches.
5751

5852
### Phase 0: Fresh Clone
5953

@@ -69,6 +63,14 @@ export RELEASE_WORKDIR=~/release-work # adjust as needed
6963

7064
# If $RELEASE_WORKDIR exists, ask the user before proceeding.
7165
# If they decline, stop here.
66+
67+
# Clean build artifacts before removing (rm -rf can fail on build outputs)
68+
if [ -d "$RELEASE_WORKDIR/nnf-deploy/workingspace" ]; then
69+
for repo in "$RELEASE_WORKDIR"/nnf-deploy/workingspace/*/; do
70+
( cd "$repo" && make clean-bin 2>/dev/null || true )
71+
done
72+
fi
73+
7274
rm -rf "$RELEASE_WORKDIR"
7375
mkdir -p "$RELEASE_WORKDIR"
7476
cd "$RELEASE_WORKDIR"
@@ -134,11 +136,11 @@ For each failing repo:
134136
6. **Push** the branch and **create a PR** titled `"Update vendor dependencies"` with body `"Pre-release vendoring update."`.
135137
7. **Assign reviewers** from `$RELEASE_REVIEWERS`.
136138
8. **Wait for CI** and reviewer approval, then **merge** the PR.
137-
9. **Verify the fix against origin:** Re-clone (repeat Step 0) so you're working from what's actually on GitHub, then re-run Step 2 starting from the repo you just fixed and continuing through the remaining downstream repos. The fixed repo must pass. If a downstream repo fails, repeat this procedure for it.
139+
9. **Verify the fix against origin:** Re-clone (repeat Step 0) so you're working from what's actually on GitHub, then re-run the `nnf_cmd master <repo>` check starting from the repo you just fixed and continuing through all remaining downstream repos in order. The fixed repo must now pass. If a downstream repo fails, repeat this procedure (steps 1–9) for it.
138140
10. Return to `$RELEASE_WORKDIR/nnf-deploy/tools/release-all`.
139141

140142
> **Dependency order matters:** An upstream repo's vendoring PR must be merged to master before any downstream repo that vendors it can be fixed. For example, if both `nnf_sos` and `nnf_dm` fail, merge the `nnf_sos` fix first — `nnf_dm` vendors `nnf_sos` and needs the updated master.
141-
143+
>
142144
> **`nnf_deploy` submodule pointers:** Any submodule repo that has had commits merged to master since the last submodule pointer update — whether from development work or vendoring fixes — will cause `nnf_deploy`'s Step 2 check to fail with "Submodules are not up to date." The script catches this automatically. Fix it the same way: clone `nnf_deploy`, create a branch, run `git submodule update --remote` for the affected submodules, commit, PR, merge. Then re-clone and verify.
143145
144146
After fixing and verifying the last failing repo, **re-run Step 2 on all repos** as a final safety net to confirm everything passes end to end.
@@ -157,15 +159,15 @@ done
157159

158160
Review output for merge conflicts before continuing.
159161

160-
> **Note:** `nnf_mfu` (and sometimes `nnf_ec`) may report "No new changes to release" if master has no commits since the last release. This is normal — skip the repo in subsequent phases.
162+
> **Note:** `nnf_mfu` and `nnf_ec` are standalone repos that often have no new commits between releases. If either reports "No new changes to release", this is normal — **skip that repo in all subsequent phases** (Steps 4a–4d and the `nnf_doc` sequence). Do not attempt to push, PR, merge, or tag a repo that had no changes.
161163
162164
#### `nnf_doc` deferral
163165

164166
`nnf_doc` must be deferred until after `nnf_deploy` is tagged and its GitHub Release is published. The `nnf_doc` script updates `mkdocs.yml` with the latest `nnf-deploy` release version by querying GitHub Releases (not just tags). Follow this sequence:
165167

166168
1. Complete Steps 3–4d for all repos through `nnf_deploy`
167169
2. **Wait ~60 seconds** for `nnf_deploy`'s "Handle Release Tag" GitHub Actions workflow to publish the GitHub Release
168-
3. Verify: `gh release view $NNF_RELEASE -R NearNodeFlash/nnf-deploy 2>&1 | cat` should succeed
170+
3. Verify: `gh release view $NNF_RELEASE -R NearNodeFlash/nnf-deploy 2>&1 | cat` should succeed. If it fails with "release not found", wait another 30 seconds and retry. The workflow typically completes within 2 minutes; if it hasn't after 5 minutes, check the Actions tab on the `nnf-deploy` repo for errors.
169171
4. Then run `nnf_doc` through Steps 3–4d:
170172

171173
```sh
@@ -266,35 +268,7 @@ The `-i` flag displays image version changes inline. Use `-d` to display the ful
266268

267269
Check each repo for correct tag, release notes, and artifacts (`manifests.tar` + `manifests-kind.tar` on `nnf-deploy`).
268270

269-
---
270271

271-
## Part B: Documentation Improvements & Corrections
272-
273-
### Corrections (issues found by comparing docs to actual scripts)
274-
275-
> Items marked ✅ have been applied to `release-all.md` on branch `improve-release-docs`.
276-
> Items marked 🔲 remain open.
277-
278-
1.**Missing prerequisites** — Added full Prerequisites section with tool table, `GH_TOKEN` env var, and SSH access check.
279-
2.**Missing `-B` flag** — Documented `-B major|minor|patch` (default: `patch`) in the Steps section.
280-
3. 🔲 **Missing `-w`, `-M` flags**`-x force-tag` is now documented for error recovery, but `-w` (workspace dir override) and `-M` (multi-API-version bypass) are not yet documented. These are advanced/optional and could be an enhancement.
281-
4.**Step structure was confusing** — Restructured as steps 3a–3d with bold headers; 3a branching path made explicit.
282-
5.**Annotated tag requirement not called out** — Added Important note at step 3d explaining annotated tag requirement and CI/CD rejection of lightweight tags.
283-
6.**`repo-list` output never shown** — Added the full 10-repo dependency-ordered list under step 0.
284-
7.**nnf-mfu and nnf-ec distinction unclear** — Added *(standalone repo, not a submodule)* annotations and dependency-order list with rationale.
285-
8.**No error recovery guidance** — Added `-x force-tag=vX.Y.Z` recovery, manual merge warning on step 3c.
286-
9.**`compare-releases.sh` not framed as verification** — Renamed section to "Verify the release" and framed as a recommended verification step.
287-
10.**`final-release-notes.sh` prereqs not listed** — Added note listing `yq`, `gh`, `jq`, `perl`, and `GH_TOKEN` requirement.
288-
11.**No mention of CI/CD workflow** — Added explanation of `handle_release_tag.yaml` at step 3d (creates GitHub release, attaches artifacts).
289-
12.**Step numbering was wrong** — Renumbered as 3a–3d (was 1–5 sub-list); removed phantom step "e".
290-
291-
### Enhancements
292-
293-
1. **Add a pre-flight checklist section** before Step 1
294-
2. **Add a troubleshooting section** — tag rejected, yq version mismatch, GH_TOKEN expired, merge conflicts
295-
3. **Add "What happens under the hood"** — Brief description of what each phase does internally (updates kustomization images, Helm versions, Dockerfile refs, repositories.yaml)
296-
4. **Document full repo ordering with rationale** — Explain the dependency chain (dws → lustre → nnf-mfu/ec → nnf-sos → nnf-dm → integration-test → deploy → docs)
297-
5. **Expand `tools/release-all/README.md`** — Add prerequisites and quick-start
298272

299273
---
300274

tools/release-all/setup-release-env.sh

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,21 @@ NNF_RELEASE=$(echo "$PREVIOUS_RELEASE" | perl -F'/\./' -ane "$PERL_CMD")
5959

6060
# --- Detect operator & reviewers --------------------------------------------
6161
RELEASE_USER=$(gh api user --jq '.login' 2>&1)
62-
REVIEWER_POOL="matthew-richerson,bdevcich,roehrich-hpe,ajfloeder"
62+
CODEOWNERS_FILE="../../.github/CODEOWNERS"
63+
if [[ ! -f "$CODEOWNERS_FILE" ]]; then
64+
echo "ERROR: $CODEOWNERS_FILE not found. Run from tools/release-all/ inside an nnf-deploy clone." >&2
65+
return 1
66+
fi
67+
REVIEWER_POOL=$(grep '^\*' "$CODEOWNERS_FILE" | tr -s ' ' | cut -d' ' -f2- | tr ' ' '\n' | sed 's/@//' | paste -sd,)
6368
export RELEASE_REVIEWERS
6469
RELEASE_REVIEWERS=$(echo "$REVIEWER_POOL" | tr ',' '\n' \
6570
| grep -v "$RELEASE_USER" | paste -sd,)
6671

72+
if [[ -z "$RELEASE_REVIEWERS" ]]; then
73+
echo "WARNING: RELEASE_REVIEWERS is empty — you are the only code owner in CODEOWNERS." >&2
74+
echo " You will need to assign reviewers manually for each PR." >&2
75+
fi
76+
6777
# --- Helper: run release-all.sh with auto -M and -B -------------------------
6878
# Usage: nnf_cmd <phase> <repo>
6979
nnf_cmd() {

0 commit comments

Comments
 (0)