Volume options - allow nocreate option

[ygalblum]

No description provided.

[ygalblum]

This change is required as a first step for containers/podman#27867

[podmanbot]

✅ A new PR has been created in buildah to vendor these changes: containers/buildah#6628

[ygalblum]

@Luap99 @mheon PTAL

[mheon]

LGTM

[mtrmac]

I know ~nothing about this code, but at a first glance I share the concern in https://github.com/containers/podman/pull/27867/changes#r2676413266 : ValidateVolumeOpts is also called from Buildah. Isn’t it, after this PR, going to start passing the nocreate string to code which is not ready to consume it?

What do we do?

• One option might be to implement the option in Buildah as well. (Using some shared code??)
• Or do we add a parameter (supportsNoCreate?) to ValidateVolumeOpts (breaking API means a Buildah PR would need to be included, or adding a new function here).

[ygalblum]

@mtrmac I agree with the issue you raised. This is why I initially implemented the handling of this flag directly in Podman. But, then following @Luap99's comment: containers/podman#27867 (comment) I moved it here.

As for how to continue from here.

1. I'm not familiar enough with Buildah. But, does it create volumes? I couldn't find something in a quick glance of the code. If it does, then I guess it can be supported. If not, then I don't see what this option will do for it.
2. The additional API will solve the issue. But, what will we do the next time? Add another parameter? Maybe we can have a slice parameter adding supported options per caller.
3. We can alway go back to the initial implementation. Keep in mind that this change does not remove the need to parse the option and filter it out later in Podman, it only affects where and when it happens.

[mtrmac]

@Luap99 , @containers/podman-maintainers ?

[mheon]

Buildah doesn't make named volumes, but all these options also apply to bind mounts as well. IMO, appropriate solution is to allow the option here, but have Buildah error if the option is actually passed (which, honestly, will probably happen anyways...)

[ygalblum]

If that's the case, I think the simplest solution will be to add a new api: ValidateVolumeOptsWithAdditionals.

• This method will get another parameter that is a list of allowed values.
• ValidateVolumeOpts will call it with no additional values, keeping the current API not requiring any changes anywhere else.
• The change in Podman will call ValidateVolumeOptsWithAdditionals with []string{"nocreate"}
  WDYT?

[nalind]

buildah already has to have some validation logic of its own to reject flags it doesn't recognize for build-time cache and bind mounts, and as long as this function accepts a superset of what it does, it won't have an effect. So I don't think the current PR will affect it.