Skip to content

create directory in container using mkdir -p#10652

Merged
glours merged 1 commit intodocker:v2from
ndeloof:watch_mkdir
Jun 8, 2023
Merged

create directory in container using mkdir -p#10652
glours merged 1 commit intodocker:v2from
ndeloof:watch_mkdir

Conversation

@ndeloof
Copy link
Copy Markdown
Contributor

@ndeloof ndeloof commented Jun 5, 2023

What I did
on fsevent related to a directory, docker exec mkdir -p to ensure target directory exists as we later run docker cp otherwise PutContainerArchive will fail with 404 as documented on https://docs.docker.com/engine/api/v1.42/#tag/Container/operation/PutContainerArchive

Related issue
fixes #10644
https://docker.atlassian.net/browse/ENV-204

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@ndeloof ndeloof force-pushed the watch_mkdir branch 2 times, most recently from f797f00 to fa8e5af Compare June 5, 2023 08:28
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2023

Codecov Report

Patch coverage: 18.75% and project coverage change: -40.36 ⚠️

Comparison is base (60fe974) 59.75% compared to head (852c9e8) 19.39%.

Additional details and impacted files
@@             Coverage Diff             @@
##               v2   #10652       +/-   ##
===========================================
- Coverage   59.75%   19.39%   -40.36%     
===========================================
  Files         107      104        -3     
  Lines        9481     9411       -70     
===========================================
- Hits         5665     1825     -3840     
- Misses       3234     7391     +4157     
+ Partials      582      195      -387     
Impacted Files Coverage Δ
pkg/compose/watch.go 22.09% <0.00%> (-2.81%) ⬇️
pkg/utils/writer.go 67.74% <100.00%> (+7.74%) ⬆️

... and 80 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ndeloof ndeloof requested review from a team, StefanScherer, glours, milas, nicksieger and ulyssessouza and removed request for a team June 5, 2023 08:40
Copy link
Copy Markdown
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, kind of annoying that there's no option in the Engine API to create the path if necessary (both because this means an extra API call & because it'll necessitate having a working mkdir in the image, though I'm fine with that assumption/limitation tbh 🙃)

Comment thread pkg/compose/watch.go Outdated
_, err := s.Exec(ctx, project.Name, api.RunOptions{
Service: opt.Service,
Command: []string{"rm", "-rf", opt.ContainerPath},
Index: 1,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Index: 1,
Index: i,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stupid me ...

Copy link
Copy Markdown
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@glours glours enabled auto-merge June 8, 2023 09:28
@glours glours merged commit 2d22c2b into docker:v2 Jun 8, 2023
@ndeloof ndeloof deleted the watch_mkdir branch December 20, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] docker compose sync doesn't create folder

3 participants