Feature/windows support #35
Conversation
rtluckie
commented
Aug 23, 2018
rtluckie
commented
- revert back to debian:9.5 image
- add user friendly outputs to init command
- add verbose flag
- use docker mounts and volumes instead of binds
- misc fixes for to support windows
- refactor provision-user script
- bump version number
- revert back to debian:9.5 image - add user friendly outputs to init command - add verbose flag - use docker mounts and volumes instead of binds - misc fixes for to support windows - refactor provision-user script - bump version number
rtluckie
requested review from
wozniakjan,
dcwangmit01,
drekle,
simt2 and
a user
README.md
Outdated
| @@ -123,18 +125,21 @@ cd ~/ | |||
| OSX: mkdir ~/Dev; cd ~/Dev | |||
| Windows: <Ignore This> | |||
|
|
|||
| # Start ssh-agent and load your key | |||
There was a problem hiding this comment.
MUST:
So this clearly is not necessary for the KDK, but unfortunately most people in our company don't have the ssh skills to be able to access github and bitbucket via ssh within the KDK. For this they need ssh-agent, and they need it either spelled out or automated.
Can you figure out a way to either automate this or document it in an Appendix section?
There was a problem hiding this comment.
Perhaps we can build it into the kdk
| @@ -42,6 +46,12 @@ func InitKdkConfig(logger logrus.Entry) error { | |||
|
|
|||
| currentUser, _ := user.Current() | |||
|
|
|||
| username := currentUser.Username | |||
|
|
|||
There was a problem hiding this comment.
Please add a comment. I assume that the username has \ in it because of Windows?
|
|
||
| for _, bind := range viper.Get("docker.binds").([]interface{}) { | ||
| configBind := make(map[string]string) | ||
| for key, value := range bind.(map[interface{}]interface{}) { | ||
| configBind[key.(string)] = value.(string) | ||
| } | ||
| binds = append(binds, configBind["source"]+":"+configBind["target"]) | ||
| mounts = append(mounts, mount.Mount{Type: mount.TypeBind, Source: configBind["source"], Target: configBind["target"]}) | ||
| volumes[configBind["target"]] = struct{}{} |
There was a problem hiding this comment.
Why is this sent in as empty?
There was a problem hiding this comment.
not sure why volumes has an empty struct -> https://github.com/docker/swarm/blob/master/vendor/github.com/docker/docker/api/types/container/config.go#L59
There was a problem hiding this comment.
One of these is a type struct{} and one of these is an instantiation of an empty struct struct{}{}
| if Verbose { | ||
| io.Copy(os.Stdout, out) | ||
| } else { | ||
| io.Copy(ioutil.Discard, out) |
There was a problem hiding this comment.
I don't understand why you might do this. Maybe this is something I should know. My understanding is that you are discarding data from a connection that is going to be closed by the defer statement above anyway. Is this useful in some way?
There was a problem hiding this comment.
There was a problem hiding this comment.
@drekle Apparently, without the ioutil.Discard, the code would finish before the command causing the defer statement to trigger and kill the command early. We specifically debugged this mystery. If you're interested, comment out the io.Copy, and try running it. The pull will fail every time.
| "github.com/docker/docker/client" | ||
| "github.com/docker/go-connections/nat" | ||
| "github.com/spf13/viper" | ||
| ) | ||
|
|
||
| func Up(ctx context.Context, dockerClient *client.Client, imageCoordinates string, logger logrus.Entry) error { | ||
| var binds []string | ||
| var volumes map[string]struct{} |
There was a problem hiding this comment.
This should be made the direct type: https://github.com/moby/moby/blob/master/api/types/volume
There was a problem hiding this comment.
There was a problem hiding this comment.
Curious why it shouldn't be:
volumes := map[string]volume.Volume{}
Instead of
var volumes map[string]struct{}
|
Please Close this PR. It has been rebased in #37 |
|
CLosing |
