From b4abe48985a7631a65ea76c1a05210b930d95698 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 13 Feb 2024 23:28:43 -0800 Subject: [PATCH] *: Add --update-service command-line option [1] includes [2] pitching local update services in managed regions. However, HyperShift currently forces ClusterVersion spec.upstream empty [3]. This commit is part of making the upstream update service configurable on HyperShift, so those clusters can hear about and assess known issues with updates in environments where the default https://api.openshift.com/api/upgrades_info update service is not accessible, or in which an alternative update service was desired for testing or policy reasons. This includes [2], mentioned above, but would also include any other instance of disconnected/restricted-network use. The alternative for folks who want HyperShift updates in places where api.openshift.com is inaccessible are: * Don't use an update service and manage that aspect manually. But the update service declares multiple new releases each week, as well as delivering information about known risks/issues for local clusters to evalute their exposure. That's a lot of information to manage manually, if folks decide not to plug into the existing update service tooling. * Run a local update service, and fiddle with DNS and X.509 certs so that packets aimed at api.openshift.com get routed to your local service . This one requires less long-term effort than manually replacing the entire update service system, but it requires your clusters to trust an X.509 Certificate Authority that is willing to sign certificates for your local service saying "yup, that one is definitely api.openshift.com". One possible data path would be: 1. HostedCluster (management cluster). 2. HostedControlPlane (management cluster). 3. ClusterVersion spec.upstream (hosted API). 4. Cluster-version operator container (managment cluster). that pathway would only require changes to the HyperShift repo. But to avoid URI passing through the customer-accessible hosted API, this commit adds a new --update-service command-line option to support: 1. HostedCluster (management cluster). 2. HostedControlPlane (management cluster). 3. Cluster-version operator Deployment --update-service command line option (management cluster). 4. Cluster-version operator container (managment cluster). If, in the future, we grow an option to give the hosted CVO kubeconfig access to both the management and hosted Kubernetes APIs, we could drop --update-service and have the hosted CVO reach out and read this configuration off HostedControlPlane or HostedCluster directly. I'd initially gone with --upstream, but moved to --update-service and updateService, etc. based on Petr's reasonable point that "upstream" is pretty generic, "update service" is not much longer and is much more specific, and diverging from the ClusterVersion spec.upstream precedent isn't that terrible [4]. [1]: https://issues.redhat.com/browse/XCMSTRAT-513 [2]: https://issues.redhat.com/browse/OTA-1185 [3]: https://github.com/openshift/hypershift/blob/5e50e633fefd88aab9588d660c4b5daddd950d9a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go#L1059 [4]: https://github.com/openshift/cluster-version-operator/pull/1035#pullrequestreview-1911703116 --- cmd/start.go | 1 + pkg/cvo/availableupdates.go | 61 +++++---- pkg/cvo/availableupdates_test.go | 3 +- pkg/cvo/cvo.go | 14 ++- pkg/cvo/cvo_test.go | 204 ++++++++++++++++++++----------- pkg/cvo/metrics.go | 4 +- pkg/start/start.go | 6 + 7 files changed, 186 insertions(+), 107 deletions(-) diff --git a/cmd/start.go b/cmd/start.go index 906c9dc323..a929480708 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -46,5 +46,6 @@ func init() { cmd.PersistentFlags().BoolVar(&opts.PromQLTarget.UseDNS, "use-dns-for-services", opts.PromQLTarget.UseDNS, "Configures the CVO to use DNS for resolution of services in the cluster.") cmd.PersistentFlags().StringVar(&opts.PrometheusURLString, "metrics-url", opts.PrometheusURLString, "The URL used to access the remote PromQL query service.") cmd.PersistentFlags().BoolVar(&opts.InjectClusterIdIntoPromQL, "hypershift", opts.InjectClusterIdIntoPromQL, "This options indicates whether the CVO is running inside a hosted control plane.") + cmd.PersistentFlags().StringVar(&opts.UpdateService, "update-service", opts.UpdateService, "The preferred update service. If set, this option overrides any upstream value configured in ClusterVersion spec.") rootCmd.AddCommand(cmd) } diff --git a/pkg/cvo/availableupdates.go b/pkg/cvo/availableupdates.go index da9ad2b41e..5673c66306 100644 --- a/pkg/cvo/availableupdates.go +++ b/pkg/cvo/availableupdates.go @@ -25,16 +25,25 @@ import ( const noArchitecture string = "NoArchitecture" const noChannel string = "NoChannel" +const defaultUpdateService string = "https://api.openshift.com/api/upgrades_info/v1/graph" // syncAvailableUpdates attempts to retrieve the latest updates and update the status of the ClusterVersion // object. It will set the RetrievedUpdates condition. Updates are only checked if it has been more than // the minimumUpdateCheckInterval since the last check. func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1.ClusterVersion) error { - usedDefaultUpstream := false - upstream := string(config.Spec.Upstream) - if len(upstream) == 0 { - usedDefaultUpstream = true - upstream = optr.defaultUpstreamServer + usedDefaultUpdateService := false + + updateService := optr.updateService + var updateServiceSource string + if len(updateService) > 0 { + updateServiceSource = "the --update-service command line option" + } else if len(config.Spec.Upstream) > 0 { + updateService = string(config.Spec.Upstream) + updateServiceSource = "ClusterVersion spec.upstream" + } else { + usedDefaultUpdateService = true + updateService = defaultUpdateService + updateServiceSource = "the operator's default update service" } channel := config.Spec.Channel @@ -63,7 +72,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1 klog.V(2).Infof("Retrieving available updates again, because the channel has changed from %q to %q", optrAvailableUpdates.Channel, channel) } else if desiredArch != optrAvailableUpdates.Architecture { klog.V(2).Infof("Retrieving available updates again, because the architecture has changed from %q to %q", optrAvailableUpdates.Architecture, desiredArch) - } else if upstream == optrAvailableUpdates.Upstream || (upstream == optr.defaultUpstreamServer && optrAvailableUpdates.Upstream == "") { + } else if updateService == optrAvailableUpdates.UpdateService || (updateService == defaultUpdateService && optrAvailableUpdates.UpdateService == "") { needsConditionalUpdateEval := false for _, conditionalUpdate := range optrAvailableUpdates.ConditionalUpdates { if recommended := findRecommendedCondition(conditionalUpdate.Conditions); recommended == nil { @@ -80,7 +89,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1 } needFreshFetch = false } else { - klog.V(2).Infof("Retrieving available updates again, because the upstream has changed from %q to %q", optrAvailableUpdates.Upstream, config.Spec.Upstream) + klog.V(2).Infof("Retrieving available updates again, because the update service has changed from %q to %q from %s", optrAvailableUpdates.UpdateService, updateService, updateServiceSource) } if needFreshFetch { @@ -93,7 +102,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1 clusterId := string(config.Spec.ClusterID) current, updates, conditionalUpdates, condition := calculateAvailableUpdatesStatus(ctx, clusterId, - transport, userAgent, upstream, desiredArch, currentArch, channel, optr.release.Version, optr.conditionRegistry) + transport, userAgent, updateService, desiredArch, currentArch, channel, optr.release.Version, optr.conditionRegistry) // Populate conditions on conditional updates from operator state for i := range optrAvailableUpdates.ConditionalUpdates { @@ -109,12 +118,12 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1 conditionalUpdates = injectClusterIdIntoConditionalUpdates(clusterId, conditionalUpdates) } - if usedDefaultUpstream { - upstream = "" + if usedDefaultUpdateService { + updateService = "" } optrAvailableUpdates.LastAttempt = time.Now() - optrAvailableUpdates.Upstream = upstream + optrAvailableUpdates.UpdateService = updateService optrAvailableUpdates.Channel = channel optrAvailableUpdates.Architecture = desiredArch optrAvailableUpdates.Current = current @@ -147,9 +156,9 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1 } type availableUpdates struct { - Upstream string - Channel string - Architecture string + UpdateService string + Channel string + Architecture string // LastAttempt records the time of the most recent attempt at update // retrieval, regardless of whether it was successful. @@ -158,10 +167,10 @@ type availableUpdates struct { // LastSyncOrConfigChange records the most recent time when any of // the following events occurred: // - // * Upstream changed, reflecting a new authority, and obsoleting + // * UpdateService changed, reflecting a new authority, and obsoleting // any information retrieved from (or failures // retrieving from) the // previous authority. - // * Channel changes. Same reasoning as for Upstream. + // * Channel changes. Same reasoning as for UpdateService. // * A slice of Updates was successfully retrieved, even if that // slice was empty. LastSyncOrConfigChange time.Time @@ -183,7 +192,7 @@ func (u *availableUpdates) NeedsUpdate(original *configv1.ClusterVersion) *confi return nil } // Architecture could change but does not reside in ClusterVersion - if u.Upstream != string(original.Spec.Upstream) || u.Channel != original.Spec.Channel { + if u.UpdateService != string(original.Spec.Upstream) || u.Channel != original.Spec.Channel { return nil } if equality.Semantic.DeepEqual(u.Updates, original.Status.AvailableUpdates) && @@ -225,7 +234,7 @@ func (optr *Operator) setAvailableUpdates(u *availableUpdates) { optr.statusLock.Lock() defer optr.statusLock.Unlock() if u != nil && (optr.availableUpdates == nil || - optr.availableUpdates.Upstream != u.Upstream || + optr.availableUpdates.UpdateService != u.UpdateService || optr.availableUpdates.Channel != u.Channel || optr.availableUpdates.Architecture != u.Architecture || success) { @@ -247,7 +256,7 @@ func (optr *Operator) getAvailableUpdates() *availableUpdates { } u := &availableUpdates{ - Upstream: optr.availableUpdates.Upstream, + UpdateService: optr.availableUpdates.UpdateService, Channel: optr.availableUpdates.Channel, Architecture: optr.availableUpdates.Architecture, LastAttempt: optr.availableUpdates.LastAttempt, @@ -295,23 +304,23 @@ func (optr *Operator) getDesiredArchitecture(update *configv1.Update) string { return "" } -func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, transport *http.Transport, userAgent, upstream, desiredArch, +func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, transport *http.Transport, userAgent, updateService, desiredArch, currentArch, channel, version string, conditionRegistry clusterconditions.ConditionRegistry) (configv1.Release, []configv1.Release, []configv1.ConditionalUpdate, configv1.ClusterOperatorStatusCondition) { var cvoCurrent configv1.Release - if len(upstream) == 0 { + if len(updateService) == 0 { return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{ Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "NoUpstream", - Message: "No upstream server has been set to retrieve updates.", + Message: "No updateService server has been set to retrieve updates.", } } - upstreamURI, err := url.Parse(upstream) + updateServiceURI, err := url.Parse(updateService) if err != nil { return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{ Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "InvalidURI", - Message: fmt.Sprintf("failed to parse upstream URL: %s", err), + Message: fmt.Sprintf("failed to parse update service URL: %s", err), } } @@ -353,11 +362,11 @@ func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, tran } } - current, updates, conditionalUpdates, err := cincinnati.NewClient(uuid, transport, userAgent, conditionRegistry).GetUpdates(ctx, upstreamURI, desiredArch, + current, updates, conditionalUpdates, err := cincinnati.NewClient(uuid, transport, userAgent, conditionRegistry).GetUpdates(ctx, updateServiceURI, desiredArch, currentArch, channel, currentVersion) if err != nil { - klog.V(2).Infof("Upstream server %s could not return available updates: %v", upstream, err) + klog.V(2).Infof("Update service %s could not return available updates: %v", updateService, err) if updateError, ok := err.(*cincinnati.Error); ok { return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{ Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: updateError.Reason, diff --git a/pkg/cvo/availableupdates_test.go b/pkg/cvo/availableupdates_test.go index 4d4856969f..d57bcf9879 100644 --- a/pkg/cvo/availableupdates_test.go +++ b/pkg/cvo/availableupdates_test.go @@ -116,7 +116,7 @@ func newOperator(url, version string, promqlMock clusterconditions.Condition) (* registry.Register("Always", &always.Always{}) registry.Register("PromQL", promqlMock) operator := &Operator{ - defaultUpstreamServer: url, + updateService: url, architecture: "amd64", proxyLister: notFoundProxyLister{}, cmConfigManagedLister: notFoundConfigMapLister{}, @@ -149,6 +149,7 @@ func TestSyncAvailableUpdates(t *testing.T) { fakeOsus, mockPromql, expectedConditionalUpdates, version := osusWithSingleConditionalEdge() defer fakeOsus.Close() expectedAvailableUpdates, optr := newOperator(fakeOsus.URL, version, mockPromql) + expectedAvailableUpdates.UpdateService = fakeOsus.URL expectedAvailableUpdates.ConditionalUpdates = expectedConditionalUpdates expectedAvailableUpdates.Channel = cvFixture.Spec.Channel expectedAvailableUpdates.Condition = configv1.ClusterOperatorStatusCondition{ diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index 134f2cafd5..634972455f 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -97,7 +97,7 @@ type Operator struct { eventRecorder record.EventRecorder // minimumUpdateCheckInterval is the minimum duration to check for updates from - // the upstream. + // the update service. minimumUpdateCheckInterval time.Duration // architecture identifies the current architecture being used to retrieve available updates // from OSUS. It's possible values and how it's set are defined by the OSUS Cincinnati API's @@ -105,8 +105,11 @@ type Operator struct { architecture string // payloadDir is intended for testing. If unset it will default to '/' payloadDir string - // defaultUpstreamServer is intended for testing. - defaultUpstreamServer string + + // updateService configures the preferred update service. If set, + // this option overrides any upstream value configured in ClusterVersion + // spec. + updateService string cvLister configlistersv1.ClusterVersionLister coLister configlistersv1.ClusterOperatorLister @@ -191,6 +194,7 @@ func New( clusterProfile string, promqlTarget clusterconditions.PromQLTarget, injectClusterIdIntoPromQL bool, + updateService string, ) (*Operator, error) { eventBroadcaster := record.NewBroadcaster() eventBroadcaster.StartLogging(klog.Infof) @@ -208,7 +212,7 @@ func New( minimumUpdateCheckInterval: minimumInterval, upgradeableCheckIntervals: defaultUpgradeableCheckIntervals(), payloadDir: overridePayloadDir, - defaultUpstreamServer: "https://api.openshift.com/api/upgrades_info/v1/graph", + updateService: updateService, client: client, kubeClient: kubeClient, @@ -807,7 +811,7 @@ func versionStringFromRelease(release configv1.Release) string { } // currentVersion returns an update object describing the current -// known cluster version. Values from the upstream Cincinnati service +// known cluster version. Values from the update service // are used as fallbacks for any properties not defined in the release // image itself. func (optr *Operator) currentVersion() configv1.Release { diff --git a/pkg/cvo/cvo_test.go b/pkg/cvo/cvo_test.go index cb3d693a90..7bbb89d92c 100644 --- a/pkg/cvo/cvo_test.go +++ b/pkg/cvo/cvo_test.go @@ -1239,8 +1239,8 @@ func TestOperator_sync(t *testing.T) { namespace: "test", name: "default", availableUpdates: &availableUpdates{ - Upstream: "http://localhost:8080/graph", - Channel: "fast", + UpdateService: "http://localhost:8080/graph", + Channel: "fast", Current: configv1.Release{ Version: "0.0.1-abc", Image: "image/image:v4.0.1", @@ -1567,7 +1567,7 @@ func TestOperator_sync(t *testing.T) { }, }, { - name: "new available updates for the default upstream URL, client has no upstream", + name: "new available updates for the default update service URL, client has no update service", syncStatus: &SyncWorkerStatus{ Generation: 2, Reconciling: true, @@ -1578,12 +1578,12 @@ func TestOperator_sync(t *testing.T) { release: configv1.Release{ Image: "image/image:v4.0.1", }, - namespace: "test", - name: "default", - defaultUpstreamServer: "http://localhost:8080/graph", + namespace: "test", + name: "default", + updateService: "http://localhost:8080/graph", availableUpdates: &availableUpdates{ - Upstream: "", - Channel: "fast", + UpdateService: "", + Channel: "fast", Updates: []configv1.Release{ {Version: "4.0.2", Image: "test/image:1"}, {Version: "4.0.3", Image: "test/image:2"}, @@ -1668,8 +1668,8 @@ func TestOperator_sync(t *testing.T) { namespace: "test", name: "default", availableUpdates: &availableUpdates{ - Upstream: "http://localhost:8080/graph", - Channel: "fast", + UpdateService: "http://localhost:8080/graph", + Channel: "fast", Updates: []configv1.Release{ {Version: "4.0.2", Image: "test/image:1"}, {Version: "4.0.3", Image: "test/image:2"}, @@ -2322,13 +2322,15 @@ func TestOperator_availableUpdatesSync(t *testing.T) { }, }, { - name: "report an error condition when no upstream is set", + name: "no operator or ClusterVersion upstream uses the default update service", handler: func(w http.ResponseWriter, req *http.Request) { http.Error(w, "bad things", http.StatusInternalServerError) }, optr: &Operator{ + architecture: "amd64", release: configv1.Release{ - Image: "image/image:v4.0.1", + Version: "4.0.1", + Image: "image/image:v4.0.1", }, namespace: "test", name: "default", @@ -2350,13 +2352,14 @@ func TestOperator_availableUpdatesSync(t *testing.T) { ), }, wantUpdates: &availableUpdates{ - Upstream: "", - Channel: "fast", + UpdateService: "", + Channel: "fast", + Architecture: "amd64", Condition: configv1.ClusterOperatorStatusCondition{ Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, - Reason: "NoUpstream", - Message: "No upstream server has been set to retrieve updates.", + Reason: "VersionNotFound", + Message: `Unable to retrieve available updates: currently reconciling cluster version 4.0.1 not found in the "fast" channel`, }, }, }, @@ -2366,7 +2369,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) { http.Error(w, "bad things", http.StatusInternalServerError) }, optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -2391,8 +2394,8 @@ func TestOperator_availableUpdatesSync(t *testing.T) { ), }, wantUpdates: &availableUpdates{ - Upstream: "", - Channel: "", + UpdateService: "http://localhost:8080/graph", + Channel: "", Condition: configv1.ClusterOperatorStatusCondition{ Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, @@ -2407,8 +2410,8 @@ func TestOperator_availableUpdatesSync(t *testing.T) { http.Error(w, "bad things", http.StatusInternalServerError) }, optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", - architecture: "amd64", + updateService: "http://localhost:8080/graph", + architecture: "amd64", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -2433,9 +2436,9 @@ func TestOperator_availableUpdatesSync(t *testing.T) { ), }, wantUpdates: &availableUpdates{ - Upstream: "", - Channel: "", - Architecture: "amd64", + UpdateService: "http://localhost:8080/graph", + Channel: "", + Architecture: "amd64", Condition: configv1.ClusterOperatorStatusCondition{ Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, @@ -2450,8 +2453,8 @@ func TestOperator_availableUpdatesSync(t *testing.T) { http.Error(w, "bad things", http.StatusInternalServerError) }, optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", - architecture: "amd64", + updateService: "http://localhost:8080/graph", + architecture: "amd64", release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -2475,9 +2478,9 @@ func TestOperator_availableUpdatesSync(t *testing.T) { ), }, wantUpdates: &availableUpdates{ - Upstream: "", - Channel: "fast", - Architecture: "amd64", + UpdateService: "http://localhost:8080/graph", + Channel: "fast", + Architecture: "amd64", Condition: configv1.ClusterOperatorStatusCondition{ Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, @@ -2492,8 +2495,8 @@ func TestOperator_availableUpdatesSync(t *testing.T) { http.Error(w, "bad things", http.StatusInternalServerError) }, optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", - architecture: "amd64", + updateService: "http://localhost:8080/graph", + architecture: "amd64", release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -2524,9 +2527,9 @@ func TestOperator_availableUpdatesSync(t *testing.T) { ), }, wantUpdates: &availableUpdates{ - Upstream: "", - Channel: "fast", - Architecture: "amd64", + UpdateService: "http://localhost:8080/graph", + Channel: "fast", + Architecture: "amd64", Condition: configv1.ClusterOperatorStatusCondition{ Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, @@ -2555,8 +2558,8 @@ func TestOperator_availableUpdatesSync(t *testing.T) { `) }, optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", - architecture: "amd64", + updateService: "http://localhost:8080/graph", + architecture: "amd64", release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -2588,9 +2591,9 @@ func TestOperator_availableUpdatesSync(t *testing.T) { ), }, wantUpdates: &availableUpdates{ - Upstream: "", - Channel: "fast", - Architecture: "amd64", + UpdateService: "http://localhost:8080/graph", + Channel: "fast", + Architecture: "amd64", Current: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -2622,8 +2625,8 @@ func TestOperator_availableUpdatesSync(t *testing.T) { `) }, optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", - architecture: "amd64", + updateService: "http://localhost:8080/graph", + architecture: "amd64", release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -2655,10 +2658,10 @@ func TestOperator_availableUpdatesSync(t *testing.T) { ), }, wantUpdates: &availableUpdates{ - Upstream: "", - Channel: "fast", - Architecture: "amd64", - Current: configv1.Release{Version: "4.0.1", Image: "image/image:v4.0.1"}, + UpdateService: "http://localhost:8080/graph", + Channel: "fast", + Architecture: "amd64", + Current: configv1.Release{Version: "4.0.1", Image: "image/image:v4.0.1"}, Updates: []configv1.Release{ {Version: "4.0.2", Image: "image/image:v4.0.2"}, {Version: "4.0.2-prerelease", Image: "some.other.registry/image/image:v4.0.2"}, @@ -2675,12 +2678,12 @@ func TestOperator_availableUpdatesSync(t *testing.T) { http.Error(w, "bad things", http.StatusInternalServerError) }, optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", minimumUpdateCheckInterval: 1 * time.Minute, availableUpdates: &availableUpdates{ - Upstream: "http://localhost:8080/graph", - Channel: "fast", - LastAttempt: time.Now(), + UpdateService: "http://localhost:8080/graph", + Channel: "fast", + LastAttempt: time.Now(), }, release: configv1.Release{ Version: "4.0.1", @@ -2715,6 +2718,56 @@ func TestOperator_availableUpdatesSync(t *testing.T) { ), }, }, + { + name: "operator update service takes precedence over ClusterVersion upstream", + handler: func(w http.ResponseWriter, req *http.Request) { + http.Error(w, "bad things", http.StatusInternalServerError) + }, + optr: &Operator{ + updateService: "http://localhost:8080/graph", + architecture: "amd64", + release: configv1.Release{ + Version: "4.0.1", + Image: "image/image:v4.0.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Upstream: configv1.URL("http://localhost:8080/does-not-exist"), + Channel: "fast", + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Image: "image/image:v4.0.1"}, + }, + Conditions: []configv1.ClusterOperatorStatusCondition{ + {Type: ImplicitlyEnabledCapabilities, Status: "False", Reason: "AsExpected", Message: "Capabilities match configured spec"}, + {Type: configv1.OperatorAvailable, Status: configv1.ConditionTrue, Message: "Done applying image/image:v4.0.1"}, + {Type: ClusterStatusFailing, Status: configv1.ConditionFalse}, + {Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse}, + }, + }, + }, + ), + }, + wantUpdates: &availableUpdates{ + UpdateService: "http://localhost:8080/graph", + Channel: "fast", + Architecture: "amd64", + Condition: configv1.ClusterOperatorStatusCondition{ + Type: configv1.RetrievedUpdates, + Status: configv1.ConditionFalse, + Reason: "ResponseFailed", + Message: "Unable to retrieve available updates: unexpected HTTP status: 500 Internal Server Error", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2726,14 +2779,16 @@ func TestOperator_availableUpdatesSync(t *testing.T) { optr.cmConfigManagedLister = &cmConfigLister{} optr.eventRecorder = record.NewFakeRecorder(100) + var updateServiceURI string if tt.handler != nil { s := httptest.NewServer(http.HandlerFunc(tt.handler)) defer s.Close() - if optr.defaultUpstreamServer == "http://localhost:8080/graph" { - optr.defaultUpstreamServer = s.URL + updateServiceURI = s.URL + if optr.updateService == "http://localhost:8080/graph" { + optr.updateService = updateServiceURI } - if optr.availableUpdates != nil && optr.availableUpdates.Upstream == "http://localhost:8080/graph" { - optr.availableUpdates.Upstream = s.URL + if optr.availableUpdates != nil && optr.availableUpdates.UpdateService == "http://localhost:8080/graph" { + optr.availableUpdates.UpdateService = updateServiceURI } } old := optr.availableUpdates @@ -2755,13 +2810,14 @@ func TestOperator_availableUpdatesSync(t *testing.T) { } if optr.availableUpdates != nil { - if optr.availableUpdates.Upstream == optr.defaultUpstreamServer && len(optr.defaultUpstreamServer) > 0 { - optr.availableUpdates.Upstream = "" - } optr.availableUpdates.Condition.LastTransitionTime = metav1.Time{} optr.availableUpdates.LastAttempt = time.Time{} optr.availableUpdates.LastSyncOrConfigChange = time.Time{} + if updateServiceURI != "" && optr.availableUpdates.UpdateService == updateServiceURI { + optr.availableUpdates.UpdateService = "http://localhost:8080/graph" + } } + if !reflect.DeepEqual(optr.availableUpdates, tt.wantUpdates) { t.Fatalf("unexpected: %s", diff.ObjectReflectDiff(tt.wantUpdates, optr.availableUpdates)) } @@ -2863,7 +2919,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "report error condition when the single clusteroperator is not upgradeable", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -2916,7 +2972,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "report error condition when single clusteroperator is not upgradeable and another has no conditions", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -2977,7 +3033,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "report error condition when single clusteroperator is not upgradeable and another is upgradeable", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -3041,7 +3097,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "report error condition when two clusteroperators are not upgradeable", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -3107,7 +3163,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "report error condition when clusteroperators and version are not upgradeable", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -3186,7 +3242,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "no error conditions", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -3220,7 +3276,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "no error conditions", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", Image: "image/image:v4.0.1", @@ -3256,7 +3312,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "no error conditions and admin ack gate does not apply to version", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.9.0", Image: "image/image:v4.9.1", @@ -3304,7 +3360,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "admin-gates configmap gate does not have value", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3348,7 +3404,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "admin ack required", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3392,7 +3448,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "admin ack required and admin ack gate does not apply to version", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3436,7 +3492,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "admin ack required and configmap gate does not have value", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3480,7 +3536,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "multiple admin acks required", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3525,7 +3581,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "admin ack found", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3566,7 +3622,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "admin ack 2 of 3 found", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3616,7 +3672,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "multiple admin acks found", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3661,7 +3717,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "admin-acks configmap not found", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", @@ -3710,7 +3766,7 @@ func TestOperator_upgradeableSync(t *testing.T) { { name: "admin-gates configmap not found", optr: &Operator{ - defaultUpstreamServer: "http://localhost:8080/graph", + updateService: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.8.0", Image: "image/image:v4.8.1", diff --git a/pkg/cvo/metrics.go b/pkg/cvo/metrics.go index c021eedc32..d759d1c514 100644 --- a/pkg/cvo/metrics.go +++ b/pkg/cvo/metrics.go @@ -454,7 +454,9 @@ func (m *operatorMetrics) Collect(ch chan<- prometheus.Metric) { if len(cv.Spec.Upstream) > 0 || len(cv.Status.AvailableUpdates) > 0 || resourcemerge.IsOperatorStatusConditionTrue(cv.Status.Conditions, configv1.RetrievedUpdates) { upstream := "" - if len(cv.Spec.Upstream) > 0 { + if len(m.optr.updateService) > 0 { + upstream = string(m.optr.updateService) + } else if len(cv.Spec.Upstream) > 0 { upstream = string(cv.Spec.Upstream) } g := m.availableUpdates.WithLabelValues(upstream, cv.Spec.Channel) diff --git a/pkg/start/start.go b/pkg/start/start.go index 46cf79140d..325621e1f3 100644 --- a/pkg/start/start.go +++ b/pkg/start/start.go @@ -66,6 +66,11 @@ type Options struct { PromQLTarget clusterconditions.PromQLTarget InjectClusterIdIntoPromQL bool + // UpdateService configures the preferred update service. If set, + // this option overrides any upstream value configured in ClusterVersion + // spec. + UpdateService string + // Exclude is used to determine whether to exclude // certain manifests based on an annotation: // exclude.release.openshift.io/=true @@ -509,6 +514,7 @@ func (o *Options) NewControllerContext(cb *ClientBuilder, startingFeatureSet str o.ClusterProfile, o.PromQLTarget, o.InjectClusterIdIntoPromQL, + o.UpdateService, ) if err != nil { return nil, err