Skip to content

job_hooks: add implicit constraint when using OCI containers #18162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

Elara6331
Copy link

This PR adds an implicit constraint for the OS and architecture to a task if it uses a driver that uses OCI container images, such as docker and podman.

Fixes #18082

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

Hi @Elara6331! I was doing some Friday afternoon mop-up and noticed we never reviewed this PR. Sorry about that.

Unfortunately I'm also fairly certain we can't accept this PR from a design standpoint. I've left a comment below getting into the details of why. I think we'd be open to other approaches that don't have those downsides though.

Comment on lines +303 to +304
} else if desc.MediaType.IsImage() {
img, err := remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain))
Copy link
Member

Choose a reason for hiding this comment

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

It looks to me like we're having the Nomad server authenticate to a remote registry during job submission here? Nomad servers are not typically going to be configured with registry credentials at all, because they don't run workloads themselves.

The approach we're taking here would make it mandatory that all users of Nomad who are using Docker/Podman drivers deploy credentials to their Nomad control plane before upgrading, and have a network topology that allows their control plane access to the registry (which isn't necessarily the case). And it would make all Nomad servers dependent on third-party registries at the time of job submission, which I don't think we can accept as a tradeoff.

@tgross
Copy link
Member

tgross commented Dec 4, 2023

For the reasons I've described above, I'm going to close this out. But again, if there's another approach that would work within the constraints we have, we'd certainly be open to discussing it (probably best to open an issue for that discussion).

@tgross tgross closed this Dec 4, 2023
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Detect supported architectures for docker containers
2 participants