fix: stop passing --kernel-build-id when the image derives it

[rugggger]

Images with the AutoBuildIds capability resolve the kernel build id
themselves, so supplying --kernel-build-id ubuntu24.04 is redundant --
and since the build id is hashed into the kernel signature, having both
the operator and the CLI decide it invites the build and load sides to
disagree. Add the flag as bit 15 and skip the ubuntu-24 argument on both
the pack and the download/install paths when it is set.

Verified on Ubuntu 24.04 and RHEL 9.7: on Ubuntu the derived and the
explicitly-passed build id produce the same signature, and on RHEL --
where neither branch ever supplied one -- the image derives "rhel9" and
the loader agrees.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[rugggger]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix: stop passing --kernel-build-id when the image derives it #2791 👈 (View in Graphite)
• fix: run the staged weka CLI in the drivers-builder container #2790
• fix: stage the cluster image weka CLI for drivers-builder #2789
• fix: match full RHEL and Rocky product names in OS normalization #2788
• main

---

How to use the Graphite Merge Queue

Add the label main-merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Claude finished @rugggger's task in 3m 53s —— View job

---

Review: stop passing --kernel-build-id when the image derives it

• Gather context (diff, parent PRs fix: stage the cluster image weka CLI for drivers-builder #2789/fix: run the staged weka CLI in the drivers-builder container #2790)
• Analyze FeaturesFlags / get_feature_flags / get_release_spec
• Check both call sites (load_drivers, main drivers-builder)
• Post inline findings

The intent is right: with AutoBuildIds the build id feeds the kernel signature, so having exactly one side decide it is the correct invariant. The gap is how the two sides learn the flag — they don't read it from the same place, and one of the two reads can fail outright.

Findings

1. get_feature_flags() in load_drivers() can raise FileNotFoundError — and is the one call that must not. (weka_runtime.py:1615)

get_release_spec() hardcodes /opt/weka/dist/release and os.listdirs it unguarded, while get_weka_version() two functions above searches ["/opt/weka/dist/release", "/shared-weka-version/opt-weka/dist/release"] — a fallback added in dbdbfbf specifically because the first path is missing in these pods. So on the staged path (TARGET_IMAGE_NAME != IMAGE_NAME, i.e. the scenario the parent PRs in this stack address) the version lookup succeeds via the shared dir and the next line throws. In drivers-loader that becomes a 120 s retry loop ending in drivers_loaded=False. It also fires on the DRIVERS_BUILD_ID != 'auto' and COS branches, which never look at the flag.

2. Even when it succeeds, the loader may read the wrong version's flags. Same call site. In the staged case the loader pod's image can differ from the target image (GetLoaderImageForNode, internal/drivers/drivers.go:68), and any /opt/weka/dist/release present there belongs to the loader image. The builder reads the target spec (its get_feature_flags() runs after the --from file://shared-weka-version/opt-weka version get). Two images, two answers for bit 15 — the build/load divergence this PR exists to close. Fix for both: extend get_release_spec() with the same dir list, preferring the shared dir when TARGET_IMAGE_NAME differs, and resolve the flags lazily inside the branch. Fix this →

3. The builder has no error path if the spec isn't there. (weka_runtime.py:4378) Placement is correct, but it assumes --driver-only writes the release spec. If it doesn't on some image, the raise lands before write_results(...), so the operator sees a crash-looping pod instead of a reported build failure. A try/except falling back to FeaturesFlags(None) keeps the previous behaviour. Lower confidence — the description says Ubuntu 24.04 and RHEL 9.7 were verified.

4. Minor — domain.FeatureFlags (internal/pkg/domain/feature_flags.go) not updated. Harmless for unmarshalling, but the two bitmaps are kept in lockstep, and since the operator already resolves per-image flags through services.GetFeatureFlags, having the field there opens the option of the operator deciding this and passing it down, which removes the loader's dependency on finding a release spec entirely.

5. Nits. feature_flags vs build_feature_flags for the same thing in the two call sites; the identical two-line comment is duplicated in both places and would read fine as a single note on the flag definition at line 311.

One question on intent

The DRIVERS_BUILD_ID != 'auto' override still passes an explicit id on AutoBuildIds images, while the builder path (line 4381) ignores DRIVERS_BUILD_ID altogether. So a user-set override on ubuntu24 has the builder deriving one id and the loader passing another — the asymmetry is pre-existing, but this PR's rationale ("both sides deciding invites disagreement") now applies to it too. Worth deciding explicitly whether the override should also yield to the image.

No blocking objection to the approach; finding 1 is the one I'd want resolved before merge, since it fails closed on exactly the staged-CLI path the rest of this stack builds.
· branch

[claude]

This can throw FileNotFoundError in the loader, before any driver work happens.

get_feature_flags() → get_release_spec() hardcodes release_dir = "/opt/weka/dist/release" and calls os.listdir on it unguarded (line 1519). get_weka_version() just above deliberately searches ["/opt/weka/dist/release", "/shared-weka-version/opt-weka/dist/release"] — that fallback was added in dbdbfbf (fix: extend get_version() to look for release in /shared-weka folder) precisely because the first dir is absent in these pods. So in the TARGET_IMAGE_NAME != IMAGE_NAME path (the case the parent PRs in this stack are about) get_weka_version() succeeds via the shared dir and the very next line blows up. In drivers-loader that surfaces as a 120 s retry loop followed by drivers_loaded=False, with a FileNotFoundError that has nothing to do with drivers.

Two additional problems with this placement:

1. It runs unconditionally, including on the DRIVERS_BUILD_ID != 'auto' and is_google_cos() branches where the flag is never consulted — a new failure mode for paths that don't need it.
2. Even when /opt/weka/dist/release does exist, in the staged case it describes the loader image's version, not the target version being installed (the loader image can differ — see GetLoaderImageForNode in internal/drivers/drivers.go:68). The builder side reads the staged target spec (after weka version get --from file://shared-weka-version/opt-weka), so the two sides can read different values for bit 15 — reintroducing exactly the build/load signature disagreement this PR is trying to eliminate.

Suggested shape: give get_release_spec() the same directory list as get_weka_version() (preferring the shared dir when TARGET_IMAGE_NAME differs, so the flags describe the version actually being installed), and only resolve the flags inside the branch that needs them:

Suggested change

	feature_flags = await get_feature_flags()
	kernelBuildIdArg = ""
	if DRIVERS_BUILD_ID != 'auto':
	kernelBuildIdArg = f"--kernel-build-id {DRIVERS_BUILD_ID}"
	elif is_google_cos():
	kernelBuildIdArg = f"--kernel-build-id {OS_BUILD_ID}"
	elif is_ubuntu_24() and weka_dist_service():
	# weka derives the build id itself when AutoBuildIds is set
	feature_flags = await get_feature_flags()
	if not feature_flags.auto_build_ids:
	kernelBuildIdArg = f"--kernel-build-id {UBUNTU24_BUILD_ID}"

(the get_release_spec() dir-list change still needs to be made separately)

[claude]

Placement here is right — it's after the weka version get ... --from file://shared-weka-version/opt-weka on line 4372, so /opt/weka/dist/release should be populated with the target version's spec by the time get_release_spec() reads it (unlike the loader call site, see the other comment).

One residual risk: this assumes --driver-only lays down the release spec. If it doesn't on some image, get_release_spec() raises before write_results(...) ever runs, so the operator sees a crash-looping builder pod rather than a build error it can report. Given drivers-builder has no retry/report wrapper like drivers-loader does, it's worth being defensive:

try:
    build_feature_flags = await get_feature_flags()
except Exception as e:
    logging.warning(f"Could not read feature flags, assuming no auto build ids: {e}")
    build_feature_flags = FeaturesFlags(None)

The description says this was verified on Ubuntu 24.04 and RHEL 9.7, so this is about the untested-image case, not the happy path.

[claude]

The operator-side mirror of this bitmap, domain.FeatureFlags in internal/pkg/domain/feature_flags.go, doesn't get the new flag. Not a bug — json.Unmarshal ignores the extra auto_build_ids key that write_feature_flags_json() now emits — but the two lists are maintained in lockstep today (they even carry the same "flags N are not used by the operator" comments), and the operator already resolves flags per-image via services.GetFeatureFlags. Adding the field keeps parity and leaves the door open for the operator to make this decision itself, which would side-step the loader having to find a release spec at all.

[claude]

Claude finished @rugggger's task in 58s —— View job

---

Review: stop passing --kernel-build-id when the image derives it

• Check whether anything changed since the last review
• Re-verify the findings still apply

No changes since my last review. This synchronize was a rebase of the stack — every commit timestamp moved to 14:24:44, but charts/weka-operator/resources/weka_runtime.py is still blob 5006f5ce and the diff against 09-02-fix_run_the_staged_weka_cli_in_the_drivers-builder_container is byte-identical. get_release_spec() is also unchanged in the rebased base, so the findings below stand as written. Inline comments from the previous pass are still on the PR.

Previous review (unchanged — click to expand)

The intent is right: with AutoBuildIds the build id feeds the kernel signature, so having exactly one side decide it is the correct invariant. The gap is how the two sides learn the flag — they don't read it from the same place, and one of the two reads can fail outright.

1. get_feature_flags() in load_drivers() can raise FileNotFoundError — and is the one call that must not. (weka_runtime.py:1615)

get_release_spec() hardcodes /opt/weka/dist/release and os.listdirs it unguarded, while get_weka_version() two functions above searches ["/opt/weka/dist/release", "/shared-weka-version/opt-weka/dist/release"] — a fallback added in dbdbfbf specifically because the first path is missing in these pods. So on the staged path (TARGET_IMAGE_NAME != IMAGE_NAME, i.e. the scenario the parent PRs in this stack address) the version lookup succeeds via the shared dir and the next line throws. In drivers-loader that becomes a 120 s retry loop ending in drivers_loaded=False. It also fires on the DRIVERS_BUILD_ID != 'auto' and COS branches, which never look at the flag.

2. Even when it succeeds, the loader may read the wrong version's flags. Same call site. In the staged case the loader pod's image can differ from the target image (GetLoaderImageForNode, internal/drivers/drivers.go:68), and any /opt/weka/dist/release present there belongs to the loader image. The builder reads the target spec (its get_feature_flags() runs after the --from file://shared-weka-version/opt-weka version get). Two images, two answers for bit 15 — the build/load divergence this PR exists to close. Fix for both: extend get_release_spec() with the same dir list, preferring the shared dir when TARGET_IMAGE_NAME differs, and resolve the flags lazily inside the branch. Fix this →

3. The builder has no error path if the spec isn't there. (weka_runtime.py:4378) Placement is correct, but it assumes --driver-only writes the release spec. If it doesn't on some image, the raise lands before write_results(...), so the operator sees a crash-looping pod instead of a reported build failure. A try/except falling back to FeaturesFlags(None) keeps the previous behaviour. Lower confidence — the description says Ubuntu 24.04 and RHEL 9.7 were verified.

4. Minor — domain.FeatureFlags (internal/pkg/domain/feature_flags.go) not updated. Harmless for unmarshalling, but the two bitmaps are kept in lockstep, and since the operator already resolves per-image flags through services.GetFeatureFlags, having the field there opens the option of the operator deciding this and passing it down, which removes the loader's dependency on finding a release spec entirely.

5. Nits. feature_flags vs build_feature_flags for the same thing in the two call sites; the identical two-line comment is duplicated in both places and would read fine as a single note on the flag definition at line 311.

One question on intent. The DRIVERS_BUILD_ID != 'auto' override still passes an explicit id on AutoBuildIds images, while the builder path (line 4381) ignores DRIVERS_BUILD_ID altogether. So a user-set override on ubuntu24 has the builder deriving one id and the loader passing another — the asymmetry is pre-existing, but this PR's rationale ("both sides deciding invites disagreement") now applies to it too. Worth deciding explicitly whether the override should also yield to the image.

No blocking objection to the approach; finding 1 is the one I'd still want resolved before merge, since it fails closed on exactly the staged-CLI path the rest of this stack builds.
· branch

[claude]

Claude finished @rugggger's task in 33s —— View job

---

Review: stop passing --kernel-build-id when the image derives it

• Check whether anything changed since the last review
• Exit early — nothing to re-review

No changes since my previous review. Verified rather than assumed:

	last review	now
head blob weka_runtime.py	5006f5ce	5006f5ce
base blob (…drivers-builder_container)	58839a2f	58839a2f
get_release_spec() (line 1517)	still hardcodes /opt/weka/dist/release, unguarded os.listdir	unchanged

The diff against the base is byte-identical and the PR is still a single commit (cb936730). The base branch tip is still ded1b12a, so the parent PR didn't move either.

My earlier findings therefore stand exactly as posted — the inline comments from the first pass are still on the PR, and the summary is in the collapsed section of the previous comment. The one I'd still want resolved before merge is finding 1: get_feature_flags() at weka_runtime.py:1615 reads a release dir that get_weka_version() already has a documented fallback for, so it raises FileNotFoundError in the loader on the staged-CLI path this stack is built around — and it runs unconditionally, including on the DRIVERS_BUILD_ID override and google-COS branches that never consult the flag.

Fix findings 1 & 2 →
· branch

[claude]

Claude finished @rugggger's task in 3m 0s —— View job

---

Review: stop passing --kernel-build-id when the image derives it

• Detect changes since last review — head blob changed (5006f5ce → 889c7670)
• Re-read the updated diff
• Re-verify previous findings 1–4
• Check the new get_release_spec() fallback against all 8 other call sites
• Post updated review

This revision addresses everything I flagged. New commit 988b926b; the diff is now +27/-6 across weka_runtime.py and internal/pkg/domain/feature_flags.go.

Previous finding	Status
1 — get_feature_flags() raises FileNotFoundError in the loader	Resolved — moved inside the is_ubuntu_24() and weka_dist_service() branch, and get_release_spec() now searches both dirs
2 — loader may read the wrong version's flags	Resolved, and better than what I suggested (below)
3 — builder has no error path if the spec is missing	Moot — the failure is now unreachable (below)
4 — domain.FeatureFlags not updated	Resolved — AutoBuildIds bool \json:"auto_build_ids"`` // 15
5 — nits	feature_flags / build_feature_flags naming still differs; comment still duplicated. Not worth a round-trip

Why findings 2 and 3 are closed

The fix gives get_release_spec() the identical candidate list, order, and skip conditions as get_weka_version() (weka_runtime.py:1499 vs :1521). Two consequences, both stronger than the "prefer the shared dir when TARGET_IMAGE_NAME differs" shape I proposed:

• Version and flags cannot diverge. Whichever directory supplies the version supplies the flags, so the loader's bit-15 answer always describes the version it is installing. No TARGET_IMAGE_NAME branching needed, and it holds either side of version_get_cmd.
• Neither call site can hit the "no release spec" raise. In both load_drivers() and main() the preceding get_weka_version() has already succeeded over the same list, which proves a readable single-file dir exists. So the builder's missing try/except (finding 3) guards a path that can't be reached — the only residue would be a malformed .spec, which would already have broken write_feature_flags_json() elsewhere.

assert len(files) == 1 inside the loop means a two-file first candidate raises rather than falling through to the second — deliberate-looking and identical to get_weka_version(), so consistent either way.

Verification

• Bit 15 against a short bitmap: parse_feature_bitmap() returns set indices only, __init__ does bit in active, so an older image with a <2-byte bitmap yields auto_build_ids = False → the explicit id is still passed. Fails closed to the old behaviour.
• get_feature_map() iterates dir(self), so auto_build_ids is now emitted in the feature-flags-update result (:4555) and in feature_flags.json — which is exactly what the Go struct addition needs in order to parse.
• The builder writing kernel_build_id: "" when the flag is set is inert: BuiltDriversResult (internal/controllers/wekacontainer/funcs_drivers.go:298) has no such field, and UploadBuiltDrivers downloads by --kernel-signature, not by build id.
• The other 8 get_feature_flags() call sites all run in weka container pods where /opt/weka/dist/release exists, so the first candidate wins and their behaviour is byte-identical. The fallback only engages where the old code raised.
• Go: internal/pkg/domain/feature_flags.go is a two-field struct addition with matching json tag and bit comment; no positional parsing anywhere.

I could not run python3 -c 'ast.parse(...)' or go build — both were denied by the sandbox, so the syntax/compile claims above are from reading, not execution. The Python change is straightforward control flow and the Go change is a struct field, so the risk there is low, but flagging it rather than implying I ran them.

One item still open (not blocking)

The DRIVERS_BUILD_ID != 'auto' override still passes an explicit id on AutoBuildIds images while the builder (:4393) ignores DRIVERS_BUILD_ID entirely — so a user-set override on ubuntu24 has the two sides deciding differently, which is precisely the disagreement this PR eliminates elsewhere. Pre-existing and defensible if the override is meant to win over everything, but it's now a decision worth making explicitly rather than a side effect of where the flag check landed. Details in the inline comment.

Note that my inline comments from the first pass are now stale — they describe code that has since been rewritten. I can't resolve them myself; feel free to mark them resolved.

LGTM.
· branch

[claude]

The lazy placement resolves my earlier concern — get_feature_flags() no longer runs on the DRIVERS_BUILD_ID override or is_google_cos() branches, and it can no longer raise here: get_weka_version() on line 1625 already succeeded using the same candidate list with the same skip conditions, so get_release_spec() is guaranteed to find a spec, and to find it in the same directory the version came from. That's a stronger invariant than the "prefer shared when TARGET_IMAGE_NAME differs" shape I suggested — version and flags can't diverge by construction, and it holds regardless of whether this runs before or after version_get_cmd populates the local dir.

The one thing still open is the asymmetry with the override, which this PR's own rationale now reaches: on an AutoBuildIds ubuntu24 image with driversBuildId set, the builder (line 4393) ignores DRIVERS_BUILD_ID entirely and lets weka derive the id, while this branch passes the user's value to weka driver download/install — the two sides decide differently, which is the disagreement this change exists to prevent. Pre-existing, and arguably correct if the override is meant as an escape hatch that wins over everything, but worth deciding explicitly rather than leaving it as a side effect of where the flag check landed. If the override should keep winning, the builder is the side that needs to honour it.

[claude]

Claude finished @rugggger's task in 35s —— View job

---

Review: stop passing --kernel-build-id when the image derives it

• Check whether anything changed since the last review
• Exit early — nothing to re-review

No changes since my previous review. This trigger was the ready_for_review event, not a code push. Verified rather than assumed:

	last review (14:27)	now
head commit	988b926b	988b926b
charts/weka-operator/resources/weka_runtime.py	889c7670	889c7670
internal/pkg/domain/feature_flags.go	9899bf8e	9899bf8e
base branch tip (…drivers-builder_container)	ded1b12a	ded1b12a

Diff against the base is unchanged at +27/-6 across the same two files.

My previous review stands as posted: findings 1–4 from the first pass are all resolved by 988b926b, and the verdict was LGTM. The single non-blocking item still open is the asymmetry on the DRIVERS_BUILD_ID != 'auto' override — the loader passes the user's id while the builder (weka_runtime.py:4393) ignores the override and lets the image derive one, so on an AutoBuildIds ubuntu24 image with driversBuildId set the two sides can still disagree. Details in the inline comment.

Also still true from last time: the inline comments from my first pass describe code that has since been rewritten and are stale — I can't resolve them myself, feel free to mark them resolved.
· branch

[graphite-app]

Graphite Automations

"Add anton/matt/sergey/kristina as reviwers on operator PRs" took an action on this PR • (09/02/26)

3 reviewers were added to this PR based on Anton Bykov's automation.

[rugggger]

Closing: this changes the ubuntu-24 build id to the same value the image derives, so it is a no-op in practice.

Measured on kernel 7.0.0-1011-aws: 6.1.0.2109 (AutoBuildIds set, id derived) and 5.1.32.19 (flag clear, ubuntu24.04 passed explicitly) both produce kernel signature 51c370852a52b1d0. Build and load already agree either way, so there is no mismatch to fix.

The change also added a release-spec read to the loader, which crashed every staged loader with FileNotFoundError on images without the local-driver-copy capability. That was fixable, but it is cost without a demonstrated benefit.

Dropping the ubuntu24.04 constant is still reasonable hygiene once images own their build id -- worth revisiting as an ownership cleanup rather than a correctness fix.