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

Unconditionally refresh storage options from config #2268

Merged

Conversation

mheon
Copy link
Member

@mheon mheon commented Feb 5, 2019

Due to our unconditionally setting some storage options, we are not always reading storage options from storage.conf. This can lead to some fields in the storage config (most notably extra storage options) being ignored, despite being set in storage.conf.

Resolve this by unconditionally refreshing our storage config from storage.conf (this was previously only done for rootless Podman)

Fixes #2217

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M labels Feb 5, 2019
@mheon
Copy link
Member Author

mheon commented Feb 5, 2019

Hm. Is it auto-approving PRs from approvers?

@rhatdan
Copy link
Member

rhatdan commented Feb 5, 2019

LGTM
@giuseppe PTAL

@Silvanoc
Copy link

Silvanoc commented Feb 5, 2019

LGTM

Branch tested on Arch Linux and behaves as expected.

@openshift-merge-robot
Copy link
Collaborator

/retest

@giuseppe
Copy link
Member

giuseppe commented Feb 6, 2019

it seems to break the creation of ~/.config/containers/storage.conf when it doesn't exist.

Reproducer:

$ rm ~/.config/containers/storage.conf
$ bin/podman run --rm -ti alpine echo hi
$ cat ~/.config/containers/storage.conf
cat: /home/gscrivano/.config/containers/storage.conf: No such file or directory

pkg/util/utils.go Outdated Show resolved Hide resolved
@mheon mheon force-pushed the force_storage_refresh branch from 1e2f31d to e46c549 Compare February 6, 2019 14:40
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

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

@giuseppe
Copy link
Member

giuseppe commented Feb 6, 2019

LGTM

@openshift-merge-robot
Copy link
Collaborator

/retest

Due to our unconditionally setting some storage options, we
are not always reading storage options from storage.conf. This
can lead to some fields in the storage config (most notably extra
storage options) being ignored, despite being set in
storage.conf.

Resolve this by unconditionally refreshing our storage config
from storage.conf (this was previously only done for rootless
Podman)

Fixes containers#2217

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@mheon mheon force-pushed the force_storage_refresh branch from e46c549 to 33845f8 Compare February 6, 2019 15:01
@Silvanoc
Copy link

Silvanoc commented Feb 6, 2019

LGTM

Current version of the branch behaves as expected on my tests for both root and rootless containers.

@baude
Copy link
Member

baude commented Feb 6, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 6, 2019
@openshift-merge-robot openshift-merge-robot merged commit 74e71c4 into containers:master Feb 6, 2019
@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 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 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. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants