Merge pull request #87 from ironcladlou/manager-refactor

Simplify multi-namespace controller setup

Makefile

@@ -38,3 +38,7 @@ clean:
 verify:
 	hack/verify-gofmt.sh
 	hack/verify-generated-bindata.sh
+
+.PHONY: uninstall
+uninstall:
+	hack/uninstall.sh

hack/uninstall.sh

@@ -1,18 +1,26 @@
 #!/bin/bash
 set -uo pipefail
 
+WHAT="${WHAT:-all}"
+
 # Disable the CVO
 oc scale --replicas 0 -n openshift-cluster-version deployments/cluster-version-operator
 
 # Uninstall the cluster-ingress-operator
 oc delete -n openshift-ingress-operator deployments/ingress-operator
 oc patch -n openshift-ingress-operator clusteringresses/default --patch '{"metadata":{"finalizers": []}}' --type=merge
-oc delete -n openshift-ingress-operator clusteroperators.operatorstatus.openshift.io/openshift-ingress
+oc delete clusteroperator.config.openshift.io/openshift-ingress-operator
 oc delete --force --grace-period=0 -n openshift-ingress-operator clusteringresses/default
-oc delete namespaces/openshift-ingress-operator
+
+if [ "$WHAT" == "all" ]; then
+  oc delete namespaces/openshift-ingress-operator
+fi
 oc delete namespaces/openshift-ingress
-oc delete clusterroles/openshift-ingress-operator
-oc delete clusterroles/openshift-ingress-router
-oc delete clusterrolebindings/openshift-ingress-operator
-oc delete clusterrolebindings/openshift-ingress-router
-oc delete customresourcedefinition.apiextensions.k8s.io/clusteringresses.ingress.openshift.io
+
+if [ "$WHAT" == "all" ]; then
+  oc delete clusterroles/openshift-ingress-operator
+  oc delete clusterroles/openshift-ingress-router
+  oc delete clusterrolebindings/openshift-ingress-operator
+  oc delete clusterrolebindings/openshift-ingress-router
+  oc delete customresourcedefinition.apiextensions.k8s.io/clusteringresses.ingress.openshift.io
+fi

manifests/02-deployment.yaml

@@ -30,7 +30,5 @@ spec:
               valueFrom:
                 fieldRef:
                   fieldPath: metadata.namespace
-            - name: OPERATOR_NAME
-              value: ingress-operator
             - name: IMAGE
               value: openshift/origin-haproxy-router:v4.0

pkg/operator/controller/controller.go

@@ -9,7 +9,6 @@ import (
 	ingressv1alpha1 "github.com/openshift/cluster-ingress-operator/pkg/apis/ingress/v1alpha1"
 	"github.com/openshift/cluster-ingress-operator/pkg/dns"
 	"github.com/openshift/cluster-ingress-operator/pkg/manifests"
-	operatormanager "github.com/openshift/cluster-ingress-operator/pkg/operator/manager"
 	"github.com/openshift/cluster-ingress-operator/pkg/util/slice"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -22,6 +21,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 )
@@ -37,9 +37,9 @@ const (
 // controller that handles all the logic for implementing ingress based on
 // ClusterIngress resources.
 //
-// The controller will be pre-configured to watch events from a component
-// source provided by the manager.
-func New(mgr operatormanager.ComponentManager, config Config) (controller.Controller, error) {
+// The controller will be pre-configured to watch for ClusterIngress resources
+// in the manager namespace.
+func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 	reconciler := &reconciler{
 		Config: config,
 	}
@@ -51,10 +51,6 @@ func New(mgr operatormanager.ComponentManager, config Config) (controller.Contro
 	if err != nil {
 		return nil, err
 	}
-	err = c.Watch(mgr.ComponentSource(), &handler.EnqueueRequestForObject{})
-	if err != nil {
-		return nil, err
-	}
 	return c, nil
 }
 

pkg/operator/operator.go

@@ -9,7 +9,6 @@ import (
 	"github.com/openshift/cluster-ingress-operator/pkg/manifests"
 	operatorconfig "github.com/openshift/cluster-ingress-operator/pkg/operator/config"
 	operatorcontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller"
-	operatormanager "github.com/openshift/cluster-ingress-operator/pkg/operator/manager"
 	"github.com/openshift/cluster-ingress-operator/pkg/util"
 
 	configv1 "github.com/openshift/api/config/v1"
@@ -27,9 +26,12 @@ import (
 
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
 // scheme contains all the API types necessary for the operator's dynamic
@@ -56,6 +58,7 @@ type Operator struct {
 	client          client.Client
 
 	manager manager.Manager
+	caches  []cache.Cache
 }
 
 // New creates (but does not start) a new operator from configuration.
@@ -66,31 +69,17 @@ func New(config operatorconfig.Config, installConfig *util.InstallConfig, dnsMan
 	}
 	mf := manifests.NewFactory(config)
 
-	// Set up an operator manager with a component manager watching for resources
-	// in the router namespace. Any new namespaces or types the operator should
-	// react to will be added here.
-	components := []operatormanager.ComponentOptions{
-		{
-			Options: manager.Options{
-				Namespace: "openshift-ingress",
-				Scheme:    scheme,
-			},
-			Types: []runtime.Object{
-				&appsv1.Deployment{},
-				&corev1.Service{},
-			},
-		},
-	}
-	operatorManager, err := operatormanager.New(kubeConfig, manager.Options{
+	// Set up an operator manager for the operator namespace.
+	operatorManager, err := manager.New(kubeConfig, manager.Options{
 		Namespace: config.Namespace,
 		Scheme:    scheme,
-	}, components...)
+	})
 	if err != nil {
 		return nil, fmt.Errorf("failed to create operator manager: %v", err)
 	}
 
 	// Create and register the operator controller with the operator manager.
-	_, err = operatorcontroller.New(operatorManager, operatorcontroller.Config{
+	operatorController, err := operatorcontroller.New(operatorManager, operatorcontroller.Config{
 		Client:          kubeClient,
 		Namespace:       config.Namespace,
 		ManifestFactory: mf,
@@ -99,8 +88,33 @@ func New(config operatorconfig.Config, installConfig *util.InstallConfig, dnsMan
 	if err != nil {
 		return nil, fmt.Errorf("failed to create operator controller: %v", err)
 	}
+
+	// Create additional controller event sources from informers in the managed
+	// namespace. Any new managed resources outside the operator's namespace
+	// should be added here.
+	mapper, err := apiutil.NewDiscoveryRESTMapper(kubeConfig)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get API Group-Resources")
+	}
+	ingressCache, err := cache.New(kubeConfig, cache.Options{Namespace: "openshift-ingress", Scheme: scheme, Mapper: mapper})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create openshift-ingress cache: %v", err)
+	}
+
+	for _, obj := range []runtime.Object{
+		&appsv1.Deployment{},
+		&corev1.Service{},
+	} {
+		informer, err := ingressCache.GetInformer(obj)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create informer for %v: %v", obj, err)
+		}
+		operatorController.Watch(&source.Informer{Informer: informer}, &handler.EnqueueRequestForObject{})
+	}
+
 	return &Operator{
 		manager: operatorManager,
+		caches:  []cache.Cache{ingressCache},
 
 		// TODO: These are only needed for the default cluster ingress stuff, which
 		// should be refactored away.
@@ -119,8 +133,34 @@ func (o *Operator) Start(stop <-chan struct{}) error {
 		return fmt.Errorf("failed to ensure default cluster ingress: %v", err)
 	}
 
-	// Start the primary manager.
-	return o.manager.Start(stop)
+	errChan := make(chan error)
+
+	// Start secondary caches.
+	for _, cache := range o.caches {
+		go func() {
+			if err := cache.Start(stop); err != nil {
+				errChan <- err
+			}
+		}()
+		logrus.Infof("waiting for cache to sync")
+		if !cache.WaitForCacheSync(stop) {
+			return fmt.Errorf("failed to sync cache")
+		}
+		logrus.Infof("cache synced")
+	}
+
+	// Secondary caches are all synced, so start the manager.
+	go func() {
+		errChan <- o.manager.Start(stop)
+	}()
+
+	// Wait for the manager to exit or a secondary cache to fail.
+	select {
+	case <-stop:
+		return nil
+	case err := <-errChan:
+		return err
+	}
 }
 
 // ensureDefaultClusterIngress ensures that a default ClusterIngress exists.