Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/ghodss/yaml v1.0.0
github.com/google/go-cmp v0.5.6
github.com/google/uuid v1.2.0
github.com/openshift/api v0.0.0-20220919112502-5eaf4250c423
github.com/openshift/api v0.0.0-20221207195358-a671aa80d995
github.com/openshift/client-go v0.0.0-20220915152853-9dfefb19db2e
github.com/openshift/library-go v0.0.0-20221012165547-f859132ee700
github.com/operator-framework/api v0.17.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE=
github.com/onsi/ginkgo/v2 v2.1.4 h1:GNapqRSid3zijZ9H77KrgVG4/8KqiyRsxcSxe+7ApXY=
github.com/onsi/gomega v1.19.0 h1:4ieX6qQjPP/BfC3mpsAtIGGlxTWPeA3Inl/7DtXw1tw=
github.com/openshift/api v0.0.0-20220919112502-5eaf4250c423 h1:6fJUUaZPDaTFCFLZ8ZgdaVaclzmerN6lb9ya17O3GGo=
github.com/openshift/api v0.0.0-20220919112502-5eaf4250c423/go.mod h1:HJAEIh4gLXPDdWxgCbvmJjzd9QIxyPZJtPU0u2W4vH4=
github.com/openshift/api v0.0.0-20221207195358-a671aa80d995 h1:pPxxrUWeT7mB/ujSfQ7jkfdjFCgjcnsP/uTRU5FcCpk=
github.com/openshift/api v0.0.0-20221207195358-a671aa80d995/go.mod h1:OW9hi5XDXOQWm/kRqUww6RVxZSf0nqrS4heerSmHBC4=
github.com/openshift/client-go v0.0.0-20220915152853-9dfefb19db2e h1:ab+BJg7h50pi2/rbMkDSXfYx8w80HLmr7NBs8H1hEvU=
github.com/openshift/client-go v0.0.0-20220915152853-9dfefb19db2e/go.mod h1:e+TTiBDGWB3O3p3iAzl054x3cZDWhrZ5+jxJRCdEFkA=
github.com/openshift/library-go v0.0.0-20221012165547-f859132ee700 h1:oMpH9KrXW4taP3OGgofy7tLOHc6I14B5uplnqJ6vr8c=
Expand Down
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 ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could probably put the "default to current arch" fallback in here, although I don't think it matters much if you'd rather keep that defaulting up in the caller level.

}

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.
Loading