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.

Sign up for GitHub

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

[Feature] Docker machine #71

Merged
merged 2 commits into from
Jun 1, 2019
Merged

[Feature] Docker machine #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de6e845
Support docker machine for kubeconfig.yaml
andyz-dev May 29, 2019
f3ec169
Add docker machine IP to SAN
andyz-dev May 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions cli/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"path"
"strconv"
"strings"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/filters"
Expand Down Expand Up @@ -139,8 +140,23 @@ func createKubeConfigFile(cluster string) error {
}
defer kubeconfigfile.Close()

// write to file, skipping the first 512 bytes which contain file metadata and trimming any NULL characters
_, err = kubeconfigfile.Write(bytes.Trim(readBytes[512:], "\x00"))
// write to file, skipping the first 512 bytes which contain file metadata
// and trimming any NULL characters
trimBytes := bytes.Trim(readBytes[512:], "\x00")

// If running on a docker machine, replace localhost with
// docker machine's IP
dockerMachineIp, err := getDockerMachineIp()
if err != nil {
return err
}

if dockerMachineIp != "" {
s := string(trimBytes)
s = strings.Replace(s, "localhost", dockerMachineIp, 1)
trimBytes = []byte(s)
}
_, err = kubeconfigfile.Write(trimBytes)
if err != nil {
return fmt.Errorf("ERROR: couldn't write to kubeconfig.yaml\n%+v", err)
}
Expand Down
7 changes: 7 additions & 0 deletions cli/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ func CreateCluster(c *cli.Context) error {
log.Println("INFO: As of v2.0.0 --port will be used for arbitrary port mapping. Please use --api-port/-a instead for configuring the Api Port")
}
k3sServerArgs := []string{"--https-listen-port", c.String("api-port")}
if ip, err := getDockerMachineIp(); ip != "" || err != nil {
if err != nil {
return err
}
log.Printf("Add TLS SAN for %s", ip)
k3sServerArgs = append(k3sServerArgs, "--tls-san", ip)
}
if c.IsSet("server-arg") || c.IsSet("x") {
k3sServerArgs = append(k3sServerArgs, c.StringSlice("server-arg")...)
}
Expand Down
26 changes: 26 additions & 0 deletions cli/docker-machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package run

import (
"os"
"os/exec"
"strings"
)

func getDockerMachineIp() (string, error) {
machine := os.ExpandEnv("$DOCKER_MACHINE_NAME")
Copy link

Choose a reason for hiding this comment

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

I think calling docker-machine active to get the machine name would be more reliable as environmental variables might not be set or overridden by users.

Also we're calling docker-machine ip instead of DOCKER_HOST already anyways.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just tried doing docker-machine active and didn't get any output (even though one of the machines is running). I am not familiar with this command can you describe more of what needs to be done ?

Copy link

@kkimdev kkimdev Jun 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Oh interesting. Mine outputs default and the explanation is that

$ docker-machine
...
Commands:
  active                Print which machine is active
..

What's your output of docker-machine env? Does it have the following line?

SET DOCKER_MACHINE_NAME=default

Copy link
Collaborator

Choose a reason for hiding this comment

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

Output for docker-machine env dvm "dvm" is the name of the docker-machine

PS C:\WINDOWS\system32> docker-machine env dvm
$Env:DOCKER_TLS_VERIFY = "1"
$Env:DOCKER_HOST = "tcp://172.17.209.142:2376"
$Env:DOCKER_CERT_PATH = "C:\Users\user\.docker\machine\machines\dvm"
$Env:DOCKER_MACHINE_NAME = "dvm"
$Env:COMPOSE_CONVERT_WINDOWS_PATHS = "true"
# Run this command to configure your shell:
# & "C:\Program Files\Docker\Docker\Resources\bin\docker-machine.exe" env dvm | Invoke-Expression

Copy link

Choose a reason for hiding this comment

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

How about docker-machine ls Does "dvm" line has an active mark on the left?

Copy link

Choose a reason for hiding this comment

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

That's really strange then. docker-machine ls reports dvm is active while docker-machine active reports nothing is active. I can't think of anything else than it's a docker-machine bug.

Copy link

Choose a reason for hiding this comment

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

I was able to find similar issues in their repo. docker/machine#1651 docker/machine#3448

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, definitely that issue. What do you suggest ?

Copy link

Choose a reason for hiding this comment

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

Well, I guess we don't have any choice other than DOCKER_HOST. I just looked up their active check, and it's also just a combination of [DOCKER_HOST + current status] https://github.com/docker/machine/blob/19973f2b1bbcb7b1c07a3de572177bcaf8dedcd5/commands/ls.go#L490

Also, docker-machine repository hasn't been updated for several months so we can't expect it to be fixed anytime soon.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Think we should leave it like this for now or create a new issue regarding this.


if machine == "" {
return "", nil
}

dockerMachinePath, err := exec.LookPath("docker-machine")
Copy link

Choose a reason for hiding this comment

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

Can we also check docker-machine.exe? "docker toolbox on windows" installs it as .exe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. It worked for me since I tested using the shell provided by docker toolbox on windows.

Do you mind send a PR that implements this idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to this patch, exec.LookPath() on windows should attempt to find executables with .exe or .com extensions.

https://codereview.appspot.com/13410045/

Copy link

Choose a reason for hiding this comment

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

Thanks for the info. Seems like fairly simple but personally I never used Go before so I'll leave it to others.

if err != nil {
return "", err
}

out, err := exec.Command(dockerMachinePath, "ip", machine).Output()

ipStr := strings.TrimSuffix(string(out), "\n")
ipStr = strings.TrimSuffix(ipStr, "\r")
return ipStr, err
}