Skip to content
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

Allow users to override all storage options at the command line #7315

Closed
wants to merge 1 commit into from

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Aug 13, 2020

podman --root /var/lib/shared --storage-opt="" pull alpine

Tells podman to ignore the storage options defined in storage.conf.

This would allow users to configure additional stores, and still use
podman to update the additional store storage.

Signed-off-by: Daniel J Walsh [email protected]

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 13, 2020
@rhatdan
Copy link
Member Author

rhatdan commented Aug 13, 2020

Fixes: #7309

@TomSweeneyRedHat
Copy link
Member

A test would be good.
Otherwise LGTM

@mheon
Copy link
Member

mheon commented Aug 13, 2020

Concur that a test would be nice

@rhatdan
Copy link
Member Author

rhatdan commented Aug 13, 2020

The problem is there is no good way to test with storage.conf that I can see, without overwriting the system configuration files.
@edsantiago Any ideas? We basically need a system wide storage.conf with storage-opt set, and then we need to test that this
podman --storage-opt "" info --format "{{.Store.GraphOptions}}"
Ends up equal to "".

@edsantiago
Copy link
Member

I was just looking at this PR and pondering that very question. I've long wanted a way to test config files, but really the only sane way to do that is with a --config-file-path=xxx command-line option. With rootless it could be possible to override by redefining $HOME or $XDG_SOMETHING`, but that would have cascading effects because of image/container storage.

For a case like this, where you don't need podman run or anything that actually uses images, I think you could:

@test ..... {
    fake_conf=$PODMAN_TMPDIR/home/.config/containers/containers.conf
    mkdir -[ $(dirname $fake_conf)
    cat >$fake_conf <<EOF
blah blah blah
EOF

    HOME=$PODMAN_TMPDIR/home run_podman --storage-opt="" info ...

Disclaimer: I typed code directly into a browser without testing. That is always a bad idea.

@rhatdan
Copy link
Member Author

rhatdan commented Aug 13, 2020

Sadly this PR will only work for rootfull podman

@rhatdan
Copy link
Member Author

rhatdan commented Aug 17, 2020

Need containers/common#262 in order to add tests.

@rhatdan rhatdan force-pushed the storageopts branch 3 times, most recently from 954dc58 to 195c66e Compare August 22, 2020 10:27
@rhatdan rhatdan force-pushed the storageopts branch 2 times, most recently from 337ef74 to 8aa5243 Compare August 28, 2020 10:06
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

podman --root /var/lib/shared --storage-opt="" pull alpine

Tells podman to ignore the storage options defined in storage.conf.

This would allow users to configure additional stores, and still use
podman to update the additional store storage.

Signed-off-by: Daniel J Walsh <[email protected]>
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@vrothberg
Copy link
Member

@rhatdan, let's get things moving before we the PR has birthday :)

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2021
@openshift-ci-robot
Copy link
Collaborator

@rhatdan: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@rhatdan
Copy link
Member Author

rhatdan commented Apr 22, 2021

I have fixed this via other means.

@rhatdan rhatdan closed this Apr 22, 2021
@adamish
Copy link

adamish commented Jun 21, 2021

Another possible solution could be podman providing a command that modified the storage.conf file in a safe and consistent way, something like

podman conf add-additionalimagestore "/foo"
podman conf remove-additionalimagestore "/foo"

@rhatdan
Copy link
Member Author

rhatdan commented Jun 22, 2021

That would be a very complex tool and would need to be modified each time a new config entry was added to containers.conf. If there was a simple way to autogenerate the tool, I might consider it. but this seems like you are building a toml editor.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. stale-pr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants