-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add --device support for Windows #1290
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
// +build !windows | ||
|
||
package container | ||
|
||
import ( | ||
"fmt" | ||
"path" | ||
"strings" | ||
|
||
"github.com/docker/docker/api/types/container" | ||
"github.com/pkg/errors" | ||
) | ||
|
||
// parseDevice parses a device mapping string to a container.DeviceMapping struct | ||
func parseDevice(device string) (container.DeviceMapping, error) { | ||
src := "" | ||
dst := "" | ||
permissions := "rwm" | ||
arr := strings.Split(device, ":") | ||
switch len(arr) { | ||
case 3: | ||
permissions = arr[2] | ||
fallthrough | ||
case 2: | ||
if validDeviceMode(arr[1]) { | ||
permissions = arr[1] | ||
} else { | ||
dst = arr[1] | ||
} | ||
fallthrough | ||
case 1: | ||
src = arr[0] | ||
default: | ||
return container.DeviceMapping{}, errors.Errorf("invalid device specification: %s", device) | ||
} | ||
|
||
if dst == "" { | ||
dst = src | ||
} | ||
|
||
deviceMapping := container.DeviceMapping{ | ||
PathOnHost: src, | ||
PathInContainer: dst, | ||
CgroupPermissions: permissions, | ||
} | ||
return deviceMapping, nil | ||
} | ||
|
||
// validDeviceMode checks if the mode for device is valid or not. | ||
// Valid mode is a composition of r (read), w (write), and m (mknod). | ||
func validDeviceMode(mode string) bool { | ||
var legalDeviceMode = map[rune]bool{ | ||
'r': true, | ||
'w': true, | ||
'm': true, | ||
} | ||
if mode == "" { | ||
return false | ||
} | ||
for _, c := range mode { | ||
if !legalDeviceMode[c] { | ||
return false | ||
} | ||
legalDeviceMode[c] = false | ||
} | ||
return true | ||
} | ||
|
||
// validateDevice validates a path for devices | ||
// It will make sure 'val' is in the form: | ||
// [host-dir:]container-path[:mode] | ||
// It also validates the device mode. | ||
func validateDevice(val string) (string, error) { | ||
return validatePath(val, validDeviceMode) | ||
} | ||
|
||
func validatePath(val string, validator func(string) bool) (string, error) { | ||
var containerPath string | ||
var mode string | ||
|
||
if strings.Count(val, ":") > 2 { | ||
return val, errors.Errorf("bad format for path: %s", val) | ||
} | ||
|
||
split := strings.SplitN(val, ":", 3) | ||
if split[0] == "" { | ||
return val, errors.Errorf("bad format for path: %s", val) | ||
} | ||
switch len(split) { | ||
case 1: | ||
containerPath = split[0] | ||
val = path.Clean(containerPath) | ||
case 2: | ||
if isValid := validator(split[1]); isValid { | ||
containerPath = split[0] | ||
mode = split[1] | ||
val = fmt.Sprintf("%s:%s", path.Clean(containerPath), mode) | ||
} else { | ||
containerPath = split[1] | ||
val = fmt.Sprintf("%s:%s", split[0], path.Clean(containerPath)) | ||
} | ||
case 3: | ||
containerPath = split[1] | ||
mode = split[2] | ||
if isValid := validator(split[2]); !isValid { | ||
return val, errors.Errorf("bad mode specified: %s", mode) | ||
} | ||
val = fmt.Sprintf("%s:%s:%s", split[0], containerPath, mode) | ||
} | ||
|
||
if !path.IsAbs(containerPath) { | ||
return val, errors.Errorf("%s is not an absolute path", containerPath) | ||
} | ||
return val, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// +build windows | ||
|
||
package container | ||
|
||
import ( | ||
"strings" | ||
|
||
"github.com/docker/docker/api/types/container" | ||
"github.com/pkg/errors" | ||
) | ||
|
||
// parseDevice parses a device mapping string to a container.DeviceMapping struct | ||
func parseDevice(device string) (container.DeviceMapping, error) { | ||
return container.DeviceMapping{ | ||
PathOnHost: device, | ||
}, nil | ||
} | ||
|
||
// validateDevice validates a path for devices | ||
// It will make sure 'val' is in the form: | ||
// class/{clsid} | ||
func validateDevice(val string) (string, error) { | ||
arr := strings.Split(val, ":") | ||
switch len(arr) { | ||
case 1: | ||
if !strings.HasPrefix(arr[0], "class") { | ||
return "", errors.New("device must have prefix: 'class'") | ||
} | ||
return val, nil | ||
} | ||
|
||
return "", errors.Errorf("device must be in the format 'class/{clsid}'") | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🤔 Not compiling this on Windows means that starting a Linux container (Docker Desktop for Windows, or a Windows client connecting to a remote Linux daemon) will no longer be functional; I guess that's not what we want 😅
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.
How do you suggest I determine when to use what parser for the device assignment? I would have to ping the end node to determine if its a Windows or Linux host first?
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.
Take a look at cli\command\cli.go L212, function initializeFromClient. You can pull the server OS out of
cli.serverInfo.OSType
. IIRC every CLI command starts with a _ping API.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.
Yes, unfortunately it gets complicated fast to take mixed environments into account 😞. I guess we should keep the separate functions for windows and linux (to keep it clean), and add a wrapper function that calls one of them, based on what the target platform is
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.
If possible (not sure that is in this case (haven't looked in depth)), we try to reduce validation on the client side to a minimum, and leave it to the daemon to return an error.