fix: minor version parsing in version compatibility check

Signed-off-by: Amir Alavi <[email protected]>

Author: a7i

pkg/descheduler/descheduler.go

@@ -18,9 +18,9 @@ package descheduler
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"math"
+	"strconv"
 	"time"
 
 	"go.opentelemetry.io/otel/attribute"
@@ -285,30 +285,30 @@ func Run(ctx context.Context, rs *options.DeschedulerServer) error {
 	return runFn()
 }
 
-func validateVersionCompatibility(discovery discovery.DiscoveryInterface, versionInfo version.Info) error {
-	serverVersionInfo, err := discovery.ServerVersion()
+func validateVersionCompatibility(discovery discovery.DiscoveryInterface, deschedulerVersionInfo version.Info) error {
+	kubeServerVersionInfo, err := discovery.ServerVersion()
 	if err != nil {
-		return errors.New("failed to discover Kubernetes server version")
+		return fmt.Errorf("failed to discover Kubernetes server version: %v", err)
 	}
 
-	serverVersion, err := utilversion.ParseSemantic(serverVersionInfo.String())
+	kubeServerVersion, err := utilversion.ParseSemantic(kubeServerVersionInfo.String())
 	if err != nil {
-		return errors.New("failed to parse Kubernetes server version")
+		return fmt.Errorf("failed to parse Kubernetes server version '%s': %v", kubeServerVersionInfo.String(), err)
 	}
 
-	deschedulerVersion, err := utilversion.ParseGeneric(versionInfo.GitVersion)
+	deschedulerMinor, err := strconv.ParseFloat(deschedulerVersionInfo.Minor, 64)
 	if err != nil {
-		return errors.New("failed to convert Descheduler minor version to float")
+		return fmt.Errorf("failed to convert Descheduler minor version '%s' to float: %v", deschedulerVersionInfo.Minor, err)
 	}
 
-	deschedulerMinor := float64(deschedulerVersion.Minor())
-	serverMinor := float64(serverVersion.Minor())
-	if math.Abs(deschedulerMinor-serverMinor) > 3 {
+	kubeServerMinor := float64(kubeServerVersion.Minor())
+	if math.Abs(deschedulerMinor-kubeServerMinor) > 3 {
 		return fmt.Errorf(
-			"descheduler version %v may not be supported on your version of Kubernetes %v."+
+			"descheduler version %s.%s may not be supported on your version of Kubernetes %v."+
 				"See compatibility docs for more info: https://github.com/kubernetes-sigs/descheduler#compatibility-matrix",
-			deschedulerVersion.String(),
-			serverVersionInfo.String(),
+			deschedulerVersionInfo.Major,
+			deschedulerVersionInfo.Minor,
+			kubeServerVersionInfo.String(),
 		)
 	}
 

pkg/descheduler/descheduler_test.go

@@ -300,44 +300,44 @@ func TestRootCancelWithNoInterval(t *testing.T) {
 func TestValidateVersionCompatibility(t *testing.T) {
 	type testCase struct {
 		name               string
-		deschedulerVersion string
+		deschedulerVersion deschedulerversion.Info
 		serverVersion      string
 		expectError        bool
 	}
 	testCases := []testCase{
 		{
 			name:               "no error when descheduler minor equals to server minor",
-			deschedulerVersion: "v0.26",
+			deschedulerVersion: deschedulerversion.Info{Major: "0", Minor: "26"},
 			serverVersion:      "v1.26.1",
 			expectError:        false,
 		},
 		{
 			name:               "no error when descheduler minor is 3 behind server minor",
-			deschedulerVersion: "0.23",
+			deschedulerVersion: deschedulerversion.Info{Major: "0", Minor: "23"},
 			serverVersion:      "v1.26.1",
 			expectError:        false,
 		},
 		{
 			name:               "no error when descheduler minor is 3 ahead of server minor",
-			deschedulerVersion: "v0.26",
+			deschedulerVersion: deschedulerversion.Info{Major: "0", Minor: "26"},
 			serverVersion:      "v1.26.1",
 			expectError:        false,
 		},
 		{
 			name:               "error when descheduler minor is 4 behind server minor",
-			deschedulerVersion: "v0.22",
+			deschedulerVersion: deschedulerversion.Info{Major: "0", Minor: "22"},
 			serverVersion:      "v1.26.1",
 			expectError:        true,
 		},
 		{
 			name:               "error when descheduler minor is 4 ahead of server minor",
-			deschedulerVersion: "v0.27",
+			deschedulerVersion: deschedulerversion.Info{Major: "0", Minor: "27"},
 			serverVersion:      "v1.23.1",
 			expectError:        true,
 		},
 		{
 			name:               "no error when using managed provider version",
-			deschedulerVersion: "v0.25",
+			deschedulerVersion: deschedulerversion.Info{Major: "0", Minor: "25"},
 			serverVersion:      "v1.25.12-eks-2d98532",
 			expectError:        false,
 		},
@@ -347,8 +347,7 @@ func TestValidateVersionCompatibility(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			fakeDiscovery.FakedServerVersion = &apiversion.Info{GitVersion: tc.serverVersion}
-			deschedulerVersion := deschedulerversion.Info{GitVersion: tc.deschedulerVersion}
-			err := validateVersionCompatibility(fakeDiscovery, deschedulerVersion)
+			err := validateVersionCompatibility(fakeDiscovery, tc.deschedulerVersion)
 
 			hasError := err != nil
 			if tc.expectError != hasError {

pkg/version/version_test.go

@@ -0,0 +1,54 @@
+package version
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+)
+
+func TestGet(t *testing.T) {
+	tests := []struct {
+		name    string
+		version string
+		want    Info
+	}{
+		{
+			name:    "parses automated container release tag",
+			version: "v20240519-v0.30.0",
+			want: Info{
+				Major:      "0",
+				Minor:      "30.0",
+				GitVersion: "v20240519-v0.30.0",
+			},
+		},
+		{
+			name:    "parses automated container build",
+			version: "v20240520-v0.30.0-5-g79990946",
+			want: Info{
+				Major:      "0",
+				Minor:      "30.0",
+				GitVersion: "v20240520-v0.30.0-5-g79990946",
+			},
+		},
+		{
+			name:    "parses helm release tag",
+			version: "v20240606-descheduler-helm-chart-0.30.0-18-g8714397b",
+			want: Info{
+				Major:      "0",
+				Minor:      "30.0",
+				GitVersion: "v20240606-descheduler-helm-chart-0.30.0-18-g8714397b",
+			},
+		},
+	}
+	ignoreRuntimeFields := cmpopts.IgnoreFields(Info{}, "GoVersion", "Compiler", "Platform")
+	for _, tt := range tests {
+		version = tt.version
+		t.Run(tt.name, func(t *testing.T) {
+			got := Get()
+			if diff := cmp.Diff(got, tt.want, ignoreRuntimeFields); diff != "" {
+				t.Errorf("Get (-want, +got):\n%s", diff)
+			}
+		})
+	}
+}