overlay: create the merged path only if it does not exist #1858
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe 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 |
drivers/overlay/overlay.go
Outdated
| if err := idtools.MkdirAs(mergedDir, 0o700, rootUID, rootGID); err != nil && !os.IsExist(err) { | ||
| return "", err | ||
| // attempt to create the merged dir only if it doesn't exist. | ||
| if _, err := os.Stat(mergedDir); err != nil { |
There was a problem hiding this comment.
I think we need to move the os.IsExist check here.
Any chance we can add a test?
There was a problem hiding this comment.
Probably not a bad idea to add byt the MkdirAs command below will blow up with similar error.
giuseppe
force-pushed
the
create-merged-only-if-it-doesnot-exist
branch
from
09b8195
to
0441674
Compare
|
added test and amended the suggestion |
drivers/overlay/overlay.go
Outdated
| if err := idtools.MkdirAs(mergedDir, 0o700, rootUID, rootGID); err != nil && !os.IsExist(err) { | ||
| return "", err | ||
| // Attempt to create the merged dir only if it doesn't exist. | ||
| if _, err := os.Stat(mergedDir); err != nil && !os.IsExist(err) { |
There was a problem hiding this comment.
| if _, err := os.Stat(mergedDir); err != nil && !os.IsExist(err) { | |
| if _, err := os.Stat(mergedDir); err != nil && os.IsExist(err) { |
There was a problem hiding this comment.
ah, it should really be os.IsNotExist(err)
There was a problem hiding this comment.
And you are not supposted to use IsExist any longer, errors.Is(err, fs.ErrNotExist) But it looks like we have some cases of IsNotExist in podman.
giuseppe
force-pushed
the
create-merged-only-if-it-doesnot-exist
branch
from
0441674
to
0fbd068
Compare
follow-up for ccb70a7 more information here: containers#1827 (comment) Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
create-merged-only-if-it-doesnot-exist
branch
from
0fbd068
to
846f78a
Compare
|
/lgtm |
follow-up for ccb70a7
more information here: #1827 (comment)