Merge pull request #13842 from spowelljr/requireDockerVersion

Implemented minimum and recommended Docker versions
kubernetes · Mar 24, 2022 · 016ef2e · 016ef2e
2 parents 2b63192 + 4f6747b
commit 016ef2e
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 48 deletions.
diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go
@@ -22,12 +22,13 @@ import (
 	"os"
 	"os/exec"
 	"runtime"
-	"strconv"
 	"strings"
 	"time"
 
+	"github.com/blang/semver/v4"
 	"github.com/docker/machine/libmachine/drivers"
 	"github.com/pkg/errors"
+	"github.com/spf13/viper"
 	"k8s.io/klog/v2"
 	"k8s.io/minikube/pkg/drivers/kic"
 	"k8s.io/minikube/pkg/drivers/kic/oci"
@@ -37,8 +38,11 @@ import (
 	"k8s.io/minikube/pkg/minikube/registry"
 )
 
-var docURL = "https://minikube.sigs.k8s.io/docs/drivers/docker/"
-var minDockerVersion = []int{18, 9, 0}
+const (
+	docURL                   = "https://minikube.sigs.k8s.io/docs/drivers/docker/"
+	minDockerVersion         = "18.09.0"
+	recommendedDockerVersion = "20.10.0"
+)
 
 func init() {
 	if err := registry.Register(registry.DriverDef{
@@ -130,11 +134,13 @@ func status() (retState registry.State) {
 	}()
 
 	klog.Infof("docker version: %s", o)
-	s := checkDockerVersion(strings.TrimSpace(string(o))) // remove '\n' from o at the end
-	if s.Error != nil {
-		return s
+	if !viper.GetBool("force") {
+		s := checkDockerVersion(strings.TrimSpace(string(o))) // remove '\n' from o at the end
+		if s.Error != nil {
+			return s
+		}
+		recordImprovement(s)
 	}
-	recordImprovement(s)
 
 	si, err := oci.CachedDaemonInfo("docker")
 	if err != nil {
@@ -174,8 +180,9 @@ func checkDockerVersion(o string) registry.State {
 		}
 	}
 
-	hintInstallOfficial := fmt.Sprintf("Install the official release of %s (Minimum recommended version is %2d.%02d.%d, current version is %s)",
-		driver.FullName(driver.Docker), minDockerVersion[0], minDockerVersion[1], minDockerVersion[2], parts[1])
+	versionMsg := fmt.Sprintf("(Minimum recommended version is %s, minimum supported version is %s, current version is %s)", recommendedDockerVersion, minDockerVersion, parts[1])
+	hintInstallOfficial := fmt.Sprintf("Install the official release of %s %s", driver.FullName(driver.Docker), versionMsg)
+	hintUpdate := fmt.Sprintf("Upgrade %s to a newer version %s", driver.FullName(driver.Docker), versionMsg)
 
 	p := strings.SplitN(parts[1], ".", 3)
 	switch l := len(p); l {
@@ -195,31 +202,40 @@ func checkDockerVersion(o string) registry.State {
 		}
 	}
 
-	for i, s := range p {
-		k, err := strconv.Atoi(s)
-		if err != nil {
-			return registry.State{
-				Installed:        true,
-				Healthy:          true,
-				NeedsImprovement: true,
-				Fix:              hintInstallOfficial,
-				Doc:              docURL,
-			}
+	currSemver, err := semver.ParseTolerant(strings.Join(p, "."))
+	if err != nil {
+		return registry.State{
+			Installed:        true,
+			Healthy:          true,
+			NeedsImprovement: true,
+			Fix:              hintInstallOfficial,
+			Doc:              docURL,
 		}
+	}
+	// these values are consts and their conversions are covered in unit tests
+	minSemver, _ := semver.ParseTolerant(minDockerVersion)
+	recSemver, _ := semver.ParseTolerant(recommendedDockerVersion)
 
-		if k > minDockerVersion[i] {
-			return registry.State{Installed: true, Healthy: true, Error: nil}
-		} else if k < minDockerVersion[i] {
-			return registry.State{
-				Installed:        true,
-				Healthy:          true,
-				NeedsImprovement: true,
-				Fix:              fmt.Sprintf("Upgrade %s to a newer version (Minimum recommended version is %2d.%02d.%d)", driver.FullName(driver.Docker), minDockerVersion[0], minDockerVersion[1], minDockerVersion[2]),
-				Doc:              docURL + "#requirements"}
-		}
+	if currSemver.GTE(recSemver) {
+		return registry.State{Installed: true, Healthy: true, Error: nil}
+	}
+	if currSemver.GTE(minSemver) {
+		return registry.State{
+			Installed:        true,
+			Healthy:          true,
+			NeedsImprovement: true,
+			Fix:              hintUpdate,
+			Doc:              docURL + "#requirements"}
 	}
 
-	return registry.State{Installed: true, Healthy: true, Error: nil}
+	return registry.State{
+		Reason:           "PROVIDER_DOCKER_VERSION_LOW",
+		Error:            oci.ErrMinDockerVersion,
+		Installed:        true,
+		Healthy:          false,
+		NeedsImprovement: true,
+		Fix:              hintUpdate,
+		Doc:              docURL + "#requirements"}
 }
 
 // checkNeedsImprovement if overlay mod is installed on a system

diff --git a/pkg/minikube/registry/drvs/docker/docker_test.go b/pkg/minikube/registry/drvs/docker/docker_test.go
@@ -21,6 +21,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/blang/semver/v4"
 	"k8s.io/minikube/pkg/minikube/driver"
 )
 
@@ -55,58 +56,81 @@ func appendVersionVariations(tc []testCase, v []int, reason string) []testCase {
 	return appendedTc
 }
 
+func stringToIntSlice(t *testing.T, s string) []int {
+	sem, err := semver.ParseTolerant(s)
+	if err != nil {
+		t.Fatalf("failed to parse %s to semver: %v", s, err)
+	}
+	return []int{int(sem.Major), int(sem.Minor), int(sem.Patch)}
+}
+
 func TestCheckDockerVersion(t *testing.T) {
+	recParts := stringToIntSlice(t, recommendedDockerVersion)
+	minParts := stringToIntSlice(t, minDockerVersion)
+
 	tc := []testCase{
 		{
 			version: "windows-20.0.1",
 			expect:  "PROVIDER_DOCKER_WINDOWS_CONTAINERS",
 		},
 		{
-			version: fmt.Sprintf("linux-%02d.%02d", minDockerVersion[0], minDockerVersion[1]),
+			version: fmt.Sprintf("linux-%02d.%02d", recParts[0], recParts[1]),
 			expect:  "",
 		},
 		{
-			version: fmt.Sprintf("linux-%02d.%02d.%02d", minDockerVersion[0], minDockerVersion[1], minDockerVersion[2]),
+			version: fmt.Sprintf("linux-%s", recommendedDockerVersion),
 			expect:  "",
 		},
 	}
 
 	for i := 0; i < 3; i++ {
 		v := make([]int, 3)
-		copy(v, minDockerVersion)
+		copy(v, minParts)
 
-		v[i] = minDockerVersion[i] + 1
+		v[i] = minParts[i] + 1
 		tc = appendVersionVariations(tc, v, "")
 
-		v[i] = minDockerVersion[i] - 1
+		v[i] = minParts[i] - 1
 		if v[2] < 0 {
 			// skip test if patch version is negative number.
 			continue
 		}
 		tc = appendVersionVariations(tc, v, "PROVIDER_DOCKER_VERSION_LOW")
 	}
 
+	recommendedSupported := fmt.Sprintf("Minimum recommended version is %s, minimum supported version is %s", recommendedDockerVersion, minDockerVersion)
+	install := fmt.Sprintf("Install the official release of %s (%s, current version is %%s)", driver.FullName(driver.Docker), recommendedSupported)
+	update := fmt.Sprintf("Upgrade %s to a newer version (%s, current version is %%s)", driver.FullName(driver.Docker), recommendedSupported)
 	tc = append(tc, []testCase{
 		{
 			// "dev" is set when Docker (Moby) was installed with `make binary && make install`
-			version: "linux-dev",
-			expect:  "",
-			expectFixContains: fmt.Sprintf("Install the official release of %s (Minimum recommended version is %02d.%02d.%d, current version is dev)",
-				driver.FullName(driver.Docker), minDockerVersion[0], minDockerVersion[1], minDockerVersion[2]),
+			version:           "linux-dev",
+			expect:            "",
+			expectFixContains: fmt.Sprintf(install, "dev"),
 		},
 		{
 			// "library-import" is set when Docker (Moby) was installed with `go build github.com/docker/docker/cmd/dockerd` (unrecommended, but valid)
-			version: "linux-library-import",
-			expect:  "",
-			expectFixContains: fmt.Sprintf("Install the official release of %s (Minimum recommended version is %02d.%02d.%d, current version is library-import)",
-				driver.FullName(driver.Docker), minDockerVersion[0], minDockerVersion[1], minDockerVersion[2]),
+			version:           "linux-library-import",
+			expect:            "",
+			expectFixContains: fmt.Sprintf(install, "library-import"),
 		},
 		{
 			// "foo.bar.baz" is a triplet that cannot be parsed as "%02d.%02d.%d"
-			version: "linux-foo.bar.baz",
-			expect:  "",
-			expectFixContains: fmt.Sprintf("Install the official release of %s (Minimum recommended version is %02d.%02d.%d, current version is foo.bar.baz)",
-				driver.FullName(driver.Docker), minDockerVersion[0], minDockerVersion[1], minDockerVersion[2]),
+			version:           "linux-foo.bar.baz",
+			expect:            "",
+			expectFixContains: fmt.Sprintf(install, "foo.bar.baz"),
+		},
+		{
+			// "linux-18.09.9" is older than minimum recommended version
+			version:           "linux-18.09.9",
+			expect:            "",
+			expectFixContains: fmt.Sprintf(update, "18.09.9"),
+		},
+		{
+			// "linux-18.06.2" is older than minimum required version
+			version:           "linux-18.06.2",
+			expect:            "PROVIDER_DOCKER_VERSION_LOW",
+			expectFixContains: fmt.Sprintf(update, "18.06.2"),
 		},
 	}...)
 

diff --git a/site/content/en/docs/drivers/docker.md b/site/content/en/docs/drivers/docker.md
@@ -13,7 +13,7 @@ The Docker driver allows you to install Kubernetes into an existing Docker insta
 {{% tab "Standard Docker" %}}
 ## Requirements
 
-- [Install Docker](https://hub.docker.com/search?q=&type=edition&offering=community&sort=updated_at&order=desc) 18.09 or higher
+- [Install Docker](https://hub.docker.com/search?q=&type=edition&offering=community&sort=updated_at&order=desc) 18.09 or higher (20.10 or higher is recommended)
 - amd64 or arm64 system.
 - If using WSL complete [these steps]({{<ref "/docs/tutorials/wsl_docker_driver">}}) first