pkg/cvo/availableupdates: Split 'At' into LastAttempt and LastSyncOrConfigChange

The previous At had been tracking the most recent attempt, regardless
of success.  With this commit, we rename it to the more explicit
LastAttempt and add godocs explaining the intended semantics.

Also grow a LastSyncOrConfigChange sibling, tracking the time since
the last success (or since the configured upstream had been changed).
This will make it more convenient to alert when the slice of available
updates is stale, although that alert itself and the metric that feeds
it will come from follow-up work [1].

Setting u.LastSyncOrConfigChange from optr.availableUpdates in the
else branch preserves any existing timestamp if the new status did not
trigger a reset.

[1]: #357

Author: wking

pkg/cvo/availableupdates.go

@@ -65,14 +65,27 @@ type availableUpdates struct {
 	Upstream string
 	Channel  string
 
-	At time.Time
+	// LastAttempt records the time of the most recent attempt at update
+	// retrieval, regardless of whether it was successful.
+	LastAttempt time.Time
+
+	// LastSyncOrConfigChange records the most recent time when any of
+	// the following events occurred:
+	//
+	// * Upstream 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.
+	// * A slice of Updates was successfully retrieved, even if that
+	//   slice was empty.
+	LastSyncOrConfigChange time.Time
 
 	Updates   []configv1.Update
 	Condition configv1.ClusterOperatorStatusCondition
 }
 
 func (u *availableUpdates) RecentlyChanged(interval time.Duration) bool {
-	return u.At.After(time.Now().Add(-interval))
+	return u.LastAttempt.After(time.Now().Add(-interval))
 }
 
 func (u *availableUpdates) NeedsUpdate(original *configv1.ClusterVersion) *configv1.ClusterVersion {
@@ -95,12 +108,26 @@ func (u *availableUpdates) NeedsUpdate(original *configv1.ClusterVersion) *confi
 
 // setAvailableUpdates updates the currently calculated version of updates.
 func (optr *Operator) setAvailableUpdates(u *availableUpdates) {
+	success := false
 	if u != nil {
-		u.At = time.Now()
+		u.LastAttempt = time.Now()
+		if u.Condition.Type == configv1.RetrievedUpdates {
+			success = u.Condition.Status == configv1.ConditionTrue
+		} else {
+			klog.Warningf("Unrecognized condition %s=%s (%s: %s): cannot judge update retrieval success", u.Condition.Type, u.Condition.Status, u.Condition.Reason, u.Condition.Message)
+		}
 	}
 
 	optr.statusLock.Lock()
 	defer optr.statusLock.Unlock()
+	if u != nil && (optr.availableUpdates == nil ||
+		optr.availableUpdates.Upstream != u.Upstream ||
+		optr.availableUpdates.Channel != u.Channel ||
+		success) {
+		u.LastSyncOrConfigChange = u.LastAttempt
+	} else if optr.availableUpdates != nil {
+		u.LastSyncOrConfigChange = optr.availableUpdates.LastSyncOrConfigChange
+	}
 	optr.availableUpdates = u
 }
 

pkg/cvo/cvo_test.go

@@ -2581,10 +2581,10 @@ func TestOperator_availableUpdatesSync(t *testing.T) {
 			optr: Operator{
 				defaultUpstreamServer:      "http://localhost:8080/graph",
 				minimumUpdateCheckInterval: 1 * time.Minute,
-				availableUpdates: &availableUpdates{
-					Upstream: "http://localhost:8080/graph",
-					Channel:  "fast",
-					At:       time.Now(),
+				availableUpdates:    &availableUpdates{
+					Upstream:    "http://localhost:8080/graph",
+					Channel:     "fast",
+					LastAttempt: time.Now(),
 				},
 
 				releaseVersion: "4.0.1",
@@ -2658,7 +2658,8 @@ func TestOperator_availableUpdatesSync(t *testing.T) {
 					optr.availableUpdates.Upstream = "<default>"
 				}
 				optr.availableUpdates.Condition.LastTransitionTime = metav1.Time{}
-				optr.availableUpdates.At = time.Time{}
+				optr.availableUpdates.LastAttempt = time.Time{}
+				optr.availableUpdates.LastSyncOrConfigChange = time.Time{}
 			}
 			if !reflect.DeepEqual(optr.availableUpdates, tt.wantUpdates) {
 				t.Fatalf("unexpected: %s", diff.ObjectReflectDiff(tt.wantUpdates, optr.availableUpdates))