fix server version check for operator (SeldonIO#3570)

* fix server version check for operator

* Add missing utils files

* default to 1.18 if minor version unknown

operator/utils/k8s/crd.go

@@ -102,11 +102,11 @@ func (cc *CrdCreator) findOrCreateCRDV1(rawYaml []byte) (*extensionsv1.CustomRes
 }
 
 func (cc *CrdCreator) findOrCreateCRD(rawYamlv1 []byte, rawYamlv1beta1 []byte) (v1.Object, error) {
-	sv, err := cc.discoveryClient.ServerVersion()
+	serverVersion, err := GetServerVersion(cc.discoveryClient, cc.logger)
 	if err != nil {
 		return nil, err
 	}
-	v, err := semver.NewVersion(sv.String())
+	v, err := semver.NewVersion(serverVersion)
 	if err != nil {
 		return nil, err
 	}
@@ -116,10 +116,10 @@ func (cc *CrdCreator) findOrCreateCRD(rawYamlv1 []byte, rawYamlv1beta1 []byte) (
 	}
 	check := c.Check(v)
 	if check {
-		cc.logger.Info("Creating V1 CRD for K8s", "version", sv.String())
+		cc.logger.Info("Creating V1 CRD for K8s", "version", serverVersion)
 		return cc.findOrCreateCRDV1(rawYamlv1)
 	} else {
-		cc.logger.Info("Creating V1Beta1 CRD for K8s", "version", sv.String())
+		cc.logger.Info("Creating V1Beta1 CRD for K8s", "version", serverVersion)
 		return cc.findOrCreateCRDV1beta1(rawYamlv1beta1)
 	}
 }

operator/utils/k8s/utils.go

@@ -0,0 +1,39 @@
+package k8s
+
+import (
+	"fmt"
+	"github.com/go-logr/logr"
+	"k8s.io/client-go/discovery"
+	"strconv"
+	"strings"
+)
+
+// This will cause CRD V1 to be installed
+const DefaultMinorVersion = 18
+
+func GetServerVersion(client discovery.DiscoveryInterface, logger logr.Logger) (string, error) {
+	serverVersion, err := client.ServerVersion()
+	if err != nil {
+		return "", err
+	}
+	logger.Info("Server version", "Major", serverVersion.Major, "Minor", serverVersion.Minor)
+	majorVersion, err := strconv.Atoi(serverVersion.Major)
+	if err != nil {
+		logger.Error(err, "Failed to parse majorVersion defaulting to 1")
+		majorVersion = 1
+	}
+	minorVersion, err := strconv.Atoi(serverVersion.Minor)
+	if err != nil {
+		if strings.HasSuffix(serverVersion.Minor, "+") {
+			minorVersion, err = strconv.Atoi(serverVersion.Minor[0 : len(serverVersion.Minor)-1])
+			if err != nil {
+				logger.Error(err, "Failed to parse minorVersion defaulting to 12")
+				minorVersion = DefaultMinorVersion
+			}
+		} else {
+			logger.Error(err, "Failed to parse minorVersion defaulting to 12")
+			minorVersion = DefaultMinorVersion
+		}
+	}
+	return fmt.Sprintf("%d.%d", majorVersion, minorVersion), nil
+}

operator/utils/k8s/utils_test.go

@@ -0,0 +1,36 @@
+package k8s
+
+import (
+	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/version"
+	discovery "k8s.io/client-go/discovery/fake"
+	coretesting "k8s.io/client-go/testing"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"testing"
+)
+
+func TestServerVersion(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	scenarios := map[string]struct {
+		major    string
+		minor    string
+		expected string
+	}{
+		"1.18 test":             {major: "1", minor: "18", expected: "1.18"},
+		"GKE 1.20":              {major: "1", minor: "20+", expected: "1.20"},
+		"Unknown minor version": {major: "1", minor: "XYZ", expected: "1.18"},
+	}
+
+	for _, scenario := range scenarios {
+		discoveryFake := &discovery.FakeDiscovery{Fake: &coretesting.Fake{},
+			FakedServerVersion: &version.Info{
+				Major: scenario.major,
+				Minor: scenario.minor,
+			}}
+		serverVersion, err := GetServerVersion(discoveryFake, ctrl.Log)
+		g.Expect(err).To(BeNil())
+		g.Expect(serverVersion).To(Equal(scenario.expected))
+	}
+
+}

operator/utils/k8s/webhook.go

@@ -12,52 +12,23 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"strconv"
-	"strings"
 )
 
 const MutatingWebhookName = "seldon-mutating-webhook-configuration"
 
 type WebhookCreator struct {
-	clientset    kubernetes.Interface
-	certs        *Cert
-	logger       logr.Logger
-	majorVersion int
-	minorVersion int
-	scheme       *runtime.Scheme
+	clientset kubernetes.Interface
+	certs     *Cert
+	logger    logr.Logger
+	scheme    *runtime.Scheme
 }
 
 func NewWebhookCreator(client kubernetes.Interface, certs *Cert, logger logr.Logger, scheme *runtime.Scheme) (*WebhookCreator, error) {
-	serverVersion, err := client.Discovery().ServerVersion()
-	if err != nil {
-		return nil, err
-	}
-	logger.Info("Server version", "Major", serverVersion.Major, "Minor", serverVersion.Minor)
-	majorVersion, err := strconv.Atoi(serverVersion.Major)
-	if err != nil {
-		logger.Error(err, "Failed to parse majorVersion defaulting to 1")
-		majorVersion = 1
-	}
-	minorVersion, err := strconv.Atoi(serverVersion.Minor)
-	if err != nil {
-		if strings.HasSuffix(serverVersion.Minor, "+") {
-			minorVersion, err = strconv.Atoi(serverVersion.Minor[0 : len(serverVersion.Minor)-1])
-			if err != nil {
-				logger.Error(err, "Failed to parse minorVersion defaulting to 12")
-				minorVersion = 12
-			}
-		} else {
-			logger.Error(err, "Failed to parse minorVersion defaulting to 12")
-			minorVersion = 12
-		}
-	}
 	return &WebhookCreator{
-		clientset:    client,
-		certs:        certs,
-		logger:       logger,
-		majorVersion: majorVersion,
-		minorVersion: minorVersion,
-		scheme:       scheme,
+		clientset: client,
+		certs:     certs,
+		logger:    logger,
+		scheme:    scheme,
 	}, nil
 }