Bug 1768260

> https://storage.googleapis.com/origin-ci-test/logs/release-openshift-origin-installer-e2e-aws-upgrade/10313/artifacts/e2e-aws-upgrade/must-gather/quay-io-openshift-release-dev-ocp-v4-0-art-dev-sha256-879583d2070d7e98b24b73535d19d84c62983f16417fe84d40249f46119f332e/namespaces/openshift-cluster-version/pods/cluster-version-operator-dd9fdbfb7-t8kqd/cluster-version-operator/cluster-version-operator/logs/current.log

```
2019-11-01T01:57:40.248306704Z I1101 01:57:40.248248       1 apps.go:115] Deployment downloads is not ready. status: (replicas: 2, updated: 2, ready: 1, unavailable: 1, reason: MinimumReplicasUnavailable, message: Deployment does not have minimum availability.)
2019-11-01T01:57:43.248385305Z I1101 01:57:43.248330       1 apps.go:115] Deployment downloads is not ready. status: (replicas: 2, updated: 2, ready: 1, unavailable: 1, reason: MinimumReplicasUnavailable, message: Deployment does not have minimum availability.)
2019-11-01T01:57:46.248534786Z I1101 01:57:46.248481       1 apps.go:115] Deployment downloads is not ready. status: (replicas: 2, updated: 2, ready: 1, unavailable: 1, reason: MinimumReplicasUnavailable, message: Deployment does not have minimum availability.)
```

If you look at the CVO logs, it shows the detailed message for easy debugging by owners, we have explicitly kept very detailed errors out of clusterversion as they don't provide value to customers.

Not sure if i agree w/ that philosophy (it means dev teams answering a support question are going to have to ask for another round of logs before they can dig into the problem, at a minimum), but ok.

Follow-on question: Why does a "not ready" deployment prevent updating that deployment?  Does the CVO refuse to patch a resource that's not healthy?  Am i misunderstanding what "could not update deployment" means in this context?

> Why does a "not ready" deployment prevent updating that deployment?  Does the CVO refuse to patch a resource that's not healthy?  Am i misunderstanding what "could not update deployment" means in this context?


Updating a deployment is not `object updated in the definition`, but rather the newest/desired version of the deployment is complete ie for deployment that criteria is Ready pods == required pods == updated pods and no unavailable pods..

Seems like it's more of a "waiting for deployment foo to rollout" condition than "could not update" then?  can we make the message clearer?

(In reply to Ben Parees from comment #4)
> Seems like it's more of a "waiting for deployment foo to rollout" condition
> than "could not update" then?  can we make the message clearer?

For CVO and user they don't care about if deployment is updated to latest version but the pods are not running to that version, the required result is still make sure the latest code is running and that what `updated` means for CVO in this context.
So imo the CVO is provding the correct error.

it's not clear who/what "could not update" the deployment.  the message makes it sound (to me) like the CVO couldn't update it.  

the deployment failed to update.  nothing failed to update it.

I would not read that error message and think "oh, i should go look at the deployment and see why it's stuck".

We should offer more targeted error causes on specific things.  We have specific messages for cluster operators, it would be acceptable to have specific errors for workload types (specifically around "not yet available").

The code here is the core (note the comment!)

	// log the errors to assist in debugging future summarization
	if klog.V(4) {
		klog.Infof("Summarizing %d errors", len(errs))
		for _, err := range errs {
			if uErr, ok := err.(*payload.UpdateError); ok {
				if uErr.Task != nil {
					klog.Infof("Update error %d of %d: %s %s (%T: %v)", uErr.Task.Index, uErr.Task.Total, uErr.Reason, uErr.Message, uErr.Nested, uErr.Nested)
				} else {
					klog.Infof("Update error: %s %s (%T: %v)", uErr.Reason, uErr.Message, uErr.Nested, uErr.Nested)
				}
			} else {
				klog.Infof("Update error: %T: %v", err, err)
			}
		}
	}

	// collapse into a set of common errors where necessary
	if len(errs) == 1 {
		return errs[0]
	}
	// hide the generic "not available yet" when there are more specific errors present
	if filtered := filterErrors(errs, isClusterOperatorNotAvailable); len(filtered) > 0 {
		return newMultipleError(filtered)
	}
	// if we're only waiting for operators, condense the error down to a singleton
	if err := newClusterOperatorsNotAvailable(errs); err != nil {
		return err
	}

If we have evidence that a large chunk of upgrade failures are related to rollout (which anecdotally I would believe), then this sort of summarization is appropriate. I would say that deployment/statefulset/daemonset that we wait on should have a roughly equivalent reason and error if we can't make progress that is roughly generic to "rollout" and perform the same summarization.  I might suggest "WorkloadNotAvailable" or "RolloutNotProgressing" as a reason and have SummaryForReason() provide a similar mapping.

In https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/logs/release-openshift-origin-installer-e2e-aws-upgrade/23885, "WorkloadNotAvailable" already is shown in log.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:2409