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

mount: Add volume-subpath option #4331

Merged
merged 1 commit into from Feb 23, 2024
Merged

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Jun 2, 2023

- What I did

  • Added volume-subpath option to the long --mount format.

- How I did it

- How to verify it
TestMountSubvolume test (more tests in Moby)

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2023

Codecov Report

Merging #4331 (edc09e6) into master (9831fea) will decrease coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4331      +/-   ##
==========================================
- Coverage   61.31%   61.30%   -0.01%     
==========================================
  Files         287      287              
  Lines       20058    20060       +2     
==========================================
  Hits        12298    12298              
- Misses       6867     6869       +2     
  Partials      893      893              

@vvoland vvoland force-pushed the mount-volsubpath branch 2 times, most recently from 2939ede to 8177eea Compare July 4, 2023 11:16
@thaJeztah thaJeztah modified the milestones: 25.0.0, 26.0.0 Jan 19, 2024
@vvoland vvoland marked this pull request as ready for review January 24, 2024 13:21
@vvoland vvoland self-assigned this Jan 24, 2024
@vvoland vvoland changed the title WIP: mount: Add volume-subpath option mount: Add volume-subpath option Jan 24, 2024
e2e/container/run_test.go Outdated Show resolved Hide resolved
e2e/container/run_test.go Outdated Show resolved Hide resolved
Comment on lines 154 to 155
func TestMountSubvolume(t *testing.T) {
environment.SkipIfAPIOlder(t, "1.45")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: This test will be skipped currently (even with #4839) because the 25.0 is still on API 1.44.

I ran the tests (🟢) locally by substituting the dind image with:

FROM docker:25-dind
COPY --from=moby/moby-bin:master /dockerd /usr/local/bin/dockerd
Tests output
$ make -f docker.Makefile test-e2e
...
✓  e2e/cli-plugins (15.291s)
∅  e2e/cli-plugins/plugins/badmeta
∅  e2e/cli-plugins/plugins/nopersistentprerun
∅  e2e/cli-plugins/plugins/presocket
∅  e2e/internal/fixtures
✓  e2e/global (71ms)
✓  e2e/context (132ms)
∅  e2e/plugin/basic
✓  e2e/plugin (112ms)
✓  e2e/system (37ms)
✓  e2e/stack (264ms)
✓  e2e/trust (1.974s)
✓  e2e/container (5.198s)
✓  e2e/image (10.05s)

=== Skipped
=== SKIP: e2e/cli-plugins TestCLIPluginDialStdio (0.00s)
    dial_test.go:16: skipping plugin dial-stdio test since DOCKER_CLI_PLUGIN_USE_DIAL_STDIO is not set

=== SKIP: e2e/container TestRunAttachedFromRemoteImageAndRemove (0.01s)
    run_test.go:24: daemonPlatform != platform: running against a non linux/amd64 daemon

=== SKIP: e2e/container TestRunWithCgroupNamespace (0.06s)
    run_test.go:147: !cgroupNsFound: running against a daemon that doesn't support cgroup namespaces (security options: [name=seccomp,profile=builtin]
        )

=== SKIP: e2e/image TestPullWithContentTrust (0.01s)
    pull_test.go:22: daemonPlatform != platform: running against a non linux/amd64 daemon

=== SKIP: e2e/image TestPushAllTags (0.01s)
    push_test.go:37: daemonPlatform != platform: running against a non linux/amd64 daemon

=== SKIP: e2e/image TestPushWithContentTrust (0.01s)
    push_test.go:59: daemonPlatform != platform: running against a non linux/amd64 daemon

=== SKIP: e2e/plugin TestInstallWithContentTrust (0.00s)
    trust_test.go:26: skipping until a fix is implemented on moby

=== SKIP: e2e/trust TestSignLocalImage (0.01s)
    sign_test.go:25: daemonPlatform != platform: running against a non linux/amd64 daemon

=== SKIP: e2e/trust TestSignWithLocalFlag (0.01s)
    sign_test.go:43: daemonPlatform != platform: running against a non linux/amd64 daemon

Copy link
Contributor

@krissetto krissetto left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM!

@krissetto krissetto mentioned this pull request Feb 12, 2024
Comment on lines 112 to 115

func SkipIfAPIOlder(t *testing.T, minimumVersion string) {
t.Helper()
// Use Client.APIVersion instead of Server.APIVersion.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done and extracted to #4873

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit dc6bfac into docker:master Feb 23, 2024
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants