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.

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

kubeadm: Extended KubeletVersionCheck #54868

Merged
merged 1 commit into from
Nov 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 15 additions & 3 deletions cmd/kubeadm/app/preflight/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,9 @@ func (kubever KubernetesVersionCheck) Check() (warnings, errors []error) {
}

// KubeletVersionCheck validates installed kubelet version
type KubeletVersionCheck struct{}
type KubeletVersionCheck struct {
KubernetesVersion string
}

// Check validates kubelet version. It should be not less than minimal supported version
func (kubever KubeletVersionCheck) Check() (warnings, errors []error) {
Expand All @@ -509,7 +511,17 @@ func (kubever KubeletVersionCheck) Check() (warnings, errors []error) {
if kubeletVersion.LessThan(kubeadmconstants.MinimumKubeletVersion) {
return nil, []error{fmt.Errorf("Kubelet version %q is lower than kubadm can support. Please upgrade kubelet", kubeletVersion)}
}
return nil, []error{}

if kubever.KubernetesVersion != "" {
k8sVersion, err := versionutil.ParseSemantic(kubever.KubernetesVersion)
if err != nil {
return nil, []error{fmt.Errorf("couldn't parse kubernetes version %q: %v", kubever.KubernetesVersion, err)}
}
if kubeletVersion.Major() > k8sVersion.Major() || kubeletVersion.Minor() > k8sVersion.Minor() {
return nil, []error{fmt.Errorf("the kubelet version is higher than the control plane version. This is not a supported version skew and may lead to a malfunctional cluster. Kubelet version: %q Control plane version: %q", kubeletVersion, k8sVersion)}
}
}
return nil, nil
}

// SwapCheck warns if swap is enabled
Expand Down Expand Up @@ -688,7 +700,7 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
SystemVerificationCheck{},
IsPrivilegedUserCheck{},
HostnameCheck{nodeName: cfg.NodeName},
KubeletVersionCheck{},
KubeletVersionCheck{KubernetesVersion: cfg.KubernetesVersion},
ServiceCheck{Service: "kubelet", CheckIfActive: false},
ServiceCheck{Service: "docker", CheckIfActive: true},
FirewalldCheck{ports: []int{int(cfg.API.BindPort), 10250}},
Expand Down
57 changes: 57 additions & 0 deletions cmd/kubeadm/app/preflight/checks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"fmt"
"io/ioutil"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -556,3 +557,59 @@ func restoreEnv(e map[string]string) {
os.Setenv(k, v)
}
}

func TestKubeletVersionCheck(t *testing.T) {
type T struct {
kubeletVersion string
k8sVersion string
expectErrors bool
expectWarnings bool
}

cases := []T{
{"v1.10.2", "", false, false}, // check minimally supported version when there is no information about control plane
{"v1.7.3", "v1.7.8", true, false}, // too old kubelet (older than kubeadmconstants.MinimumKubeletVersion), should fail.
{"v1.9.0", "v1.9.5", false, false}, // kubelet within same major.minor as control plane
{"v1.9.5", "v1.9.1", false, false}, // kubelet is newer, but still within same major.minor as control plane
{"v1.9.0", "v1.10.1", false, false}, // kubelet is lower than control plane, but newer than minimally supported
{"v1.10.0-alpha.1", "v1.9.1", true, false}, // kubelet is newer (development build) than control plane, should fail.
{"v1.10.0", "v1.9.5", true, false}, // kubelet is newer (release) than control plane, should fail.
}

dir, err := ioutil.TempDir("", "test-kubelet-version-check")
if err != nil {
t.Errorf("Failed to create directory for testing GetKubeletVersion: %v", err)
}
defer os.RemoveAll(dir)

// We don't want to call real kubelet or something else in $PATH
oldPATH := os.Getenv("PATH")
Copy link
Member

Choose a reason for hiding this comment

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

Can you please re-engineer this to use k8s.io/utils/exec for this instead so you can fake out the exec calls in unit tests instead of doing this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't noticed that lib. Will check it. From quick look, it might require change also in implementation of GetKubeletVersion(). Do you want to have it as part of this PR or maybe let's do re-factoring to use utils/exec as separate PR ?

Copy link
Member

Choose a reason for hiding this comment

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

Follow up is fine by me

defer os.Setenv("PATH", oldPATH)

os.Setenv("PATH", dir)

kubeletFn := filepath.Join(dir, "kubelet")
for _, tc := range cases {

content := []byte(fmt.Sprintf("#!/bin/sh\necho 'Kubernetes %s'", tc.kubeletVersion))
if err := ioutil.WriteFile(kubeletFn, content, 0755); err != nil {
t.Errorf("Error creating test stub file %s: %v", kubeletFn, err)
}

check := KubeletVersionCheck{KubernetesVersion: tc.k8sVersion}
warnings, errors := check.Check()

switch {
case warnings != nil && !tc.expectWarnings:
t.Errorf("KubeletVersionCheck: unexpected warnings for kubelet version %q and kubernetes version %q. Warnings: %v", tc.kubeletVersion, tc.k8sVersion, warnings)
case warnings == nil && tc.expectWarnings:
t.Errorf("KubeletVersionCheck: expected warnings for kubelet version %q and kubernetes version %q but got nothing", tc.kubeletVersion, tc.k8sVersion)
case errors != nil && !tc.expectErrors:
t.Errorf("KubeletVersionCheck: unexpected errors for kubelet version %q and kubernetes version %q. errors: %v", tc.kubeletVersion, tc.k8sVersion, errors)
case errors == nil && tc.expectErrors:
t.Errorf("KubeletVersionCheck: expected errors for kubelet version %q and kubernetes version %q but got nothing", tc.kubeletVersion, tc.k8sVersion)
}

}

}