containers / storage

Container Storage Library
Apache License 2.0
556 stars 237 forks source link

AdditionalImageStores configuration #1306

Open meox opened 2 years ago

meox commented 2 years ago

In order to configure the AdditionalImageStores we have to change the config file. I'd like to have the possibility to add additional image stores just adding a file inside a folder like /etc/containsers/additionalstore.conf.d. In this way I don't have to modify the configuration file if I add a new image store layer.

The content of this folder should be smthg like:

where each file contains the real path of the store

vrothberg commented 2 years ago

Thanks for reaching out, @meox!

I sympathize with the proposal but think we could generalize it to a /etc/containers/storage.conf.d. This wouldn't limit it to the additional_imagestores option but allow for overriding/extending more options of storage.conf.

@giuseppe @nalind @rhatdan @flouthoc WDYT?

flouthoc commented 2 years ago

@meox @vrothberg Just confirming please correct me if i got this wrong, the request is for a different UX where instead of editing .config/containers/storage.conf it is being requested that we can add additionalstore by creating files.

I sympathize with the proposal but think we could generalize it to a /etc/containers/storage.conf.d. This wouldn't limit it to the additional_imagestores option but allow for overriding/extending more options of storage.conf.

@vrothberg for this do you think using files for controlling each knob of storage.conf.d could cause confusion for users who have both a storage.conf and some options getting overrides by files in /etc/containers/storage.conf.d.

vrothberg commented 2 years ago

@meox @vrothberg Just confirming please correct me if i got this wrong, the request is for a different UX where instead of editing .config/containers/storage.conf it is being requested that we can add additionalstore by creating files.

Yes, that's correct.

@vrothberg for this do you think using files for controlling each knob of storage.conf.d could cause confusion for users who have both a storage.conf and some options getting overrides by files in /etc/containers/storage.conf.d.

I do not expect confusion. We support .d drop-in files for containers.conf, registries.conf and others already. It's a quite common feature. The beauty is that users can tweak configurations without having to change the defaults from the distribution. Config-management tools such as Ansible can easily "rollback" by removing the files that were added; muuuch easier than trying to keep the original files around somewhere.

flouthoc commented 2 years ago

Ah i see, I get it now yes i think it could be really useful for orchestration/automation tools like ansible and automation scripts where different parameter could be controlled by creating/removing files much better than editing the central config file.

meox commented 2 years ago

@flouthoc exactly is what I need because we have a similar use case with my build systems: different app with separated storage. But what I want is to have a "global" view of them without touching the storage.conf file.

rhatdan commented 2 years ago

I would be ok with this. BUT storage.conf right now does not handle overriding fields. We only replace and fall back to defaults. This is different then what we do in containers.conf and registries.conf, I believe.

Changing this to inheritance my be a breaking change.

Changing it to only inherit from storage.conf.d/* Would not be a breaking change, though.

/usr/share/containers/storage.conf -> /etc/containers/storage.conf/ -> $HOME/.config/storage.conf override completely

/etc/containers/storage.conf.d/ and $HOME/.config/storage.conf/.d/ would only override fields in the default storage.conf.

meox commented 2 years ago

Overrides in which way? I mean if I have multiple have multiple storage.conf.d files and then I run podman images I will able to see all the images from all storage?

rhatdan commented 2 years ago

Right now if I have /usr/containers/storage.conf and /etc/containers/storage.conf, None of the fields in /usr/containers/storage.conf are used or even read. We would need to change this. So that fields in /etc/containers/storage.conf/.d would be merged (replacing just the specified fields) that are present in /etc/containers/storage.conf.

If two drop in files used the same fields, then the last one read would win.

giuseppe commented 2 years ago

What would be the way to add an additional store without completely overriding the previous ones? Even the inherit semantics we have now won't allow it

meox commented 2 years ago

I'd like to have the opportunity to add multiple storage (AdditionalImageStores) just adding multiple config files in a folder. What @rhatdan said seems not able to cover my proposed use case.

rhatdan commented 2 years ago

Yes you would have to add multiple images to a single file.

meox commented 2 years ago

Yes you would have to add multiple images to a single file.

ok but is just like now if I understand correctly

jacksgt commented 1 year ago

I just want to chime in and say that having the possibility of defining overrides in a storage.conf.d directory (like it is currently possible for containers.conf) would be super handy for OpenShift/OKD, where we want to modify the upstream config as little as possible (ideally not at all) to avoid issues when upgrading RHCOS / FCOS nodes in place.

rhatdan commented 1 year ago

Best way to make this happen is to open a PR.

jacksgt commented 1 year ago

Alright, at least it's good to know that a PR is welcome for this :-)

rhatdan commented 1 year ago

Yes I think this is a good idea. If we had it to do all over again, I think we would have done storage.conf the same as containers.conf.

Since .d directories are a new feature, I see them as enhancing the storage.conf file rather then replacing.