From 1f6a736aab7f101c891726cd4bd2ace8daad643b Mon Sep 17 00:00:00 2001 From: Amir Alavi Date: Wed, 22 Mar 2023 11:46:58 -0400 Subject: [PATCH] add unit tests for version compatibility check --- pkg/descheduler/descheduler.go | 31 ++++++++++----- pkg/descheduler/descheduler_test.go | 58 +++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/pkg/descheduler/descheduler.go b/pkg/descheduler/descheduler.go index 783497faea..91c2e0cdd2 100644 --- a/pkg/descheduler/descheduler.go +++ b/pkg/descheduler/descheduler.go @@ -18,12 +18,14 @@ package descheduler import ( "context" + "errors" "fmt" "math" "strconv" "strings" "time" + "k8s.io/client-go/discovery" componentbaseconfig "k8s.io/component-base/config" "k8s.io/klog/v2" @@ -76,7 +78,9 @@ func Run(ctx context.Context, rs *options.DeschedulerServer) error { } // Add k8s compatibility warnings to logs - versionCompatibilityCheck(rs) + if err := validateVersionCompatibility(rs.Client.Discovery(), version.Get()); err != nil { + klog.Warning(err.Error()) + } evictionPolicyGroupVersion, err := eutils.SupportEviction(rs.Client) if err != nil || len(evictionPolicyGroupVersion) == 0 { @@ -92,7 +96,7 @@ func Run(ctx context.Context, rs *options.DeschedulerServer) error { } if rs.LeaderElection.LeaderElect && rs.DryRun { - klog.V(1).InfoS("Warning: DryRun is set to True. You need to disable it to use Leader Election.") + klog.V(1).Info("Warning: DryRun is set to True. You need to disable it to use Leader Election.") } if rs.LeaderElection.LeaderElect && !rs.DryRun { @@ -105,27 +109,34 @@ func Run(ctx context.Context, rs *options.DeschedulerServer) error { return runFn() } -func versionCompatibilityCheck(rs *options.DeschedulerServer) { - serverVersion, serverErr := rs.Client.Discovery().ServerVersion() +func validateVersionCompatibility(discovery discovery.DiscoveryInterface, versionInfo version.Info) error { + serverVersion, serverErr := discovery.ServerVersion() if serverErr != nil { - klog.V(1).InfoS("Warning: Get Kubernetes server version fail") - return + return errors.New("failed to get Kubernetes server version") } - deschedulerMinorVersion := strings.Split(version.Get().Minor, ".")[0] + deschedulerMinorVersion := strings.Split(versionInfo.Minor, ".")[0] deschedulerMinorVersionFloat, err := strconv.ParseFloat(deschedulerMinorVersion, 64) if err != nil { - klog.Warning("Warning: Convert Descheduler minor version to float fail") + return errors.New("failed to convert Descheduler minor version to float") } kubernetesMinorVersionFloat, err := strconv.ParseFloat(serverVersion.Minor, 64) if err != nil { - klog.Warning("Warning: Convert Kubernetes server minor version to float fail") + return errors.New("failed to convert Kubernetes server minor version to float") } if math.Abs(deschedulerMinorVersionFloat-kubernetesMinorVersionFloat) > 3 { - klog.Warningf("Warning: Descheduler minor version %v is not supported on your version of Kubernetes %v.%v. See compatibility docs for more info: https://github.com/kubernetes-sigs/descheduler#compatibility-matrix", deschedulerMinorVersion, serverVersion.Major, serverVersion.Minor) + return fmt.Errorf( + "descheduler minor version %v is not supported on your version of Kubernetes %v.%v. "+ + "See compatibility docs for more info: https://github.com/kubernetes-sigs/descheduler#compatibility-matrix", + deschedulerMinorVersion, + serverVersion.Major, + serverVersion.Minor, + ) } + + return nil } func cachedClient( diff --git a/pkg/descheduler/descheduler_test.go b/pkg/descheduler/descheduler_test.go index 89a8998129..afa0830362 100644 --- a/pkg/descheduler/descheduler_test.go +++ b/pkg/descheduler/descheduler_test.go @@ -11,6 +11,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/runtime" + apiversion "k8s.io/apimachinery/pkg/version" + fakediscovery "k8s.io/client-go/discovery/fake" fakeclientset "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" "sigs.k8s.io/descheduler/cmd/descheduler/app/options" @@ -20,6 +22,7 @@ import ( "sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor" "sigs.k8s.io/descheduler/pkg/framework/plugins/removeduplicates" "sigs.k8s.io/descheduler/pkg/framework/plugins/removepodsviolatingnodetaints" + deschedulerversion "sigs.k8s.io/descheduler/pkg/version" "sigs.k8s.io/descheduler/test" ) @@ -245,6 +248,61 @@ func TestRootCancelWithNoInterval(t *testing.T) { } } +func TestValidateVersionCompatibility(t *testing.T) { + type testCase struct { + name string + deschedulerMinor string + serverMinor string + expectError bool + } + testCases := []testCase{ + { + name: "no error when descheduler minor equals to server minor", + deschedulerMinor: "26.0", + serverMinor: "26", + expectError: false, + }, + { + name: "no error when descheduler minor is 3 behind server minor", + deschedulerMinor: "23.0", + serverMinor: "26", + expectError: false, + }, + { + name: "no error when descheduler minor is 3 ahead of server minor", + deschedulerMinor: "26.0", + serverMinor: "23", + expectError: false, + }, + { + name: "error when descheduler minor is 4 behind server minor", + deschedulerMinor: "22.0", + serverMinor: "26", + expectError: true, + }, + { + name: "error when descheduler minor is 4 ahead of server minor", + deschedulerMinor: "27.0", + serverMinor: "23", + expectError: true, + }, + } + client := fakeclientset.NewSimpleClientset() + fakeDiscovery, _ := client.Discovery().(*fakediscovery.FakeDiscovery) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fakeDiscovery.FakedServerVersion = &apiversion.Info{Major: "1", Minor: tc.serverMinor} + deschedulerVersion := deschedulerversion.Info{Major: "0", Minor: tc.deschedulerMinor} + err := validateVersionCompatibility(fakeDiscovery, deschedulerVersion) + + hasError := err != nil + if tc.expectError != hasError { + t.Error("unexpected version compatibility behavior") + } + }) + } +} + func podEvictionReactionFuc(evictedPods *[]string) func(action core.Action) (bool, runtime.Object, error) { return func(action core.Action) (bool, runtime.Object, error) { if action.GetSubresource() == "eviction" {