Skip to content

Commit

Permalink
Use prerelease info when looking up conformance image (#978)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
johnSchnake authored Oct 25, 2019
1 parent f463b6d commit 871ba62
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 24 deletions.
13 changes: 8 additions & 5 deletions pkg/image/imageversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

version "github.com/hashicorp/go-version"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"k8s.io/client-go/discovery"
)

Expand Down Expand Up @@ -104,17 +103,21 @@ 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
}

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")
}
}
Expand Down
21 changes: 2 additions & 19 deletions pkg/image/imageversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -149,7 +144,6 @@ func TestGetConformanceImageVersion(t *testing.T) {
serverVersion discovery.ServerVersionInterface
expected string
error bool
warning bool
}{
{
name: "auto retrieves server version",
Expand All @@ -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",
Expand Down Expand Up @@ -216,23 +209,13 @@ 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")
} else if !test.error && err != nil {
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)
}
Expand Down

0 comments on commit 871ba62

Please sign in to comment.