Skip to content

Commit

Permalink
E2E Image ignores metadata and provider build info
Browse files Browse the repository at this point in the history 
Two builds with different metadata should refer to the same version.
As a result, we should just ignore the metadata in almost every case,
it is not relevent for choosing the e2e image.

Also, some API server versions reflect the cloud service (eks, gke,
etc). We need to continue to ignore those and choose just the basic
upstream image (e.g. 1.2.3-gke.2 should test against 1.2.3).

Fixes #1213
Fixes #1211

Signed-off-by: John Schnake <[email protected]>
  • Loading branch information
@johnSchnake
johnSchnake committed Apr 2, 2021
1 parent 04acfcb commit 649cfac
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 58 deletions.
58 changes: 31 additions & 27 deletions pkg/image/imageversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ func (c *ConformanceImageVersion) String() string { return string(*c) }
// Type needed for pflag.Value.
func (c *ConformanceImageVersion) Type() string { return "ConformanceImageVersion" }

// Set the ImageVersion to either the string "auto" or a version string
// Set the ImageVersion to either the string "auto" or a version string. The resulting version string
// will be forced into semver version with a 'v' prefix. You can set pre-release/metadata but it will
// fill in the minor/patch values as '0' if missing. E.g. 1+beta would yield v1.0.0+beta
func (c *ConformanceImageVersion) Set(str string) error {
switch str {
case ConformanceImageVersionAuto:
Expand Down Expand Up @@ -80,38 +82,40 @@ func (c *ConformanceImageVersion) Get(client discovery.ServerVersionInterface) (
return "", errors.Wrap(err, "couldn't retrieve server version")
}

parsedVersion, err := validateVersion(version.GitVersion)
if err != nil {
return "", err
}
return conformanceTagFromSemver(version.GitVersion)
}
return string(*c), nil
}

segments := parsedVersion.Segments()
if len(segments) < 2 {
return "", fmt.Errorf("version %q only has %d segments, need at least 2", version.GitVersion, len(segments))
}
// conformanceTagFromSemver uses the gitversion to choose the proper conformance image to use.
// Prereleases are considered, but metadata and provider-specific info is discarded.
func conformanceTagFromSemver(gitVersion string) (string, error) {
parsedVersion, err := validateVersion(gitVersion)
if err != nil {
return "", err
}

// Temporary logic in place to truncate auto-resolved versions while we
// transition to upstream. If < 1.14 return 2 segments due to lag behind
// releases. Otherwise return 3. Use the segments instead of .major and
// .minor because GKE's .minor is `10+` instead of `10`.
if segments[0] == 1 && segments[1] < 14 {
return fmt.Sprintf("v%d.%d", segments[0], segments[1]), nil
}
segments := parsedVersion.Segments()
if len(segments) < 2 {
return "", fmt.Errorf("version %q only has %d segments, need at least 2", gitVersion, len(segments))
}

// Not sure that this would be hit but default to adding the last
// segment as 0 per convention (upstream + semver).
if len(segments) < 3 {
return fmt.Sprintf("v%d.%d.%d", segments[0], segments[1], 0), nil
}
// Not sure that this would be hit but default to adding the last
// segment as 0 per convention (upstream + semver).
if len(segments) < 3 {
return fmt.Sprintf("v%d.%d.%d", segments[0], segments[1], 0), 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
}
// Upstream Kubernetes publishes the conformance images for prereleases as well; we should use them
// to ease testing new versions. Some vendors seem to put their name as prerelease instead of
// build metadata so handle on a case-by-case basis.
switch pr := parsedVersion.Prerelease(); {
case strings.HasPrefix(pr, "rc"),
strings.HasPrefix(pr, "alpha"),
strings.HasPrefix(pr, "beta"):
return fmt.Sprintf("v%d.%d.%d-%v", segments[0], segments[1], segments[2], parsedVersion.Prerelease()), nil
}
return string(*c), nil
return fmt.Sprintf("v%d.%d.%d", segments[0], segments[1], segments[2]), nil
}

func validateVersion(v string) (*version.Version, error) {
Expand Down
86 changes: 55 additions & 31 deletions pkg/image/imageversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,22 @@ func TestSetConformanceImageVersion(t *testing.T) {
error: true,
},
{
name: "version with addendum",
name: "version with prerelease and metadata",
version: "v1.13.0-beta.2.78+e0b33dbc2bde88",
expect: "v1.13.0-beta.2.78+e0b33dbc2bde88",
error: false,
},
{
name: "version with plus",
name: "version with empty metadata",
version: "v1.11+",
error: true,
},
{
name: "version without patch but with metadata",
version: "v1.11+vendor.1",
expect: "v1.11.0+vendor.1",
error: false,
},
}

for _, tc := range tests {
Expand Down Expand Up @@ -118,22 +124,6 @@ func TestGetConformanceImageVersion(t *testing.T) {
},
}

betaServerVersion := &fakeServerVersionInterface{
version: version.Info{
Major: "1",
Minor: "14",
GitVersion: "v1.14.1-beta.2.78+e0b33dbc2bde88",
},
}

gkeServerVersion := &fakeServerVersionInterface{
version: version.Info{
Major: "1",
Minor: "11+",
GitVersion: "v1.11.5-gke.3",
},
}

brokenServerVersion := &fakeServerVersionInterface{
err: errors.New("can't connect"),
}
Expand All @@ -157,18 +147,6 @@ func TestGetConformanceImageVersion(t *testing.T) {
serverVersion: brokenServerVersion,
error: true,
},
{
name: "prerelease info used without warning but metadata dropped",
version: "auto",
serverVersion: betaServerVersion,
expected: "v1.14.1-beta.2.78",
},
{
name: "gke server strips plus sign",
version: "auto",
serverVersion: gkeServerVersion,
expected: "v1.11",
},
{
name: "set version ignores server version",
version: "v1.11.2",
Expand Down Expand Up @@ -213,7 +191,7 @@ func TestGetConformanceImageVersion(t *testing.T) {
if test.error && err == nil {
t.Fatalf("expected error, got nil")
} else if !test.error && err != nil {
t.Fatalf("unexpecter error %v", err)
t.Fatalf("unexpected error %v", err)
}

if v != test.expected {
Expand All @@ -223,6 +201,52 @@ func TestGetConformanceImageVersion(t *testing.T) {
}
}

func TestConformanceTagFromSemver(t *testing.T) {
tcs := []struct {
desc string
input string
expected string
error bool
}{
{
desc: "Alpha releases supported",
input: "v1.14.1-alpha.2.78+e0b33dbc2bde88",
expected: "v1.14.1-alpha.2.78",
}, {
desc: "Beta releases supported",
input: "v1.14.1-beta.2.78+e0b33dbc2bde88",
expected: "v1.14.1-beta.2.78",
}, {
desc: "Release candidates supported",
input: "v1.14.1-rc.2.78+e0b33dbc2bde88",
expected: "v1.14.1-rc.2.78",
}, {
desc: "Misc release ignored",
input: "v1.14.1-34.2.78+e0b33dbc2bde88",
expected: "v1.14.1",
}, {
desc: "providers version ignored",
input: "v1.14.1-gke.2.78+e0b33dbc2bde88",
expected: "v1.14.1",
},
}

for _, tc := range tcs {
t.Run(tc.desc, func(t *testing.T) {
out, err := conformanceTagFromSemver(tc.input)
if tc.error && err == nil {
t.Fatalf("expected error, got nil")
} else if !tc.error && err != nil {
t.Fatalf("unexpected error %v", err)
}

if out != tc.expected {
t.Errorf("expected version %q, got %q", tc.expected, out)
}
})
}
}

// fakeServerVersionInterface is used as a test implementation as
// discovery.ServerVersionInterface.
type fakeServerVersionInterface struct {
Expand Down

0 comments on commit 649cfac

Please sign in to comment.