*: 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]: #1035 (review)

Author: wking

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)
 }

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,

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{

pkg/cvo/cvo.go

@@ -97,16 +97,19 @@ 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
 	// "arch" property.
 	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 {