pkg/cvo: Surface errors in update-risk detector creation

vendor/k8s.io/client-go/tools/cache/shared_informer.go has:

  func (s *sharedIndexInformer) AddEventHandlerWithOptions(handler ResourceEventHandler, options HandlerOptions) (ResourceEventHandlerRegistration, error) {
  	s.startedLock.Lock()
  	defer s.startedLock.Unlock()

  	if s.stopped {
  		return nil, fmt.Errorf("handler %v was not added to shared informer because it has stopped already", handler)
  	}

which might be the only reason AddEeventHandler could fail.  But
regardless, it is still good practice to bubble things up and fail
fast, to summon a cluster administrator to help sort out whatever the
issue is.

Author: wking

pkg/cvo/cvo.go

@@ -305,13 +305,31 @@ func New(
 
 	// make sure this is initialized after all the listers are initialized
 	riskSourceCallback := func() { optr.availableUpdatesQueue.Add(optr.queueKey()) }
-	optr.upgradeable = aggregate.New(
-		updatingrisk.New("ClusterVersionUpdating", optr.name, cvInformer, riskSourceCallback),
-		overrides.New("ClusterVersionOverrides", optr.name, cvInformer, riskSourceCallback),
-		deletionrisk.New("ResourceDeletionInProgress", optr.currentVersion),
-		adminack.New("AdminAck", optr.currentVersion, cmConfigManagedInformer, cmConfigInformer, riskSourceCallback),
-		upgradeablerisk.New("ClusterOperatorUpgradeable", optr.currentVersion, coInformer, riskSourceCallback),
-	)
+
+	risks := []riskSource{}
+	if source, err := updatingrisk.New("ClusterVersionUpdating", optr.name, cvInformer, riskSourceCallback); err != nil {
+		return optr, err
+	} else {
+		risks = append(risks, source)
+	}
+	if source, err := overrides.New("ClusterVersionOverrides", optr.name, cvInformer, riskSourceCallback); err != nil {
+		return optr, err
+	} else {
+		risks = append(risks, source)
+	}
+	risks = append(risks, deletionrisk.New("ResourceDeletionInProgress", optr.currentVersion))
+	if source, err := adminack.New("AdminAck", optr.currentVersion, cmConfigManagedInformer, cmConfigInformer, riskSourceCallback); err != nil {
+		return optr, err
+	} else {
+		risks = append(risks, source)
+	}
+	if source, err := upgradeablerisk.New("ClusterOperatorUpgradeable", optr.currentVersion, coInformer, riskSourceCallback); err != nil {
+		return optr, err
+	} else {
+		risks = append(risks, source)
+	}
+
+	optr.upgradeable = aggregate.New(risks...)
 
 	optr.risks = aggregate.New(
 		alert.New("Alert", promqlTarget),

pkg/risk/adminack/adminack.go

@@ -4,6 +4,7 @@ package adminack
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"regexp"
 	"slices"
@@ -41,21 +42,21 @@ type adminAck struct {
 }
 
 // New returns a new update-risk source, tracking administrator acknowledgements.
-func New(name string, currentVersion func() configv1.Release, adminGatesInformer, adminAcksInformer informerscorev1.ConfigMapInformer, changeCallback func()) risk.Source {
+func New(name string, currentVersion func() configv1.Release, adminGatesInformer, adminAcksInformer informerscorev1.ConfigMapInformer, changeCallback func()) (risk.Source, error) {
 	adminGatesLister := adminGatesInformer.Lister().ConfigMaps(internal.ConfigManagedNamespace)
 	adminAcksLister := adminAcksInformer.Lister().ConfigMaps(internal.ConfigNamespace)
 	source := &adminAck{name: name, currentVersion: currentVersion, adminGatesLister: adminGatesLister, adminAcksLister: adminAcksLister}
-	adminGatesInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+	_, err1 := adminGatesInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    func(_ interface{}) { source.eventHandler(changeCallback) },
 		UpdateFunc: func(_, _ interface{}) { source.eventHandler(changeCallback) },
 		DeleteFunc: func(_ interface{}) { source.eventHandler(changeCallback) },
 	})
-	adminAcksInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+	_, err2 := adminAcksInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    func(_ interface{}) { source.eventHandler(changeCallback) },
 		UpdateFunc: func(_, _ interface{}) { source.eventHandler(changeCallback) },
 		DeleteFunc: func(_ interface{}) { source.eventHandler(changeCallback) },
 	})
-	return source
+	return source, errors.Join(err1, err2)
 }
 
 // Name returns the source's name.

pkg/risk/adminack/adminack_test.go

@@ -285,7 +285,10 @@ func TestOperator_upgradeableSync(t *testing.T) {
 	for _, tt := range tests {
 		{
 			t.Run(tt.name, func(t *testing.T) {
-				source := New("AdminAck", func() configv1.Release { return configv1.Release{Version: "4.21.0"} }, cmInformer, cmInformer, nil)
+				source, err := New("AdminAck", func() configv1.Release { return configv1.Release{Version: "4.21.0"} }, cmInformer, cmInformer, nil)
+				if err != nil {
+					t.Fatal(err)
+				}
 				if tt.gateCm != nil {
 					if tt.gateCm.Name == "delete" {
 						err := f.CoreV1().ConfigMaps(internal.ConfigManagedNamespace).Delete(ctx, internal.AdminGatesConfigMap, metav1.DeleteOptions{})

pkg/risk/overrides/overrides.go

@@ -29,14 +29,14 @@ type overrides struct {
 }
 
 // New returns a new update-risk source, tracking the use of ClusterVersion overrides.
-func New(name string, cvName string, cvInformer configinformersv1.ClusterVersionInformer, changeCallback func()) risk.Source {
+func New(name string, cvName string, cvInformer configinformersv1.ClusterVersionInformer, changeCallback func()) (risk.Source, error) {
 	cvLister := cvInformer.Lister()
 	source := &overrides{name: name, cvName: cvName, cvLister: cvLister}
-	cvInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+	_, err := cvInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    func(_ interface{}) { source.eventHandler(changeCallback) },
 		UpdateFunc: func(_, _ interface{}) { source.eventHandler(changeCallback) },
 	})
-	return source
+	return source, err
 }
 
 // Name returns the source's name.

pkg/risk/overrides/overrides_test.go

@@ -42,7 +42,10 @@ func Test_New(t *testing.T) {
 	versions := []string{"4.21.1", "4.22.0", "5.0.0"}
 
 	expectedName := "ClusterVersionOverrides"
-	source := overrides.New(expectedName, cv.ObjectMeta.Name, cvInformer, changeCallback)
+	source, err := overrides.New(expectedName, cv.ObjectMeta.Name, cvInformer, changeCallback)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Run("name", func(t *testing.T) {
 		if name := source.Name(); name != expectedName {

pkg/risk/updating/updating.go

@@ -30,14 +30,14 @@ type updating struct {
 }
 
 // New returns a new updating-risk source, tracking whether an update is in progress.
-func New(name string, cvName string, cvInformer configinformersv1.ClusterVersionInformer, changeCallback func()) risk.Source {
+func New(name string, cvName string, cvInformer configinformersv1.ClusterVersionInformer, changeCallback func()) (risk.Source, error) {
 	cvLister := cvInformer.Lister()
 	source := &updating{name: name, cvName: cvName, cvLister: cvLister}
-	cvInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+	_, err := cvInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    func(_ interface{}) { source.eventHandler(changeCallback) },
 		UpdateFunc: func(_, _ interface{}) { source.eventHandler(changeCallback) },
 	})
-	return source
+	return source, err
 }
 
 // Name returns the source's name.

pkg/risk/updating/updating_test.go

@@ -43,7 +43,10 @@ func Test_New(t *testing.T) {
 	versions := []string{"4.21.1", "4.22.0", "5.0.0"}
 
 	expectedName := "ClusterVersionUpdating"
-	source := updating.New(expectedName, cv.ObjectMeta.Name, cvInformer, changeCallback)
+	source, err := updating.New(expectedName, cv.ObjectMeta.Name, cvInformer, changeCallback)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Run("name", func(t *testing.T) {
 		if name := source.Name(); name != expectedName {

pkg/risk/upgradeable/upgradeable.go

@@ -30,14 +30,14 @@ type upgradeable struct {
 }
 
 // New returns a new update-risk source, tracking ClusterOperator Upgradeable conditions.
-func New(name string, currentVersion func() configv1.Release, coInformer configinformersv1.ClusterOperatorInformer, changeCallback func()) risk.Source {
+func New(name string, currentVersion func() configv1.Release, coInformer configinformersv1.ClusterOperatorInformer, changeCallback func()) (risk.Source, error) {
 	coLister := coInformer.Lister()
 	source := &upgradeable{name: name, currentVersion: currentVersion, coLister: coLister}
-	coInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+	_, err := coInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    func(_ interface{}) { source.eventHandler(changeCallback) },
 		UpdateFunc: func(_, _ interface{}) { source.eventHandler(changeCallback) },
 	})
-	return source
+	return source, err
 }
 
 // Name returns the source's name.

pkg/risk/upgradeable/upgradeable_test.go

@@ -51,7 +51,10 @@ func Test_New(t *testing.T) {
 	versions := []string{"4.21.1", "4.22.0", "5.0.0"}
 
 	expectedName := "ClusterOperatorUpgradeable"
-	source := upgradeable.New(expectedName, currentVersion, coInformer, changeCallback)
+	source, err := upgradeable.New(expectedName, currentVersion, coInformer, changeCallback)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Run("name", func(t *testing.T) {
 		if name := source.Name(); name != expectedName {