Bug 1685338: pkg/cvo: Reason granularity for RemoteFailed

pkg/cincinnati/cincinnati.go

@@ -34,31 +34,44 @@ func NewClient(id uuid.UUID, proxyURL *url.URL, tlsConfig *tls.Config) Client {
 // Update is a single node from the update graph.
 type Update node
 
+// Error is returned when are unable to get updates.
+type Error struct {
+	// Reason is the reason suggested for the ClusterOperator status condition.
+	Reason string
+
+	// Message is the message suggested for the ClusterOperator status condition.
+	Message string
+
+	// cause is the upstream error, if any, being wrapped by this error.
+	cause error
+}
+
+// Error serializes the error as a string, to satisfy the error interface.
+func (err *Error) Error() string {
+	return fmt.Sprintf("%s: %s", err.Reason, err.Message)
+}
+
 // GetUpdates fetches the next-applicable update payloads from the specified
 // upstream Cincinnati stack given the current version and channel. The next-
 // applicable updates are determined by downloading the update graph, finding
 // the current version within that graph (typically the root node), and then
 // finding all of the children. These children are the available updates for
 // the current version and their payloads indicate from where the actual update
 // image can be downloaded.
-func (c Client) GetUpdates(upstream string, arch string, channel string, version semver.Version) ([]Update, error) {
+func (c Client) GetUpdates(uri *url.URL, arch string, channel string, version semver.Version) ([]Update, error) {
 	transport := http.Transport{}
 	// Prepare parametrized cincinnati query.
-	cincinnatiURL, err := url.Parse(upstream)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse upstream URL: %s", err)
-	}
-	queryParams := cincinnatiURL.Query()
+	queryParams := uri.Query()
 	queryParams.Add("arch", arch)
 	queryParams.Add("channel", channel)
 	queryParams.Add("id", c.id.String())
 	queryParams.Add("version", version.String())
-	cincinnatiURL.RawQuery = queryParams.Encode()
+	uri.RawQuery = queryParams.Encode()
 
 	// Download the update graph.
-	req, err := http.NewRequest("GET", cincinnatiURL.String(), nil)
+	req, err := http.NewRequest("GET", uri.String(), nil)
 	if err != nil {
-		return nil, err
+		return nil, &Error{Reason: "InvalidRequest", Message: err.Error(), cause: err}
 	}
 	req.Header.Add("Accept", GraphMediaType)
 	if c.tlsConfig != nil {
@@ -72,23 +85,23 @@ func (c Client) GetUpdates(upstream string, arch string, channel string, version
 	client := http.Client{Transport: &transport}
 	resp, err := client.Do(req)
 	if err != nil {
-		return nil, err
+		return nil, &Error{Reason: "RemoteFailed", Message: err.Error(), cause: err}
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("unexpected HTTP status: %s", resp.Status)
+		return nil, &Error{Reason: "ResponseFailed", Message: fmt.Sprintf("unexpected HTTP status: %s", resp.Status)}
 	}
 
 	// Parse the graph.
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		return nil, err
+		return nil, &Error{Reason: "ResponseFailed", Message: err.Error(), cause: err}
 	}
 
 	var graph graph
 	if err = json.Unmarshal(body, &graph); err != nil {
-		return nil, err
+		return nil, &Error{Reason: "ResponseInvalid", Message: err.Error(), cause: err}
 	}
 
 	// Find the current version within the graph.
@@ -102,7 +115,10 @@ func (c Client) GetUpdates(upstream string, arch string, channel string, version
 		}
 	}
 	if !found {
-		return nil, fmt.Errorf("currently installed version %s not found in the %q channel", version, channel)
+		return nil, &Error{
+			Reason:  "VersionNotFound",
+			Message: fmt.Sprintf("currently installed version %s not found in the %q channel", version, channel),
+		}
 	}
 
 	// Find the children of the current version.

pkg/cincinnati/cincinnati_test.go

@@ -49,7 +49,7 @@ func TestGetUpdates(t *testing.T) {
 		name:          "unknown version",
 		version:       "4.0.0-3",
 		expectedQuery: "arch=test-arch&channel=test-channel&id=01234567-0123-0123-0123-0123456789ab&version=4.0.0-3",
-		err:           "currently installed version 4.0.0-3 not found in the \"test-channel\" channel",
+		err:           "VersionNotFound: currently installed version 4.0.0-3 not found in the \"test-channel\" channel",
 	}}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
@@ -127,7 +127,12 @@ func TestGetUpdates(t *testing.T) {
 
 			c := NewClient(clientID, proxyURL, tlsConfig)
 
-			updates, err := c.GetUpdates(ts.URL, arch, channelName, semver.MustParse(test.version))
+			uri, err := url.Parse(ts.URL)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			updates, err := c.GetUpdates(uri, arch, channelName, semver.MustParse(test.version))
 			if test.err == "" {
 				if err != nil {
 					t.Fatalf("expected nil error, got: %v", err)

pkg/cvo/availableupdates.go

@@ -127,6 +127,22 @@ func calculateAvailableUpdatesStatus(clusterID string, proxyURL *url.URL, tlsCon
 		}
 	}
 
+	upstreamURI, err := url.Parse(upstream)
+	if err != nil {
+		return nil, configv1.ClusterOperatorStatusCondition{
+			Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "InvalidURI",
+			Message: fmt.Sprintf("failed to parse upstream URL: %s", err),
+		}
+	}
+
+	uuid, err := uuid.Parse(string(clusterID))
+	if err != nil {
+		return nil, configv1.ClusterOperatorStatusCondition{
+			Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "InvalidID",
+			Message: fmt.Sprintf("invalid cluster ID: %s", err),
+		}
+	}
+
 	if len(arch) == 0 {
 		return nil, configv1.ClusterOperatorStatusCondition{
 			Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "NoArchitecture",
@@ -157,12 +173,19 @@ func calculateAvailableUpdatesStatus(clusterID string, proxyURL *url.URL, tlsCon
 		}
 	}
 
-	updates, err := checkForUpdate(clusterID, proxyURL, tlsConfig, upstream, arch, channel, currentVersion)
+	updates, err := cincinnati.NewClient(uuid, proxyURL, tlsConfig).GetUpdates(upstreamURI, arch, channel, currentVersion)
 	if err != nil {
 		klog.V(2).Infof("Upstream server %s could not return available updates: %v", upstream, err)
+		if updateError, ok := err.(*cincinnati.Error); ok {
+			return nil, configv1.ClusterOperatorStatusCondition{
+				Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: updateError.Reason,
+				Message: fmt.Sprintf("Unable to retrieve available updates: %s", updateError.Message),
+			}
+		}
+		// this should never happen
 		return nil, configv1.ClusterOperatorStatusCondition{
-			Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "RemoteFailed",
-			Message: fmt.Sprintf("Unable to retrieve available updates: %v", err),
+			Type: configv1.RetrievedUpdates, Status: configv1.ConditionFalse, Reason: "Unknown",
+			Message: fmt.Sprintf("Unable to retrieve available updates: %s", err),
 		}
 	}
 
@@ -182,18 +205,6 @@ func calculateAvailableUpdatesStatus(clusterID string, proxyURL *url.URL, tlsCon
 	}
 }
 
-func checkForUpdate(clusterID string, proxyURL *url.URL, tlsConfig *tls.Config, upstream, arch, channel string, currentVersion semver.Version) ([]cincinnati.Update, error) {
-	uuid, err := uuid.Parse(string(clusterID))
-	if err != nil {
-		return nil, err
-	}
-
-	if len(upstream) == 0 {
-		return nil, fmt.Errorf("no upstream URL set for cluster version")
-	}
-	return cincinnati.NewClient(uuid, proxyURL, tlsConfig).GetUpdates(upstream, arch, channel, currentVersion)
-}
-
 // getHTTPSProxyURL returns a url.URL object for the configured
 // https proxy only. It can be nil if does not exist or there is an error.
 func (optr *Operator) getHTTPSProxyURL() (*url.URL, string, error) {

pkg/cvo/cvo_test.go

@@ -2451,7 +2451,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) {
 				Condition: configv1.ClusterOperatorStatusCondition{
 					Type:    configv1.RetrievedUpdates,
 					Status:  configv1.ConditionFalse,
-					Reason:  "RemoteFailed",
+					Reason:  "ResponseFailed",
 					Message: "Unable to retrieve available updates: unexpected HTTP status: 500 Internal Server Error",
 				},
 			},