Skip to content

Commit

Permalink
Support ClusterVersion spec DesiredUpdate Architecture
Browse files Browse the repository at this point in the history
  • Loading branch information
jottofar committed Dec 12, 2022
1 parent 00997af commit 5730ebf
Show file tree
Hide file tree
Showing 14 changed files with 336 additions and 38 deletions.
13 changes: 8 additions & 5 deletions lib/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,20 @@ func ValidateClusterVersion(config *configv1.ClusterVersion) field.ErrorList {
}
if u := config.Spec.DesiredUpdate; u != nil {
switch {
case len(u.Version) == 0 && len(u.Image) == 0:
errs = append(errs, field.Required(field.NewPath("spec", "desiredUpdate", "version"), "must specify version or image"))
case len(u.Architecture) == 0 && len(u.Version) == 0 && len(u.Image) == 0:
errs = append(errs, field.Required(field.NewPath("spec", "desiredUpdate"), "must specify architecture, version, or image"))
case len(u.Version) > 0 && !validSemVer(u.Version):
errs = append(errs, field.Invalid(field.NewPath("spec", "desiredUpdate", "version"), u.Version, "must be a semantic version (1.2.3[-...])"))
errs = append(errs, field.Invalid(field.NewPath("spec", "desiredUpdate", "version"), u.Version,
"must be a semantic version (1.2.3[-...])"))
case len(u.Version) > 0 && len(u.Image) == 0:
switch countPayloadsForVersion(config, u.Version) {
case 0:
errs = append(errs, field.Invalid(field.NewPath("spec", "desiredUpdate", "version"), u.Version, "when image is empty the update must be a previous version or an available update"))
errs = append(errs, field.Invalid(field.NewPath("spec", "desiredUpdate", "version"), u.Version,
"when image is empty the update must be a previous version or an available update"))
case 1:
default:
errs = append(errs, field.Invalid(field.NewPath("spec", "desiredUpdate", "version"), u.Version, "there are multiple possible payloads for this version, specify the exact image"))
errs = append(errs, field.Invalid(field.NewPath("spec", "desiredUpdate", "version"), u.Version,
"there are multiple possible payloads for this version, specify the exact image"))
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/autoupdate/autoupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,9 @@ func (ctrl *Controller) sync(ctx context.Context, key string) error {
}
up := nextUpdate(clusterversion.Status.AvailableUpdates)
clusterversion.Spec.DesiredUpdate = &v1.Update{
Version: up.Version,
Image: up.Image,
Architecture: clusterversion.Spec.DesiredUpdate.Architecture,
Version: up.Version,
Image: up.Image,
}

_, updated, err := resourceapply.ApplyClusterVersionFromCache(ctx, ctrl.cvLister, ctrl.client.ConfigV1(), clusterversion)
Expand Down
29 changes: 23 additions & 6 deletions pkg/cincinnati/cincinnati.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,31 @@ func (err *Error) Error() string {
}

// GetUpdates fetches the current and next-applicable update payloads from the specified
// upstream Cincinnati stack given the current version and channel. The command:
// upstream Cincinnati stack given the current version, desired architecture, and channel.
// The command:
//
// 1. Downloads the update graph from the requested URI for the requested arch and channel.
// 1. Downloads the update graph from the requested URI for the requested desired arch and channel.
// 2. Finds the current version entry under .nodes.
// 3. Finds recommended next-hop updates by searching .edges for updates from the current
// 3. If a transition from single to multi architecture has been requested, the only valid
// version is the current version so it's returned.
// 4. Finds recommended next-hop updates by searching .edges for updates from the current
// version. Returns a slice of target Releases with these unconditional recommendations.
// 4. Finds conditionally recommended next-hop updates by searching .conditionalEdges for
// 5. Finds conditionally recommended next-hop updates by searching .conditionalEdges for
// updates from the current version. Returns a slice of ConditionalUpdates with these
// conditional recommendations.
func (c Client) GetUpdates(ctx context.Context, uri *url.URL, arch string, channel string, version semver.Version) (configv1.Release, []configv1.Release, []configv1.ConditionalUpdate, error) {
func (c Client) GetUpdates(ctx context.Context, uri *url.URL, desiredArch, currentArch, channel string,
version semver.Version) (configv1.Release, []configv1.Release, []configv1.ConditionalUpdate, error) {

var current configv1.Release

releaseArch := desiredArch
if desiredArch == string(configv1.ClusterVersionArchitectureMulti) {
releaseArch = "multi"
}

// Prepare parametrized cincinnati query.
queryParams := uri.Query()
queryParams.Add("arch", arch)
queryParams.Add("arch", releaseArch)
queryParams.Add("channel", channel)
queryParams.Add("id", c.id.String())
queryParams.Add("version", version.String())
Expand Down Expand Up @@ -138,6 +149,12 @@ func (c Client) GetUpdates(ctx context.Context, uri *url.URL, arch string, chann
Message: fmt.Sprintf("invalid current node: %s", err),
}
}

// Migrating from single to multi architecture. Only valid update for required heterogeneous graph
// is heterogeneous version of current version.
if desiredArch == string(configv1.ClusterVersionArchitectureMulti) && currentArch != desiredArch {
return current, []configv1.Release{current}, nil, nil
}
break
}
}
Expand Down
155 changes: 151 additions & 4 deletions pkg/cincinnati/cincinnati_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ import (

func TestGetUpdates(t *testing.T) {
clientID := uuid.Must(uuid.Parse("01234567-0123-0123-0123-0123456789ab"))
arch := "test-arch"
channelName := "test-channel"
tests := []struct {
name string
version string
name string
version string
arch string
currentArch string

expectedQuery string
graph string
Expand Down Expand Up @@ -171,6 +172,146 @@ func TestGetUpdates(t *testing.T) {
URL: "https://example.com/errata/4.1.0-0.okd-0",
Channels: []string{"channel-a", "test-channel"},
},
}, {
name: "single to multi arch, only current version available",
version: "4.1.2",
arch: "Multi",
currentArch: "test-arch",
graph: `{
"nodes": [
{
"version": "4.1.0",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.0",
"metadata": {
"url": "https://example.com/errata/4.1.0",
"io.openshift.upgrades.graph.release.channels": "test-channel"
}
},
{
"version": "4.1.1",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.1",
"metadata": {
"url": "https://example.com/errata/4.1.1",
"io.openshift.upgrades.graph.release.channels": "test-channel"
}
},
{
"version": "4.1.2",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.2",
"metadata": {
"url": "https://example.com/errata/4.1.2",
"io.openshift.upgrades.graph.release.channels": "test-channel"
}
},
{
"version": "4.1.3",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.3",
"metadata": {
"url": "https://example.com/errata/4.1.3",
"io.openshift.upgrades.graph.release.channels": "test-channel"
}
}
],
"edges": [[0,1]]
}`,
expectedQuery: "arch=multi&channel=test-channel&id=01234567-0123-0123-0123-0123456789ab&version=4.1.2",
current: configv1.Release{
Version: "4.1.2",
Image: "quay.io/openshift-release-dev/ocp-release:4.1.2",
URL: "https://example.com/errata/4.1.2",
Channels: []string{"test-channel"},
},
available: []configv1.Release{
{
Version: "4.1.2",
Image: "quay.io/openshift-release-dev/ocp-release:4.1.2",
URL: "https://example.com/errata/4.1.2",
Channels: []string{"test-channel"},
},
},
}, {
name: "single to multi arch, current version not found",
version: "4.1.2",
arch: "Multi",
currentArch: "test-arch",
graph: `{
"nodes": [
{
"version": "4.1.0",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.0",
"metadata": {
"url": "https://example.com/errata/4.1.0",
"io.openshift.upgrades.graph.release.channels": "test-channel"
}
},
{
"version": "4.1.1",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.1",
"metadata": {
"url": "https://example.com/errata/4.1.1",
"io.openshift.upgrades.graph.release.channels": "test-channel"
}
},
{
"version": "4.1.3",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.3",
"metadata": {
"url": "https://example.com/errata/4.1.3",
"io.openshift.upgrades.graph.release.channels": "test-channel"
}
}
],
"edges": [[0,1]]
}`,
expectedQuery: "arch=multi&channel=test-channel&id=01234567-0123-0123-0123-0123456789ab&version=4.1.2",
current: configv1.Release{
Version: "4.1.2",
Image: "quay.io/openshift-release-dev/ocp-release:4.1.2",
URL: "https://example.com/errata/4.1.2",
Channels: []string{"test-channel"},
},
err: "VersionNotFound: currently reconciling cluster version 4.1.2 not found in the \"test-channel\" channel",
}, {
name: "multi to multi arch, one update available",
version: "4.1.0",
arch: "Multi",
currentArch: "Multi",
graph: `{
"nodes": [
{
"version": "4.1.0",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.0",
"metadata": {
"url": "https://example.com/errata/4.1.0",
"io.openshift.upgrades.graph.release.channels": "test-channel,channel-a"
}
},
{
"version": "4.1.1",
"payload": "quay.io/openshift-release-dev/ocp-release:4.1.1",
"metadata": {
"url": "https://example.com/errata/4.1.1",
"io.openshift.upgrades.graph.release.channels": "test-channel"
}
}
],
"edges": [[0,1]]
}`,
expectedQuery: "arch=multi&channel=test-channel&id=01234567-0123-0123-0123-0123456789ab&version=4.1.0",
current: configv1.Release{
Version: "4.1.0",
Image: "quay.io/openshift-release-dev/ocp-release:4.1.0",
URL: "https://example.com/errata/4.1.0",
Channels: []string{"channel-a", "test-channel"},
},
available: []configv1.Release{
{
Version: "4.1.1",
Image: "quay.io/openshift-release-dev/ocp-release:4.1.1",
URL: "https://example.com/errata/4.1.1",
Channels: []string{"test-channel"},
},
},
}, {
name: "conditional updates available",
version: "4.1.0",
Expand Down Expand Up @@ -611,7 +752,13 @@ func TestGetUpdates(t *testing.T) {
t.Fatal(err)
}

current, updates, conditionalUpdates, err := c.GetUpdates(context.Background(), uri, arch, channelName, semver.MustParse(test.version))
if test.arch == "" {
test.arch = "test-arch"
}
if test.currentArch == "" {
test.currentArch = test.arch
}
current, updates, conditionalUpdates, err := c.GetUpdates(context.Background(), uri, test.arch, test.currentArch, channelName, semver.MustParse(test.version))
if test.err == "" {
if err != nil {
t.Fatalf("expected nil error, got: %v", err)
Expand Down
36 changes: 27 additions & 9 deletions pkg/cvo/availableupdates.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
}

channel := config.Spec.Channel
arch := optr.getArchitecture()
desiredArch := optr.getDesiredArchitecture(config.Spec.DesiredUpdate)
currentArch := optr.getArchitecture()

if desiredArch == "" {
desiredArch = currentArch
}

// updates are only checked at most once per minimumUpdateCheckInterval or if the generation changes
u := optr.getAvailableUpdates()
Expand All @@ -47,9 +52,9 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
klog.V(2).Infof("Retrieving available updates again, because more than %s has elapsed since %s", optr.minimumUpdateCheckInterval, u.LastAttempt)
} else if channel != u.Channel {
klog.V(2).Infof("Retrieving available updates again, because the channel has changed from %q to %q", u.Channel, channel)
} else if arch != u.Architecture {
} else if desiredArch != u.Architecture {
klog.V(2).Infof("Retrieving available updates again, because the architecture has changed from %q to %q",
u.Architecture, arch)
u.Architecture, desiredArch)
} else if upstream == u.Upstream || (upstream == optr.defaultUpstreamServer && u.Upstream == "") {
klog.V(2).Infof("Available updates were recently retrieved, with less than %s elapsed since %s, will try later.", optr.minimumUpdateCheckInterval, u.LastAttempt)
return nil
Expand All @@ -62,7 +67,8 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
return err
}

current, updates, conditionalUpdates, condition := calculateAvailableUpdatesStatus(ctx, string(config.Spec.ClusterID), transport, upstream, arch, channel, optr.release.Version)
current, updates, conditionalUpdates, condition := calculateAvailableUpdatesStatus(ctx, string(config.Spec.ClusterID),
transport, upstream, desiredArch, currentArch, channel, optr.release.Version)

if usedDefaultUpstream {
upstream = ""
Expand All @@ -71,7 +77,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
au := &availableUpdates{
Upstream: upstream,
Channel: config.Spec.Channel,
Architecture: arch,
Architecture: desiredArch,
Current: current,
Updates: updates,
ConditionalUpdates: conditionalUpdates,
Expand Down Expand Up @@ -198,7 +204,17 @@ func (optr *Operator) SetArchitecture(architecture string) {
optr.architecture = architecture
}

func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, transport *http.Transport, upstream, arch, channel, version string) (configv1.Release, []configv1.Release, []configv1.ConditionalUpdate, configv1.ClusterOperatorStatusCondition) {
func (optr *Operator) getDesiredArchitecture(update *configv1.Update) string {
if update != nil {
return string(update.Architecture)
}
return ""
}

func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, transport *http.Transport, upstream, desiredArch,
currentArch, channel, version string) (configv1.Release, []configv1.Release, []configv1.ConditionalUpdate,
configv1.ClusterOperatorStatusCondition) {

var cvoCurrent configv1.Release
if len(upstream) == 0 {
return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{
Expand All @@ -223,10 +239,10 @@ func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, tran
}
}

if len(arch) == 0 {
if len(desiredArch) == 0 {
return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{
Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: noArchitecture,
Message: "The set of architectures has not been configured.",
Message: "Architecture has not been configured.",
}
}

Expand All @@ -253,7 +269,9 @@ func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, tran
}
}

current, updates, conditionalUpdates, err := cincinnati.NewClient(uuid, transport).GetUpdates(ctx, upstreamURI, arch, channel, currentVersion)
current, updates, conditionalUpdates, err := cincinnati.NewClient(uuid, transport).GetUpdates(ctx, upstreamURI, desiredArch,
currentArch, channel, currentVersion)

if err != nil {
klog.V(2).Infof("Upstream server %s could not return available updates: %v", upstream, err)
if updateError, ok := err.(*cincinnati.Error); ok {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cvo/cvo.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ func (optr *Operator) sync(ctx context.Context, key string) error {
config := validation.ClearInvalidFields(original, errs)

// identify the desired next version
desired, ok := findUpdateFromConfig(config)
desired, ok := findUpdateFromConfig(config, optr.getArchitecture())
if ok {
klog.V(2).Infof("Desired version from spec is %#v", desired)
} else {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cvo/cvo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2397,7 +2397,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) {
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionFalse,
Reason: noArchitecture,
Message: "The set of architectures has not been configured.",
Message: "Architecture has not been configured.",
},
},
},
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"kind": "Test",
"apiVersion": "v1",
"metadata": {
"name": "file",
"annotations": {
"include.release.openshift.io/self-managed-high-availability": "true"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
kind: ImageStream
apiVersion: image.openshift.io/v1
metadata:
name: 1.0.0-abc
Loading

0 comments on commit 5730ebf

Please sign in to comment.