Merge pull request #1189 from petr-muller/ota-1531-04-extract-informer-factories

OTA-1531: [4/x] cvo: extract config informer creation

Author: openshift-merge-bot[bot]

pkg/start/start.go

@@ -13,15 +13,14 @@ import (
 	"time"
 
 	"github.com/google/uuid"
-	configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/informers"
+	coreinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
 	coreclientsetv1 "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -35,9 +34,10 @@ import (
 
 	configv1 "github.com/openshift/api/config/v1"
 	clientset "github.com/openshift/client-go/config/clientset/versioned"
-	"github.com/openshift/client-go/config/informers/externalversions"
+	configinformers "github.com/openshift/client-go/config/informers/externalversions"
+	configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
 	operatorclientset "github.com/openshift/client-go/operator/clientset/versioned"
-	operatorexternalversions "github.com/openshift/client-go/operator/informers/externalversions"
+	operatorinformers "github.com/openshift/client-go/operator/informers/externalversions"
 	"github.com/openshift/library-go/pkg/config/clusterstatus"
 	libgoleaderelection "github.com/openshift/library-go/pkg/config/leaderelection"
 
@@ -208,14 +208,16 @@ func (o *Options) Run(ctx context.Context) error {
 	case err != nil:
 		klog.Warningf("Failed to read release metadata to determine OCP version for this CVO (will use placeholder version %q): %v", cvoOcpVersion, err)
 	case releaseMetadata.Version == "":
-		klog.Warningf("Version missing from release metadata, cannot determine OCP version for this CVO (will use placeholder version %q): %v", cvoOcpVersion, err)
+		klog.Warningf("Version missing from release metadata, cannot determine OCP version for this CVO (will use placeholder version %q)", cvoOcpVersion)
 	default:
 		cvoOcpVersion = releaseMetadata.Version
 		klog.Infof("Determined OCP version for this CVO: %q", cvoOcpVersion)
 	}
 
+	clusterVersionConfigInformerFactory, configInformerFactory := o.prepareConfigInformerFactories(cb)
+
 	// initialize the controllers and attempt to load the payload information
-	controllerCtx, err := o.NewControllerContext(cb)
+	controllerCtx, err := o.NewControllerContext(cb, clusterVersionConfigInformerFactory, configInformerFactory)
 	if err != nil {
 		return err
 	}
@@ -224,6 +226,17 @@ func (o *Options) Run(ctx context.Context) error {
 	return nil
 }
 
+func (o *Options) prepareConfigInformerFactories(cb *ClientBuilder) (configinformers.SharedInformerFactory, configinformers.SharedInformerFactory) {
+	client := cb.ClientOrDie("shared-informer")
+	filterByName := func(opts *metav1.ListOptions) {
+		opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", o.Name).String()
+	}
+	clusterVersionConfigInformerFactory := configinformers.NewSharedInformerFactoryWithOptions(client, resyncPeriod(o.ResyncInterval), configinformers.WithTweakListOptions(filterByName))
+	configInformerFactory := configinformers.NewSharedInformerFactory(client, resyncPeriod(o.ResyncInterval))
+
+	return clusterVersionConfigInformerFactory, configInformerFactory
+}
+
 // run launches a number of goroutines to handle manifest application,
 // metrics serving, etc.  It continues operating until ctx.Done(),
 // and then attempts a clean shutdown limited by an internal context
@@ -255,13 +268,13 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource
 
 	informersDone := postMainContext.Done()
 	// FIXME: would be nice if there was a way to collect these.
-	controllerCtx.CVInformerFactory.Start(informersDone)
+	controllerCtx.ClusterVersionInformerFactory.Start(informersDone)
 	controllerCtx.OpenshiftConfigInformerFactory.Start(informersDone)
 	controllerCtx.OpenshiftConfigManagedInformerFactory.Start(informersDone)
-	controllerCtx.InformerFactory.Start(informersDone)
+	controllerCtx.ConfigInformerFactory.Start(informersDone)
 	controllerCtx.OperatorInformerFactory.Start(informersDone)
 
-	allSynced := controllerCtx.CVInformerFactory.WaitForCacheSync(informersDone)
+	allSynced := controllerCtx.ClusterVersionInformerFactory.WaitForCacheSync(informersDone)
 	for _, synced := range allSynced {
 		if !synced {
 			klog.Fatalf("Caches never synchronized: %v", postMainContext.Err())
@@ -489,34 +502,39 @@ type Context struct {
 	AutoUpdate              *autoupdate.Controller
 	StopOnFeatureGateChange *featuregates.ChangeStopper
 
-	CVInformerFactory                     externalversions.SharedInformerFactory
-	OpenshiftConfigInformerFactory        informers.SharedInformerFactory
-	OpenshiftConfigManagedInformerFactory informers.SharedInformerFactory
-	InformerFactory                       externalversions.SharedInformerFactory
-	OperatorInformerFactory               operatorexternalversions.SharedInformerFactory
+	// ClusterVersionInformerFactory should be used to get informers / listers for code that works with ClusterVersion resource
+	// singleton in the cluster.
+	ClusterVersionInformerFactory configinformers.SharedInformerFactory
+	// ConfigInformerFactory should be used to get informers / listers for code that works with resources from the
+	// config.openshift.io group, _except_ the ClusterVersion resource singleton.
+	ConfigInformerFactory configinformers.SharedInformerFactory
+	// OpenshiftConfigManagedInformerFactory should be used to get informers / listers for code that works with core k8s
+	// resources in the openshift-config namespace.
+	OpenshiftConfigInformerFactory coreinformers.SharedInformerFactory
+	// OpenshiftConfigManagedInformerFactory should be used to get informers / listers for code that works with core k8s
+	// resources in the openshift-config-managed namespace.
+	OpenshiftConfigManagedInformerFactory coreinformers.SharedInformerFactory
+	// OperatorInformerFactory should be used to get informers / listers for code that works with resources from the
+	// operator.openshift.io group
+	OperatorInformerFactory operatorinformers.SharedInformerFactory
 
 	fgLister configlistersv1.FeatureGateLister
 }
 
 // NewControllerContext initializes the default Context for the current Options. It does
 // not start any background processes.
-func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
-	client := cb.ClientOrDie("shared-informer")
+func (o *Options) NewControllerContext(cb *ClientBuilder, clusterVersionConfigInformerFactory, configInformerFactory configinformers.SharedInformerFactory) (*Context, error) {
 	kubeClient := cb.KubeClientOrDie(internal.ConfigNamespace, useProtobuf)
-	operatorClient := cb.OperatorClientOrDie("operator-client")
+	openshiftConfigInformerFactory := coreinformers.NewSharedInformerFactoryWithOptions(kubeClient, resyncPeriod(o.ResyncInterval), coreinformers.WithNamespace(internal.ConfigNamespace))
+	openshiftConfigManagedInformerFactory := coreinformers.NewSharedInformerFactoryWithOptions(kubeClient, resyncPeriod(o.ResyncInterval), coreinformers.WithNamespace(internal.ConfigManagedNamespace))
 
-	cvInformer := externalversions.NewFilteredSharedInformerFactory(client, resyncPeriod(o.ResyncInterval), "", func(opts *metav1.ListOptions) {
-		opts.FieldSelector = fmt.Sprintf("metadata.name=%s", o.Name)
-	})
-	openshiftConfigInformer := informers.NewSharedInformerFactoryWithOptions(kubeClient, resyncPeriod(o.ResyncInterval), informers.WithNamespace(internal.ConfigNamespace))
-	openshiftConfigManagedInformer := informers.NewSharedInformerFactoryWithOptions(kubeClient, resyncPeriod(o.ResyncInterval), informers.WithNamespace(internal.ConfigManagedNamespace))
-	sharedInformers := externalversions.NewSharedInformerFactory(client, resyncPeriod(o.ResyncInterval))
-	operatorInformerFactory := operatorexternalversions.NewSharedInformerFactoryWithOptions(operatorClient, o.ResyncInterval,
-		operatorexternalversions.WithTweakListOptions(func(opts *metav1.ListOptions) {
-			opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", configuration.ClusterVersionOperatorConfigurationName).String()
-		}))
+	operatorClient := cb.OperatorClientOrDie("operator-client")
+	filterByName := func(opts *metav1.ListOptions) {
+		opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", configuration.ClusterVersionOperatorConfigurationName).String()
+	}
+	operatorInformerFactory := operatorinformers.NewSharedInformerFactoryWithOptions(operatorClient, o.ResyncInterval, operatorinformers.WithTweakListOptions(filterByName))
 
-	coInformer := sharedInformers.Config().V1().ClusterOperators()
+	coInformer := configInformerFactory.Config().V1().ClusterOperators()
 
 	cvoKubeClient := cb.KubeClientOrDie(o.Namespace, useProtobuf)
 	o.PromQLTarget.KubeClient = cvoKubeClient
@@ -527,11 +545,11 @@ func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
 		o.ReleaseImage,
 		o.PayloadOverride,
 		resyncPeriod(o.ResyncInterval),
-		cvInformer.Config().V1().ClusterVersions(),
+		clusterVersionConfigInformerFactory.Config().V1().ClusterVersions(),
 		coInformer,
-		openshiftConfigInformer.Core().V1().ConfigMaps(),
-		openshiftConfigManagedInformer.Core().V1().ConfigMaps(),
-		sharedInformers.Config().V1().Proxies(),
+		openshiftConfigInformerFactory.Core().V1().ConfigMaps(),
+		openshiftConfigManagedInformerFactory.Core().V1().ConfigMaps(),
+		configInformerFactory.Config().V1().Proxies(),
 		operatorInformerFactory,
 		cb.ClientOrDie(o.Namespace),
 		cvoKubeClient,
@@ -548,28 +566,28 @@ func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
 		return nil, err
 	}
 
-	featureChangeStopper, err := featuregates.NewChangeStopper(sharedInformers.Config().V1().FeatureGates())
+	featureChangeStopper, err := featuregates.NewChangeStopper(configInformerFactory.Config().V1().FeatureGates())
 	if err != nil {
 		return nil, err
 	}
 
 	ctx := &Context{
-		CVInformerFactory:                     cvInformer,
-		OpenshiftConfigInformerFactory:        openshiftConfigInformer,
-		OpenshiftConfigManagedInformerFactory: openshiftConfigManagedInformer,
-		InformerFactory:                       sharedInformers,
+		ClusterVersionInformerFactory:         clusterVersionConfigInformerFactory,
+		ConfigInformerFactory:                 configInformerFactory,
+		OpenshiftConfigInformerFactory:        openshiftConfigInformerFactory,
+		OpenshiftConfigManagedInformerFactory: openshiftConfigManagedInformerFactory,
 		OperatorInformerFactory:               operatorInformerFactory,
 		CVO:                                   cvo,
 		StopOnFeatureGateChange:               featureChangeStopper,
 
-		fgLister: sharedInformers.Config().V1().FeatureGates().Lister(),
+		fgLister: configInformerFactory.Config().V1().FeatureGates().Lister(),
 	}
 
 	if o.EnableAutoUpdate {
 		ctx.AutoUpdate, err = autoupdate.New(
 			o.Namespace, o.Name,
-			cvInformer.Config().V1().ClusterVersions(),
-			sharedInformers.Config().V1().ClusterOperators(),
+			clusterVersionConfigInformerFactory.Config().V1().ClusterVersions(),
+			configInformerFactory.Config().V1().ClusterOperators(),
 			cb.ClientOrDie(o.Namespace),
 			cb.KubeClientOrDie(o.Namespace),
 		)

pkg/start/start_integration_test.go

@@ -188,7 +188,9 @@ func TestIntegrationCVO_initializeAndUpgrade(t *testing.T) {
 	if err := options.ValidateAndComplete(); err != nil {
 		t.Fatalf("incorrectly initialized options: %v", err)
 	}
-	controllers, err := options.NewControllerContext(cb)
+
+	cvif, cif := options.prepareConfigInformerFactories(cb)
+	controllers, err := options.NewControllerContext(cb, cvif, cif)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -323,7 +325,9 @@ func TestIntegrationCVO_gracefulStepDown(t *testing.T) {
 	if err := options.ValidateAndComplete(); err != nil {
 		t.Fatalf("incorrectly initialized options: %v", err)
 	}
-	controllers, err := options.NewControllerContext(cb)
+
+	cvif, cif := options.prepareConfigInformerFactories(cb)
+	controllers, err := options.NewControllerContext(cb, cvif, cif)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -520,7 +524,9 @@ metadata:
 	if err := options.ValidateAndComplete(); err != nil {
 		t.Fatalf("incorrectly initialized options: %v", err)
 	}
-	controllers, err := options.NewControllerContext(cb)
+
+	cvif, cif := options.prepareConfigInformerFactories(cb)
+	controllers, err := options.NewControllerContext(cb, cvif, cif)
 	if err != nil {
 		t.Fatal(err)
 	}