-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Implemented bind mount type; added tests #4761
Conversation
@@ -168,9 +168,11 @@ type DockerLoggingOpts struct { | |||
} | |||
|
|||
type DockerMount struct { | |||
Type string `mapstructure:"type"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type
should imo be defaulted to volume
to keep it compat for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Added in 4432486#diff-a6757d41ad7cd7f37f0f0243e3575307R403
hm.VolumeOptions.DriverConfig = docker.VolumeDriverConfig{ | ||
Name: dc.Name, | ||
if hm.Type == "bind" { | ||
volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should there be a different client option for allowing dockerVolumesConfigOption
and dockerBindVolumesConfigOption
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this to preserve configuration compatibility between the -v
style implementation and the mount(type=bind) version to ease migration from one to the other. I think that this could be worth further consideration later though.
@jippi, thanks for the eyes on this. It's on-hold behind a refactor, but I very much appreciate you taking the time to review it. |
We will need to apply a62a151 once this is rebased |
Closing in favor of #4958 |
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Implemented the
bind
mount type. This will allow Windows users to mount absolute paths.