Skip to content

Commit 69bee91

Browse files
Merge pull request #1035 from wking/command-line-upstream
OTA-1210: *: Add --update-service command-line option
2 parents e3677e0 + b4abe48 commit 69bee91

File tree

7 files changed

+186
-107
lines changed

7 files changed

+186
-107
lines changed

cmd/start.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,6 @@ func init() {
4646
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.")
4747
cmd.PersistentFlags().StringVar(&opts.PrometheusURLString, "metrics-url", opts.PrometheusURLString, "The URL used to access the remote PromQL query service.")
4848
cmd.PersistentFlags().BoolVar(&opts.InjectClusterIdIntoPromQL, "hypershift", opts.InjectClusterIdIntoPromQL, "This options indicates whether the CVO is running inside a hosted control plane.")
49+
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.")
4950
rootCmd.AddCommand(cmd)
5051
}

pkg/cvo/availableupdates.go

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,25 @@ import (
2525

2626
const noArchitecture string = "NoArchitecture"
2727
const noChannel string = "NoChannel"
28+
const defaultUpdateService string = "https://api.openshift.com/api/upgrades_info/v1/graph"
2829

2930
// syncAvailableUpdates attempts to retrieve the latest updates and update the status of the ClusterVersion
3031
// object. It will set the RetrievedUpdates condition. Updates are only checked if it has been more than
3132
// the minimumUpdateCheckInterval since the last check.
3233
func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1.ClusterVersion) error {
33-
usedDefaultUpstream := false
34-
upstream := string(config.Spec.Upstream)
35-
if len(upstream) == 0 {
36-
usedDefaultUpstream = true
37-
upstream = optr.defaultUpstreamServer
34+
usedDefaultUpdateService := false
35+
36+
updateService := optr.updateService
37+
var updateServiceSource string
38+
if len(updateService) > 0 {
39+
updateServiceSource = "the --update-service command line option"
40+
} else if len(config.Spec.Upstream) > 0 {
41+
updateService = string(config.Spec.Upstream)
42+
updateServiceSource = "ClusterVersion spec.upstream"
43+
} else {
44+
usedDefaultUpdateService = true
45+
updateService = defaultUpdateService
46+
updateServiceSource = "the operator's default update service"
3847
}
3948

4049
channel := config.Spec.Channel
@@ -63,7 +72,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
6372
klog.V(2).Infof("Retrieving available updates again, because the channel has changed from %q to %q", optrAvailableUpdates.Channel, channel)
6473
} else if desiredArch != optrAvailableUpdates.Architecture {
6574
klog.V(2).Infof("Retrieving available updates again, because the architecture has changed from %q to %q", optrAvailableUpdates.Architecture, desiredArch)
66-
} else if upstream == optrAvailableUpdates.Upstream || (upstream == optr.defaultUpstreamServer && optrAvailableUpdates.Upstream == "") {
75+
} else if updateService == optrAvailableUpdates.UpdateService || (updateService == defaultUpdateService && optrAvailableUpdates.UpdateService == "") {
6776
needsConditionalUpdateEval := false
6877
for _, conditionalUpdate := range optrAvailableUpdates.ConditionalUpdates {
6978
if recommended := findRecommendedCondition(conditionalUpdate.Conditions); recommended == nil {
@@ -80,7 +89,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
8089
}
8190
needFreshFetch = false
8291
} else {
83-
klog.V(2).Infof("Retrieving available updates again, because the upstream has changed from %q to %q", optrAvailableUpdates.Upstream, config.Spec.Upstream)
92+
klog.V(2).Infof("Retrieving available updates again, because the update service has changed from %q to %q from %s", optrAvailableUpdates.UpdateService, updateService, updateServiceSource)
8493
}
8594

8695
if needFreshFetch {
@@ -93,7 +102,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
93102
clusterId := string(config.Spec.ClusterID)
94103

95104
current, updates, conditionalUpdates, condition := calculateAvailableUpdatesStatus(ctx, clusterId,
96-
transport, userAgent, upstream, desiredArch, currentArch, channel, optr.release.Version, optr.conditionRegistry)
105+
transport, userAgent, updateService, desiredArch, currentArch, channel, optr.release.Version, optr.conditionRegistry)
97106

98107
// Populate conditions on conditional updates from operator state
99108
for i := range optrAvailableUpdates.ConditionalUpdates {
@@ -109,12 +118,12 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
109118
conditionalUpdates = injectClusterIdIntoConditionalUpdates(clusterId, conditionalUpdates)
110119
}
111120

112-
if usedDefaultUpstream {
113-
upstream = ""
121+
if usedDefaultUpdateService {
122+
updateService = ""
114123
}
115124

116125
optrAvailableUpdates.LastAttempt = time.Now()
117-
optrAvailableUpdates.Upstream = upstream
126+
optrAvailableUpdates.UpdateService = updateService
118127
optrAvailableUpdates.Channel = channel
119128
optrAvailableUpdates.Architecture = desiredArch
120129
optrAvailableUpdates.Current = current
@@ -147,9 +156,9 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
147156
}
148157

149158
type availableUpdates struct {
150-
Upstream string
151-
Channel string
152-
Architecture string
159+
UpdateService string
160+
Channel string
161+
Architecture string
153162

154163
// LastAttempt records the time of the most recent attempt at update
155164
// retrieval, regardless of whether it was successful.
@@ -158,10 +167,10 @@ type availableUpdates struct {
158167
// LastSyncOrConfigChange records the most recent time when any of
159168
// the following events occurred:
160169
//
161-
// * Upstream changed, reflecting a new authority, and obsoleting
170+
// * UpdateService changed, reflecting a new authority, and obsoleting
162171
// any information retrieved from (or failures // retrieving from) the
163172
// previous authority.
164-
// * Channel changes. Same reasoning as for Upstream.
173+
// * Channel changes. Same reasoning as for UpdateService.
165174
// * A slice of Updates was successfully retrieved, even if that
166175
// slice was empty.
167176
LastSyncOrConfigChange time.Time
@@ -183,7 +192,7 @@ func (u *availableUpdates) NeedsUpdate(original *configv1.ClusterVersion) *confi
183192
return nil
184193
}
185194
// Architecture could change but does not reside in ClusterVersion
186-
if u.Upstream != string(original.Spec.Upstream) || u.Channel != original.Spec.Channel {
195+
if u.UpdateService != string(original.Spec.Upstream) || u.Channel != original.Spec.Channel {
187196
return nil
188197
}
189198
if equality.Semantic.DeepEqual(u.Updates, original.Status.AvailableUpdates) &&
@@ -225,7 +234,7 @@ func (optr *Operator) setAvailableUpdates(u *availableUpdates) {
225234
optr.statusLock.Lock()
226235
defer optr.statusLock.Unlock()
227236
if u != nil && (optr.availableUpdates == nil ||
228-
optr.availableUpdates.Upstream != u.Upstream ||
237+
optr.availableUpdates.UpdateService != u.UpdateService ||
229238
optr.availableUpdates.Channel != u.Channel ||
230239
optr.availableUpdates.Architecture != u.Architecture ||
231240
success) {
@@ -247,7 +256,7 @@ func (optr *Operator) getAvailableUpdates() *availableUpdates {
247256
}
248257

249258
u := &availableUpdates{
250-
Upstream: optr.availableUpdates.Upstream,
259+
UpdateService: optr.availableUpdates.UpdateService,
251260
Channel: optr.availableUpdates.Channel,
252261
Architecture: optr.availableUpdates.Architecture,
253262
LastAttempt: optr.availableUpdates.LastAttempt,
@@ -295,23 +304,23 @@ func (optr *Operator) getDesiredArchitecture(update *configv1.Update) string {
295304
return ""
296305
}
297306

298-
func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, transport *http.Transport, userAgent, upstream, desiredArch,
307+
func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, transport *http.Transport, userAgent, updateService, desiredArch,
299308
currentArch, channel, version string, conditionRegistry clusterconditions.ConditionRegistry) (configv1.Release, []configv1.Release, []configv1.ConditionalUpdate,
300309
configv1.ClusterOperatorStatusCondition) {
301310

302311
var cvoCurrent configv1.Release
303-
if len(upstream) == 0 {
312+
if len(updateService) == 0 {
304313
return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{
305314
Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "NoUpstream",
306-
Message: "No upstream server has been set to retrieve updates.",
315+
Message: "No updateService server has been set to retrieve updates.",
307316
}
308317
}
309318

310-
upstreamURI, err := url.Parse(upstream)
319+
updateServiceURI, err := url.Parse(updateService)
311320
if err != nil {
312321
return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{
313322
Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "InvalidURI",
314-
Message: fmt.Sprintf("failed to parse upstream URL: %s", err),
323+
Message: fmt.Sprintf("failed to parse update service URL: %s", err),
315324
}
316325
}
317326

@@ -353,11 +362,11 @@ func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, tran
353362
}
354363
}
355364

356-
current, updates, conditionalUpdates, err := cincinnati.NewClient(uuid, transport, userAgent, conditionRegistry).GetUpdates(ctx, upstreamURI, desiredArch,
365+
current, updates, conditionalUpdates, err := cincinnati.NewClient(uuid, transport, userAgent, conditionRegistry).GetUpdates(ctx, updateServiceURI, desiredArch,
357366
currentArch, channel, currentVersion)
358367

359368
if err != nil {
360-
klog.V(2).Infof("Upstream server %s could not return available updates: %v", upstream, err)
369+
klog.V(2).Infof("Update service %s could not return available updates: %v", updateService, err)
361370
if updateError, ok := err.(*cincinnati.Error); ok {
362371
return cvoCurrent, nil, nil, configv1.ClusterOperatorStatusCondition{
363372
Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: updateError.Reason,

pkg/cvo/availableupdates_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func newOperator(url, version string, promqlMock clusterconditions.Condition) (*
116116
registry.Register("Always", &always.Always{})
117117
registry.Register("PromQL", promqlMock)
118118
operator := &Operator{
119-
defaultUpstreamServer: url,
119+
updateService: url,
120120
architecture: "amd64",
121121
proxyLister: notFoundProxyLister{},
122122
cmConfigManagedLister: notFoundConfigMapLister{},
@@ -149,6 +149,7 @@ func TestSyncAvailableUpdates(t *testing.T) {
149149
fakeOsus, mockPromql, expectedConditionalUpdates, version := osusWithSingleConditionalEdge()
150150
defer fakeOsus.Close()
151151
expectedAvailableUpdates, optr := newOperator(fakeOsus.URL, version, mockPromql)
152+
expectedAvailableUpdates.UpdateService = fakeOsus.URL
152153
expectedAvailableUpdates.ConditionalUpdates = expectedConditionalUpdates
153154
expectedAvailableUpdates.Channel = cvFixture.Spec.Channel
154155
expectedAvailableUpdates.Condition = configv1.ClusterOperatorStatusCondition{

pkg/cvo/cvo.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,19 @@ type Operator struct {
9898
eventRecorder record.EventRecorder
9999

100100
// minimumUpdateCheckInterval is the minimum duration to check for updates from
101-
// the upstream.
101+
// the update service.
102102
minimumUpdateCheckInterval time.Duration
103103
// architecture identifies the current architecture being used to retrieve available updates
104104
// from OSUS. It's possible values and how it's set are defined by the OSUS Cincinnati API's
105105
// "arch" property.
106106
architecture string
107107
// payloadDir is intended for testing. If unset it will default to '/'
108108
payloadDir string
109-
// defaultUpstreamServer is intended for testing.
110-
defaultUpstreamServer string
109+
110+
// updateService configures the preferred update service. If set,
111+
// this option overrides any upstream value configured in ClusterVersion
112+
// spec.
113+
updateService string
111114

112115
cvLister configlistersv1.ClusterVersionLister
113116
coLister configlistersv1.ClusterOperatorLister
@@ -189,6 +192,7 @@ func New(
189192
clusterProfile string,
190193
promqlTarget clusterconditions.PromQLTarget,
191194
injectClusterIdIntoPromQL bool,
195+
updateService string,
192196
) (*Operator, error) {
193197
eventBroadcaster := record.NewBroadcaster()
194198
eventBroadcaster.StartLogging(klog.Infof)
@@ -206,7 +210,7 @@ func New(
206210
minimumUpdateCheckInterval: minimumInterval,
207211
upgradeableCheckIntervals: defaultUpgradeableCheckIntervals(),
208212
payloadDir: overridePayloadDir,
209-
defaultUpstreamServer: "https://api.openshift.com/api/upgrades_info/v1/graph",
213+
updateService: updateService,
210214

211215
client: client,
212216
kubeClient: kubeClient,
@@ -811,7 +815,7 @@ func versionStringFromRelease(release configv1.Release) string {
811815
}
812816

813817
// currentVersion returns an update object describing the current
814-
// known cluster version. Values from the upstream Cincinnati service
818+
// known cluster version. Values from the update service
815819
// are used as fallbacks for any properties not defined in the release
816820
// image itself.
817821
func (optr *Operator) currentVersion() configv1.Release {

0 commit comments

Comments
 (0)