Skip to content

fix(core): pin supervisor image tag to gateway version for all drivers - #2070

Merged
TaylorMutch merged 2 commits into
NVIDIA:mainfrom
benoitf:2068-pin-supervisor-image/fb
Jul 10, 2026
Merged

fix(core): pin supervisor image tag to gateway version for all drivers#2070
TaylorMutch merged 2 commits into
NVIDIA:mainfrom
benoitf:2068-pin-supervisor-image/fb

Conversation

@benoitf

@benoitfbenoitf commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Extract the Docker driver's compile-time supervisor image tag resolution into openshell-core so all three compute drivers (Docker, Podman, Kubernetes) pin the supervisor image to the gateway version instead of defaulting to :latest.

Related Issue

Closes#2068

Changes

  • Add resolve_supervisor_image_tag() and default_supervisor_image() to openshell-core::config with the same priority chain the Docker driver already used: OPENSHELL_IMAGE_TAG > IMAGE_TAG > CARGO_PKG_VERSION (falls back to dev for 0.0.0)
  • Add DEFAULT_SUPERVISOR_IMAGE_REPO constant (repo without tag) to openshell-core
  • Deprecate DEFAULT_SUPERVISOR_IMAGE (the :latest constant)
  • Update Podman driver to use default_supervisor_image() in config default and make the CLI --supervisor-image arg optional with version-pinned fallback
  • Update Kubernetes driver to use default_supervisor_image() in config default and CLI fallback
  • Refactor Docker driver to delegate to the shared resolver, removing its local copy
  • Update Podman container tests that asserted :latest to use the dynamic default

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

copy-pr-botBot commented Jun 30, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@TaylorMutchTaylorMutch added the test:e2e Requires end-to-end coverage label Jun 30, 2026
@TaylorMutch

TaylorMutch commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

/ok to test cb62ca9

@github-actions

github-actionsBot commented Jun 30, 2026

Copy link
Copy Markdown

Label test:e2e applied, but pull-request/2070 is at {"messa while the PR head is cb62ca9. A maintainer needs to comment /ok to test cb62ca9f1715aa52108571d81a17411a07765aeb to refresh the mirror. Once the mirror catches up, re-run Branch E2E Checks from the Actions tab.

TaylorMutch
TaylorMutch reviewed Jun 30, 2026
View reviewed changes
Comment threadcrates/openshell-core/src/config.rs Outdated
@benoitf
benoitfforce-pushed the 2068-pin-supervisor-image/fb branch 3 times, most recently from 0582804 to aa8af37CompareJuly 1, 2026 09:33
@TaylorMutch

TaylorMutch commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

/ok to test aa8af37

@TaylorMutch

TaylorMutch commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Minor test concern: latest is an acceptable explicit image tag, but default_supervisor_image_is_version_pinned asserts that default_supervisor_image() never ends with :latest. Because that function reads OPENSHELL_IMAGE_TAG/IMAGE_TAG at compile time, OPENSHELL_IMAGE_TAG=latest cargo test -p openshell-core will fail even though the resolver accepts that input.

Please update the test to verify the stable contract instead (the expected repository plus a non-empty tag), and consider adding an explicit resolver case documenting that latest is accepted:

assert_eq!(
resolve_supervisor_image_tag(Some("latest"),None,"1.2.3"),"latest",);

This is a non-blocking test robustness issue rather than a runtime defect.

@benoitf
benoitfforce-pushed the 2068-pin-supervisor-image/fb branch from aa8af37 to fd09c75CompareJuly 1, 2026 18:41
@benoitf

benoitf commented Jul 1, 2026

Copy link
Copy Markdown
ContributorAuthor

@TaylorMutch PR amended

@TaylorMutch

TaylorMutch commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

/ok to test fd09c75

@bmahabirbubmahabirbu mentioned this pull request Jul 3, 2026
Merged
elezar
elezar reviewed Jul 3, 2026
View reviewed changes
Comment threadcrates/openshell-driver-docker/src/lib.rs Outdated
elezar
elezar reviewed Jul 3, 2026
View reviewed changes
Comment threadcrates/openshell-core/src/config.rs Outdated
benoitf added 2 commits July 3, 2026 15:33
@benoitf
fix(core): pin supervisor image tag to gateway version for all drivers
bcbe7af
The Podman and Kubernetes drivers defaulted the supervisor image to
`:latest` via DEFAULT_SUPERVISOR_IMAGE, while the Docker driver already
resolved a version-pinned tag. Extract the tag resolution logic into
openshell-core so all three drivers use the same
OPENSHELL_IMAGE_TAG > IMAGE_TAG > CARGO_PKG_VERSION priority chain.
ClosesNVIDIA#2068
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@benoitf
refactor(core): simplify supervisor image tag resolver to slice-based…
ac66187
… API
Remove the Docker driver's wrapper functions and call
openshell_core::config::default_supervisor_image() directly.
Simplify resolve_supervisor_image_tag to accept &[&str] instead
of three separate parameters.
Signed-off-by: Florent Benoit <fbenoit@nvidia.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@benoitf
benoitfforce-pushed the 2068-pin-supervisor-image/fb branch from fd09c75 to ac66187CompareJuly 3, 2026 14:08
@benoitf

benoitf commented Jul 3, 2026

Copy link
Copy Markdown
ContributorAuthor

@elezar PR updated with your suggested changes

@TaylorMutch

TaylorMutch commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

/ok to test ac66187

@TaylorMutch

TaylorMutch commented Jul 10, 2026•
edited
Loading

Copy link
Copy Markdown
Collaborator

Changes look good to me. I will likely submit a follow up to address some Helm-specific pieces, instead of blocking this PR from landing.

Follow-up: #2216

@TaylorMutch
TaylorMutch merged commit 1070213 into NVIDIA:mainJul 10, 2026
47 checks passed
@TaylorMutchTaylorMutch mentioned this pull request Jul 10, 2026
Merged
6 tasks
@waynesun09waynesun09 mentioned this pull request Jul 11, 2026
Closed
7 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@elezarelezarelezar left review comments
@TaylorMutchTaylorMutchTaylorMutch approved these changes
@mrunalpmrunalpAwaiting requested review from mrunalpmrunalp is a code owner
@maxamillionmaxamillionAwaiting requested review from maxamillionmaxamillion is a code owner
@derekwaynecarrderekwaynecarrAwaiting requested review from derekwaynecarrderekwaynecarr is a code owner

Assignees

No one assigned

Labels

test:e2eRequires end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Podman and Kubernetes drivers default supervisor image to :latest instead of pinning to gateway version

3 participants

@benoitf@TaylorMutch@elezar