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 DetailsNeeds 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.
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.
0441674 to
0fbd068
Compare
follow-up for ccb70a7 more information here: containers#1827 (comment) Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
0fbd068 to
846f78a
Compare
|
/lgtm |
follow-up for ccb70a7
more information here: #1827 (comment)