-
Notifications
You must be signed in to change notification settings - Fork 486
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
Support changing io.podman label domain namespace #1015
base: main
Are you sure you want to change the base?
Support changing io.podman label domain namespace #1015
Conversation
bbcc79a
to
fb873f2
Compare
8b4ac31
to
9d7a7c9
Compare
I think this feature makes sense in principle, but instead of environment variable and command-line option I would add a customization point to the compose file. For example, we already have default_net_name_compat. Environment variable and command line option are error prone, because a single compose file will need to have the same values passed on every invocation of podman-compose, otherwise it will break horribly. |
My thinking on this matter:
The motivation here is to allow transparently using the same compose files 1) together with other implementations that may differ in spec-compliance and how these labels are treated semantically, 2) using just So doing it by env/cli would at least be preferred for us. |
9d7a7c9
to
1483106
Compare
A couple of nitpicks:
This is a conscious decision due to the fact that podman-compose will always be missing some features of docker-compose. Therefore it is impossible to claim full compatibility with docker-compose and thus podman-compose should not try to manage resources created by docker-compose.
I think it's not possible to set the labels with the prefix used by the compose itself. From the spec:
Going back to whole argument I think I understand what you're saying, we are arriving to different conclusions due to different points of view. I consider both command line arguments and the supported syntax of the compose file as the API provided by podman-compose. If we want to expose a feature to the users we should consider both command line arguments and the compose file as potential points where users could configure the feature. Flexibility, ease of use, whether API is error prone, maintainability and so on are arguments for and against a particular approach. In this particular case, resources created using different domain namespace will be invisible to both podman-compose and docker-compose. It is easy to miss a command line argument in some invocation of podman-compose and this will result in error that is not immediately visible but may result in confusing errors later. Even Putting the configuration of the feature into compose file as a vendor extension significantly reduces the chances of errors because all invocations of podman-compose will be guaranteed to use the same label domain namespace. Therefore this looks like a preferable way to expose the feature. I am personally weakly against exposing this feature in general on the grounds of error-proneness. However I don't want to be a maintainer that limits a project to the limits of my personal imagination. Therefore I will accept this feature with some modifications to reduce impact that I don't like but preserving ability to do what you want. I think this should be a reasonable compromise. Are there limitations in the approach that I proposed that would prevent your use case? Do you just want to configure |
Thank you for the thoughtful follow-up!
This makes perfect sense! Pointed it out for context, not problematic I think.
I think this can be left to the user? Of course it would be up to the user to ensure they're not using unsupported features. Just like if they would be using two different instances/version of I'm mostly considering Compose spec. Which is separate from implementation parity with Very much agree that the totality of files (with schemas and semantics), cli + env vars, are all part of the
Yes, so, this PR does not touch these labels, as it is it does not allow changing/removing/overriding these in any new way. What it concerns itself with is the
Actually, the main motivation was/is to workaround what I assume is some bug in compose where filtering of resources only look for But I can also imagine it being a useful feature on its own to namespace resources and use custom domains (which again, would not touch the implied Does any of this change the picture a bit? :) |
1483106
to
6ee405d
Compare
This is where our point of view differs. The fact is that the default io.podman domain namespace is already used. The reason why a value different from com.docker has been selected was most likely to not break docker-compose. Regardless of reasons, that's the current situation. Whether default io.podman domain namespace is compatible with compose spec or not does not matter because we are not going to change it. Firstly, a lot of deployments rely on default io.podman domain namespace not being changed. Additionally, it is not good idea to change it right now because podman-compose is still lacking features compared to docker-compose. For the time being these are strong reasons. You present a good argument why user may want to change the default domain namespace anyway. I agree with it. The way how this is going to be exposed is completely up to podman-compose. Whether this is exposed as vendor setting in compose file or via command line option does not affect compatibility to compose spec in practice because out of the box podman-compose is incompatible anyway. I agree that in theory it is a bit nicer to be able to specify a custom command line option to podman-compose and make it use the same domain namespace as docker-compose. However given practical considerations I think better idea is to have a setting in compose file instead. Finally, the reason why compose exists at all was that it was inconvenient to setup a bunch of arguments to docker command when creating containers. Specifying them declaratively turned out way more convenient and less error prone. I think adding new setting keys to compose file (especially important ones such as domain namespace) is in the spirit of compose ecosystem as a whole.
Here I was responding to "If users prefer editing the Compose file, this can mostly already be achieved today by configuring labels on resources explicitly". I interpreted that you were suggesting that users may add
Unfortunately no, sorry. Having said that, it was a very useful discussion. I do wonder though, does my proposal not work in your use case for some reason? If it works then perhaps the best path forward is just to implement that solution, get it merged and move on. Compatibility of default domain namespace will become more important once podman-compose has relative feature parity with docker-compose. |
Hm, just to clarify on this, from
I would hope that this still reflects current intentions: for podman-compose to be an implementation of Compose Spec, and that any violations/deviations of the spec are to be treated as bugs? (Which, AIUI, is not at conflict with the additional existing use of custom |
This is true with a temporary exception of default domain namespace. That bug will fixed once podman-compose is relatively compatible with docker-compose. |
By the way, one incompatibility between podman-compose and docker-compose has already been fixed by introducing a setting into the compose file. See https://github.com/containers/podman-compose/blob/main/docs/Extensions.md#compatibility-of-default-network-names-between-docker-compose-and-podman-compose Compatibility between podman-compose and docker-compose in terms of default label domain namespace could be handled in a similar way. I'm sure we will find more similar incompatibilities in the future. |
6ee405d
to
88f73bb
Compare
Signed-off-by: legobt <[email protected]>
88f73bb
to
757d14e
Compare
Add global cli flag
--label-domain
and env varCOMPOSE_LABEL_DOMAIN
(defaultio.podman
).If set, it changes the out-of-spec root domain set and filtered for alongside the spec-mandated
com.docker.compose
.Related:
io.podman
label domain namespace #1013Compose spec