Skip to content

Commit 59f624e

Browse files
wkingwking
committed
pkg/cli/admin/upgrade/recommend: Add a --quiet option
As part of the --affirm feature, because listing unaccepted issues names gives us a way to say "we have more problems we think you should consider" for folks who just want a short summary before deciding whether they want more details. I'm keeping the chatty version as the default, because historically we have had users who don't want to look at the details, and instead hope that waiting will be enough for the issues to resolve. For "there was a regression, and Red Hat will ship a fix", waiting will eventually work. But for "your cluster is alerting, and you should look into that", waiting is unlikely to help. And even for the regression situation, it is the user's responsibility to weigh the risk of updating into the known regression against the risk of remaining on an old, possibly buggy release. Red Hat will support either choice [1]. Having --quiet be an explicit choice makes it clear that Red Hat thinks the user should be engaging with and evaluating any detected issues, while allowing users to say "I don't have time now, sorry. Give me the short summary". [1]: https://docs.redhat.com/en/documentation/openshift_container_platform/4.18/html/updating_clusters/understanding-openshift-updates-1#conditional-updates-overview_understanding-update-channels-releases
1 parent 2d7e004 commit 59f624e

File tree

1 file changed

+77
-41
lines changed

1 file changed

+77
-41
lines changed

‎pkg/cli/admin/upgrade/recommend/recommend.go‎

Lines changed: 77 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ func New(f kcmdutil.Factory, streams genericiooptions.IOStreams) *cobra.Command
6363
flags.StringVar(&o.rawVersion, "version", o.rawVersion, "Select a particular target release to display by version.")
6464

6565
if kcmdutil.FeatureGate("OC_ENABLE_CMD_UPGRADE_RECOMMEND_ACCEPT").IsEnabled() {
66+
flags.BoolVar(&o.quiet, "quiet", o.quiet, "When --quiet is true and --version is set, only print unaccepted issue names.")
6667
flags.StringSliceVar(&o.accept, "accept", o.accept, "Comma-delimited names for issues that you find acceptable. With --version, any unaccepted issues will result in a non-zero exit code.")
6768
}
6869

@@ -80,6 +81,9 @@ type options struct {
8081
showOutdatedReleases bool
8182
precheckEnabled bool
8283

84+
// quiet configures the verbosity of output. When 'quiet' is true and 'version' is set, only print unaccepted issue names.
85+
quiet bool
86+
8387
// rawVersion is parsed into version by options.Complete. Do not consume it directly outside of that early option handling.
8488
rawVersion string
8589

@@ -115,7 +119,11 @@ func (o *options) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string
115119
}
116120
}
117121

118-
if o.rawVersion != "" {
122+
if o.rawVersion == "" {
123+
if o.quiet {
124+
return errors.New("--quiet can only be set when --version is set")
125+
}
126+
} else {
119127
if o.showOutdatedReleases {
120128
return errors.New("when --version is set, --show-outdated-releases is unnecessary")
121129
}
@@ -154,15 +162,19 @@ func (o *options) Run(ctx context.Context) error {
154162
if accept.Has(string(clusterStatusFailing)) {
155163
acceptContext = "accepted "
156164
}
157-
fmt.Fprintf(o.Out, "%s%s=%s:\n\n Reason: %s\n Message: %s\n\n", acceptContext, c.Type, c.Status, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
165+
if !o.quiet {
166+
fmt.Fprintf(o.Out, "%s%s=%s:\n\n Reason: %s\n Message: %s\n\n", acceptContext, c.Type, c.Status, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
167+
}
158168
issues.Insert(string(clusterStatusFailing))
159169
}
160170
} else {
161171
acceptContext := ""
162172
if accept.Has(string(clusterStatusFailing)) {
163173
acceptContext = "accepted "
164174
}
165-
fmt.Fprintf(o.ErrOut, "%swarning: No current %s info, see `oc describe clusterversion` for more details.\n", acceptContext, clusterStatusFailing)
175+
if !o.quiet {
176+
fmt.Fprintf(o.ErrOut, "%swarning: No current %s info, see `oc describe clusterversion` for more details.\n", acceptContext, clusterStatusFailing)
177+
}
166178
issues.Insert(string(clusterStatusFailing))
167179
}
168180

@@ -171,14 +183,18 @@ func (o *options) Run(ctx context.Context) error {
171183
if accept.Has(string(configv1.OperatorProgressing)) {
172184
acceptContext = "accepted "
173185
}
174-
fmt.Fprintf(o.Out, "%sinfo: An update is in progress. You may wish to let this update complete before requesting a new update.\n %s\n\n", acceptContext, strings.ReplaceAll(c.Message, "\n", "\n "))
186+
if !o.quiet {
187+
fmt.Fprintf(o.Out, "%sinfo: An update is in progress. You may wish to let this update complete before requesting a new update.\n %s\n\n", acceptContext, strings.ReplaceAll(c.Message, "\n", "\n "))
188+
}
175189
issues.Insert(string(configv1.OperatorProgressing))
176190
}
177191

178192
if o.precheckEnabled {
179193
conditions, err := o.precheck(ctx)
180194
if err != nil {
181-
fmt.Fprintf(o.Out, "Failed to check for at least some preconditions: %v\n", err)
195+
if !o.quiet {
196+
fmt.Fprintf(o.Out, "Failed to check for at least some preconditions: %v\n", err)
197+
}
182198
issues.Insert("FailedToCompletePrecheck")
183199
}
184200
var happyConditions []string
@@ -196,40 +212,48 @@ func (o *options) Run(ctx context.Context) error {
196212
}
197213
}
198214
}
199-
if len(happyConditions) > 0 {
200-
sort.Strings(happyConditions)
201-
fmt.Fprintf(o.Out, "The following conditions found no cause for concern in updating this cluster to later releases: %s\n\n", strings.Join(happyConditions, ", "))
202-
}
203-
if len(acceptedConditions) > 0 {
204-
sort.Strings(acceptedConditions)
205-
fmt.Fprintf(o.Out, "The following conditions found cause for concern in updating this cluster to later releases, but were explicitly accepted via --accept: %s\n\n", strings.Join(acceptedConditions, ", "))
206-
}
207-
if len(unhappyConditions) > 0 {
208-
sort.Strings(unhappyConditions)
209-
fmt.Fprintf(o.Out, "The following conditions found cause for concern in updating this cluster to later releases: %s\n\n", strings.Join(unhappyConditions, ", "))
210215

211-
for _, c := range conditions {
212-
if c.Status != metav1.ConditionTrue {
213-
fmt.Fprintf(o.Out, "%s=%s:\n\n Reason: %s\n Message: %s\n\n", c.Type, c.Status, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
216+
if !o.quiet {
217+
if len(happyConditions) > 0 {
218+
sort.Strings(happyConditions)
219+
fmt.Fprintf(o.Out, "The following conditions found no cause for concern in updating this cluster to later releases: %s\n\n", strings.Join(happyConditions, ", "))
220+
}
221+
if len(acceptedConditions) > 0 {
222+
sort.Strings(acceptedConditions)
223+
fmt.Fprintf(o.Out, "The following conditions found cause for concern in updating this cluster to later releases, but were explicitly accepted via --accept: %s\n\n", strings.Join(acceptedConditions, ", "))
224+
}
225+
if len(unhappyConditions) > 0 {
226+
sort.Strings(unhappyConditions)
227+
fmt.Fprintf(o.Out, "The following conditions found cause for concern in updating this cluster to later releases: %s\n\n", strings.Join(unhappyConditions, ", "))
228+
229+
for _, c := range conditions {
230+
if c.Status != metav1.ConditionTrue {
231+
fmt.Fprintf(o.Out, "%s=%s:\n\n Reason: %s\n Message: %s\n\n", c.Type, c.Status, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
232+
}
214233
}
215234
}
216235
}
217236
}
218237

219238
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.RetrievedUpdates); c != nil && c.Status != configv1.ConditionTrue {
220-
fmt.Fprintf(o.ErrOut, "warning: Cannot refresh available updates:\n Reason: %s\n Message: %s\n\n", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
239+
if !o.quiet {
240+
fmt.Fprintf(o.ErrOut, "warning: Cannot refresh available updates:\n Reason: %s\n Message: %s\n\n", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
241+
}
242+
issues.Insert("CannotRetrieveUpdates")
221243
}
222244

223-
if cv.Spec.Channel != "" {
224-
if cv.Spec.Upstream == "" {
225-
fmt.Fprint(o.Out, "Upstream update service is unset, so the cluster will use an appropriate default.\n")
226-
} else {
227-
fmt.Fprintf(o.Out, "Upstream update service: %s\n", cv.Spec.Upstream)
228-
}
229-
if len(cv.Status.Desired.Channels) > 0 {
230-
fmt.Fprintf(o.Out, "Channel: %s (available channels: %s)\n", cv.Spec.Channel, strings.Join(cv.Status.Desired.Channels, ", "))
231-
} else {
232-
fmt.Fprintf(o.Out, "Channel: %s\n", cv.Spec.Channel)
245+
if !o.quiet {
246+
if cv.Spec.Channel != "" {
247+
if cv.Spec.Upstream == "" {
248+
fmt.Fprint(o.Out, "Upstream update service is unset, so the cluster will use an appropriate default.\n")
249+
} else {
250+
fmt.Fprintf(o.Out, "Upstream update service: %s\n", cv.Spec.Upstream)
251+
}
252+
if len(cv.Status.Desired.Channels) > 0 {
253+
fmt.Fprintf(o.Out, "Channel: %s (available channels: %s)\n", cv.Spec.Channel, strings.Join(cv.Status.Desired.Channels, ", "))
254+
} else {
255+
fmt.Fprintf(o.Out, "Channel: %s\n", cv.Spec.Channel)
256+
}
233257
}
234258
}
235259

@@ -238,7 +262,9 @@ func (o *options) Run(ctx context.Context) error {
238262
for i, update := range cv.Status.ConditionalUpdates {
239263
version, err := semver.Parse(update.Release.Version)
240264
if err != nil {
241-
fmt.Fprintf(o.ErrOut, "warning: Cannot parse SemVer available update %q: %v", update.Release.Version, err)
265+
if !o.quiet {
266+
fmt.Fprintf(o.ErrOut, "warning: Cannot parse SemVer available update %q: %v", update.Release.Version, err)
267+
}
242268
continue
243269
}
244270

@@ -263,7 +289,9 @@ func (o *options) Run(ctx context.Context) error {
263289

264290
version, err := semver.Parse(update.Version)
265291
if err != nil {
266-
fmt.Fprintf(o.ErrOut, "warning: Cannot parse SemVer available update %q: %v", update.Version, err)
292+
if !o.quiet {
293+
fmt.Fprintf(o.ErrOut, "warning: Cannot parse SemVer available update %q: %v", update.Version, err)
294+
}
267295
continue
268296
}
269297

@@ -277,8 +305,10 @@ func (o *options) Run(ctx context.Context) error {
277305
}
278306

279307
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorUpgradeable); c != nil && c.Status == configv1.ConditionFalse {
280-
if err := injectUpgradeableAsCondition(cv.Status.Desired.Version, c, majorMinorBuckets); err != nil {
281-
fmt.Fprintf(o.ErrOut, "warning: Cannot inject %s=%s as a conditional update risk: %s\n\nReason: %s\n Message: %s\n\n", c.Type, c.Status, err, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
308+
if err := injectUpgradeableAsCondition(cv.Status.Desired.Version, c, majorMinorBuckets); err != nil && !o.quiet {
309+
if !o.quiet {
310+
fmt.Fprintf(o.ErrOut, "warning: Cannot inject %s=%s as a conditional update risk: %s\n\nReason: %s\n Message: %s\n\n", c.Type, c.Status, err, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
311+
}
282312
}
283313
}
284314

@@ -294,21 +324,27 @@ func (o *options) Run(ctx context.Context) error {
294324
} else {
295325
for _, update := range minor {
296326
if update.Release.Version == o.version.String() {
297-
fmt.Fprintln(o.Out)
327+
if !o.quiet {
328+
fmt.Fprintln(o.Out)
329+
}
298330
if c := notRecommendedCondition(update); c == nil {
299-
fmt.Fprintf(o.Out, "Update to %s has no known issues relevant to this cluster.\nImage: %s\nRelease URL: %s\n", update.Release.Version, update.Release.Image, update.Release.URL)
331+
if !o.quiet {
332+
fmt.Fprintf(o.Out, "Update to %s has no known issues relevant to this cluster.\nImage: %s\nRelease URL: %s\n", update.Release.Version, update.Release.Image, update.Release.URL)
333+
}
300334
} else {
301-
acceptContext := ""
302-
if accept.Has("ConditionalUpdateRisk") {
303-
acceptContext = "accepted "
335+
if !o.quiet {
336+
acceptContext := ""
337+
if accept.Has("ConditionalUpdateRisk") {
338+
acceptContext = "accepted "
339+
}
340+
fmt.Fprintf(o.Out, "Update to %s %s=%s:\nImage: %s\nRelease URL: %s\nReason: %s%s\nMessage: %s\n", update.Release.Version, c.Type, c.Status, update.Release.Image, update.Release.URL, acceptContext, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
304341
}
305-
fmt.Fprintf(o.Out, "Update to %s %s=%s:\nImage: %s\nRelease URL: %s\nReason: %s%s\nMessage: %s\n", update.Release.Version, c.Type, c.Status, update.Release.Image, update.Release.URL, acceptContext, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
306342
issues.Insert("ConditionalUpdateRisk")
307343
}
308344
unaccepted := issues.Difference(accept)
309345
if unaccepted.Len() > 0 {
310346
return fmt.Errorf("issues that apply to this cluster but which were not included in --accept: %s", strings.Join(sets.List(unaccepted), ","))
311-
} else if issues.Len() > 0 {
347+
} else if issues.Len() > 0 && !o.quiet {
312348
fmt.Fprintf(o.Out, "Update to %s has no known issues relevant to this cluster other than the accepted %s.\n", update.Release.Version, strings.Join(sets.List(issues), ","))
313349
}
314350
return nil

0 commit comments

Comments
 (0)