Skip to content

Commit

Permalink
Merge pull request #15851 from spowelljr/fixDockerVersionPanic
Browse files Browse the repository at this point in the history 
Prevent panic if docker version returns exit code 0 with unexpected output
  • Loading branch information
@medyagh
medyagh committed Feb 15, 2023
2 parents ddaa494 + a93baae commit 906f30c
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 39 deletions.
84 changes: 48 additions & 36 deletions pkg/minikube/registry/drvs/docker/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,32 +94,9 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) {
}

func status() (retState registry.State) {
_, err := exec.LookPath(oci.Docker)
if err != nil {
return registry.State{Error: err, Installed: false, Healthy: false, Fix: "Install Docker", Doc: docURL}
}

ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second)
defer cancel()

cmd := exec.CommandContext(ctx, oci.Docker, "version", "--format", "{{.Server.Os}}-{{.Server.Version}}:{{.Server.Platform.Name}}")
o, err := cmd.Output()
if err != nil {
reason := ""
if ctx.Err() == context.DeadlineExceeded {
err = errors.Wrapf(err, "deadline exceeded running %q", strings.Join(cmd.Args, " "))
reason = "PROVIDER_DOCKER_DEADLINE_EXCEEDED"
}

klog.Warningf("docker version returned error: %v", err)

if exitErr, ok := err.(*exec.ExitError); ok {
stderr := strings.TrimSpace(string(exitErr.Stderr))
newErr := fmt.Errorf(`%q %v: %s`, strings.Join(cmd.Args, " "), exitErr, stderr)
return suggestFix("version", exitErr.ExitCode(), stderr, newErr)
}

return registry.State{Reason: reason, Error: err, Installed: true, Healthy: false, Fix: "Restart the Docker service", Doc: docURL}
version, state := dockerVersionOrState()
if state.Error != nil {
return state
}

var improvement string
Expand All @@ -135,15 +112,18 @@ func status() (retState registry.State) {
}
}()

versions := strings.Split(string(o), ":")
versions := strings.Split(version, ":")
if len(versions) < 2 {
versions = append(versions, "")
}
dockerEngineVersion := versions[0]
dockerPlatformVersion := versions[1]
klog.Infof("docker version: %s", o)
klog.Infof("docker version: %s", version)
if !viper.GetBool("force") {
if s := checkDockerDesktopVersion(dockerPlatformVersion); s != nil {
return *s
if s := checkDockerDesktopVersion(dockerPlatformVersion); s.Error != nil {
return s
}
s := checkDockerEngineVersion(strings.TrimSpace(dockerEngineVersion)) // remove '\n' from o at the end
s := checkDockerEngineVersion(dockerEngineVersion)
if s.Error != nil {
return s
}
Expand All @@ -165,6 +145,38 @@ func status() (retState registry.State) {
return checkNeedsImprovement()
}

var dockerVersionOrState = func() (string, registry.State) {
if _, err := exec.LookPath(oci.Docker); err != nil {
return "", registry.State{Error: err, Installed: false, Healthy: false, Fix: "Install Docker", Doc: docURL}
}

ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second)
defer cancel()

cmd := exec.CommandContext(ctx, oci.Docker, "version", "--format", "{{.Server.Os}}-{{.Server.Version}}:{{.Server.Platform.Name}}")
o, err := cmd.Output()
if err == nil {
return string(o), registry.State{}
}

reason := ""
if ctx.Err() == context.DeadlineExceeded {
err = errors.Wrapf(err, "deadline exceeded running %q", strings.Join(cmd.Args, " "))
reason = "PROVIDER_DOCKER_DEADLINE_EXCEEDED"
}

klog.Warningf("docker version returned error: %v", err)

exitErr, ok := err.(*exec.ExitError)
if !ok {
return "", registry.State{Reason: reason, Error: err, Installed: true, Healthy: false, Fix: "Restart the Docker service", Doc: docURL}
}

stderr := strings.TrimSpace(string(exitErr.Stderr))
newErr := fmt.Errorf(`%q %v: %s`, strings.Join(cmd.Args, " "), exitErr, stderr)
return "", suggestFix("version", exitErr.ExitCode(), stderr, newErr)
}

func checkDockerEngineVersion(o string) registry.State {
parts := strings.SplitN(o, "-", 2)
if len(parts) != 2 {
Expand Down Expand Up @@ -246,25 +258,25 @@ func checkDockerEngineVersion(o string) registry.State {
Doc: docURL + "#requirements"}
}

func checkDockerDesktopVersion(version string) *registry.State {
func checkDockerDesktopVersion(version string) (s registry.State) {
fields := strings.Fields(version)
if len(fields) < 3 || fields[0] != "Docker" || fields[1] != "Desktop" {
return nil
return s
}
currSemver, err := semver.Parse(fields[2])
if err != nil {
return nil
return s
}
if currSemver.EQ(semver.MustParse("4.16.0")) {
return &registry.State{
return registry.State{
Reason: "PROVIDER_DOCKER_DESKTOP_VERSION_BAD",
Running: true,
Error: errors.New("Docker Desktop 4.16.0 has a regression that prevents minikube from starting"),
Installed: true,
Fix: "Update Docker Desktop to 4.16.1 or greater",
}
}
return nil
return s
}

// checkNeedsImprovement if overlay mod is installed on a system
Expand Down
28 changes: 25 additions & 3 deletions pkg/minikube/registry/drvs/docker/docker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/blang/semver/v4"
"k8s.io/minikube/pkg/minikube/driver"
"k8s.io/minikube/pkg/minikube/registry"
)

type testCase struct {
Expand Down Expand Up @@ -154,7 +155,7 @@ func TestCheckDockerEngineVersion(t *testing.T) {
func TestCheckDockerDesktopVersion(t *testing.T) {
tests := []struct {
input string
shouldReturnState bool
shouldReturnError bool
}{
{"Docker Desktop", false},
{"Cat Desktop 4.16.0", false},
Expand All @@ -165,8 +166,29 @@ func TestCheckDockerDesktopVersion(t *testing.T) {
}
for _, tt := range tests {
state := checkDockerDesktopVersion(tt.input)
if (state == nil && tt.shouldReturnState) || (state != nil && !tt.shouldReturnState) {
t.Errorf("checkDockerDesktopVersion(%q) = %v; expected shouldRetunState = %t", tt.input, state, tt.shouldReturnState)
err := state.Error
if (err == nil && tt.shouldReturnError) || (err != nil && !tt.shouldReturnError) {
t.Errorf("checkDockerDesktopVersion(%q) = %+v; expected shouldReturnError = %t", tt.input, state, tt.shouldReturnError)
}
}
}

func TestStatus(t *testing.T) {
tests := []struct {
input string
shouldReturnError bool
}{
{"linux-20.10.22:Docker Desktop 4.16.2 (95914)", false},
{"noDashHere:Docker Desktop 4.16.2 (95914)", true},
{"linux-20.10.22:Docker Desktop 4.16.0 (95914)", true},
{"", true},
}
for _, tt := range tests {
dockerVersionOrState = func() (string, registry.State) { return tt.input, registry.State{} }
state := status()
err := state.Error
if (err == nil && tt.shouldReturnError) || (err != nil && !tt.shouldReturnError) {
t.Errorf("status(%q) = %+v; expected shouldReturnError = %t", tt.input, state, tt.shouldReturnError)
}
}
}

0 comments on commit 906f30c

Please sign in to comment.