From c69a6db79472a5d2adcf5c976addddf903c63c66 Mon Sep 17 00:00:00 2001
From: John Schnake <jschnake@vmware.com>
Date: Fri, 25 Oct 2019 11:48:09 -0500
Subject: [PATCH] Use prerelease info when looking up conformance image

Upstream publishes prerelease images so there is no need to throw
that info away. It is a remnant of Heptio publishing their own
images.

In addition, if we're using that info we dont need to warn about
the existence of prerelease info and never really needed to warn
about the metadata on the version. As a result we can remove that
entire warning check/log.

Fixes #976
Fixes #977

Signed-off-by: John Schnake <jschnake@vmware.com>
---
 pkg/image/imageversion.go      | 13 ++++++++-----
 pkg/image/imageversion_test.go | 21 ++-------------------
 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/pkg/image/imageversion.go b/pkg/image/imageversion.go
index 469bf1c40..bf6462315 100644
--- a/pkg/image/imageversion.go
+++ b/pkg/image/imageversion.go
@@ -22,7 +22,6 @@ import (
 
 	version "github.com/hashicorp/go-version"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	"k8s.io/client-go/discovery"
 )
 
@@ -104,7 +103,13 @@ func (c *ConformanceImageVersion) Get(client discovery.ServerVersionInterface) (
 		if len(segments) < 3 {
 			return fmt.Sprintf("v%d.%d.%d", segments[0], segments[1], 0), nil
 		}
-		return fmt.Sprintf("v%d.%d.%d", segments[0], segments[1], segments[2]), nil
+
+		// Upstream Kubernetes publishes the conformance images for prereleases as well; we should use them
+		// to ease testing new versions.
+		if parsedVersion.Prerelease() == "" {
+			return fmt.Sprintf("v%d.%d.%d", segments[0], segments[1], segments[2]), nil
+		}
+		return fmt.Sprintf("v%d.%d.%d-%v", segments[0], segments[1], segments[2], parsedVersion.Prerelease()), nil
 	}
 	return string(*c), nil
 }
@@ -112,9 +117,7 @@ func (c *ConformanceImageVersion) Get(client discovery.ServerVersionInterface) (
 func validateVersion(v string) (*version.Version, error) {
 	version, err := version.NewVersion(v)
 	if err == nil {
-		if version.Metadata() != "" || version.Prerelease() != "" {
-			logrus.Warningf("Version %v is not a stable version, conformance image may not exist upstream", v)
-		} else if !strings.HasPrefix(v, "v") {
+		if !strings.HasPrefix(v, "v") {
 			err = errors.New("version must start with v")
 		}
 	}
diff --git a/pkg/image/imageversion_test.go b/pkg/image/imageversion_test.go
index a85322539..37bc1b14c 100644
--- a/pkg/image/imageversion_test.go
+++ b/pkg/image/imageversion_test.go
@@ -22,7 +22,6 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	testhook "github.com/sirupsen/logrus/hooks/test"
 	"k8s.io/apimachinery/pkg/version"
 	"k8s.io/client-go/discovery"
 )
@@ -111,10 +110,6 @@ func TestSetConformanceImageVersion(t *testing.T) {
 }
 
 func TestGetConformanceImageVersion(t *testing.T) {
-	testHook := &testhook.Hook{}
-	logrus.AddHook(testHook)
-	logrus.SetOutput(ioutil.Discard)
-
 	workingServerVersion := &fakeServerVersionInterface{
 		version: version.Info{
 			Major:      "1",
@@ -149,7 +144,6 @@ func TestGetConformanceImageVersion(t *testing.T) {
 		serverVersion discovery.ServerVersionInterface
 		expected      string
 		error         bool
-		warning       bool
 	}{
 		{
 			name:          "auto retrieves server version",
@@ -164,11 +158,10 @@ func TestGetConformanceImageVersion(t *testing.T) {
 			error:         true,
 		},
 		{
-			name:          "beta server version throws warning",
+			name:          "prerelease info used without warning but metadata dropped",
 			version:       "auto",
 			serverVersion: betaServerVersion,
-			warning:       true,
-			expected:      "v1.14.1",
+			expected:      "v1.14.1-beta.2.78",
 		},
 		{
 			name:          "gke server strips plus sign",
@@ -216,7 +209,6 @@ func TestGetConformanceImageVersion(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			testHook.Reset()
 			v, err := test.version.Get(test.serverVersion)
 			if test.error && err == nil {
 				t.Fatalf("expected error, got nil")
@@ -224,15 +216,6 @@ func TestGetConformanceImageVersion(t *testing.T) {
 				t.Fatalf("unexpecter error %v", err)
 			}
 
-			if test.warning {
-				last := testHook.LastEntry()
-				if last == nil {
-					t.Errorf("expected warning entry, got nothing")
-				} else if last.Level != logrus.WarnLevel {
-					t.Errorf("expected level %v, got %v", logrus.WarnLevel, last.Level)
-				}
-			}
-
 			if v != test.expected {
 				t.Errorf("expected version %q, got %q", test.expected, v)
 			}