controllers: refactor code and start informers first, fix template/render race

cmd/common/helpers.go

@@ -1,10 +1,10 @@
 package common
 
 import (
-	"math/rand"
 	"os"
 	"time"
 
+	"github.com/openshift/machine-config-operator/internal/clients"
 	"github.com/golang/glog"
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -21,19 +21,10 @@ const (
 	RenewDeadline = 60 * time.Second
 	// RetryPeriod is the default duration for the leader electrion retrial.
 	RetryPeriod = 30 * time.Second
-
-	minResyncPeriod = 20 * time.Minute
 )
 
-func resyncPeriod() func() time.Duration {
-	return func() time.Duration {
-		factor := rand.Float64() + 1
-		return time.Duration(float64(minResyncPeriod.Nanoseconds()) * factor)
-	}
-}
-
 // CreateResourceLock returns an interface for the resource lock.
-func CreateResourceLock(cb *ClientBuilder, componentNamespace, componentName string) resourcelock.Interface {
+func CreateResourceLock(cb *clients.Builder, componentNamespace, componentName string) resourcelock.Interface {
 	recorder := record.
 		NewBroadcaster().
 		NewRecorder(runtime.NewScheme(), v1.EventSource{Component: componentName})

cmd/machine-config-controller/start.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/golang/glog"
 	"github.com/openshift/machine-config-operator/cmd/common"
+	"github.com/openshift/machine-config-operator/internal/clients"
+	controllercommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/openshift/machine-config-operator/pkg/controller/container-runtime-config"
 	"github.com/openshift/machine-config-operator/pkg/controller/kubelet-config"
 	"github.com/openshift/machine-config-operator/pkg/controller/node"
@@ -46,22 +48,25 @@ func runStartCmd(cmd *cobra.Command, args []string) {
 	// To help debugging, immediately log version
 	glog.Infof("Version: %+v", version.Version)
 
-	cb, err := common.NewClientBuilder(startOpts.kubeconfig)
+	cb, err := clients.NewBuilder(startOpts.kubeconfig)
 	if err != nil {
 		glog.Fatalf("error creating clients: %v", err)
 	}
 	run := func(ctx context.Context) {
-		ctrlctx := common.CreateControllerContext(cb, ctx.Done(), componentName)
-		if err := startControllers(ctrlctx); err != nil {
-			glog.Fatalf("error starting controllers: %v", err)
-		}
+		ctrlctx := controllercommon.CreateControllerContext(cb, ctx.Done(), componentName)
+
+		controllers := createControllers(ctrlctx)
 
 		// Start the shared factory informers that you need to use in your controller
 		ctrlctx.InformerFactory.Start(ctrlctx.Stop)
 		ctrlctx.KubeInformerFactory.Start(ctrlctx.Stop)
 		ctrlctx.ConfigInformerFactory.Start(ctrlctx.Stop)
 		close(ctrlctx.InformersStarted)
 
+		for _, c := range controllers {
+			go c.Run(2, ctrlctx.Stop)
+		}
+
 		select {}
 	}
 
@@ -80,53 +85,54 @@ func runStartCmd(cmd *cobra.Command, args []string) {
 	panic("unreachable")
 }
 
-func startControllers(ctx *common.ControllerContext) error {
-	// Our primary MCs come from here
-	go template.New(
-		rootOpts.templates,
-		ctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
-		ctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(),
-		ctx.ClientBuilder.KubeClientOrDie("template-controller"),
-		ctx.ClientBuilder.MachineConfigClientOrDie("template-controller"),
-	).Run(2, ctx.Stop)
-
-	// Add all "sub-renderers here"
-	go kubeletconfig.New(
-		rootOpts.templates,
-		ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
-		ctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
-		ctx.InformerFactory.Machineconfiguration().V1().KubeletConfigs(),
-		ctx.ClientBuilder.KubeClientOrDie("kubelet-config-controller"),
-		ctx.ClientBuilder.MachineConfigClientOrDie("kubelet-config-controller"),
-	).Run(2, ctx.Stop)
-
-	go containerruntimeconfig.New(
-		rootOpts.templates,
-		ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
-		ctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
-		ctx.InformerFactory.Machineconfiguration().V1().ContainerRuntimeConfigs(),
-		ctx.ConfigInformerFactory.Config().V1().Images(),
-		ctx.ClientBuilder.KubeClientOrDie("container-runtime-config-controller"),
-		ctx.ClientBuilder.MachineConfigClientOrDie("container-runtime-config-controller"),
-		ctx.ClientBuilder.ConfigClientOrDie("container-runtime-config-controller"),
-	).Run(2, ctx.Stop)
-
-	// The renderer creates "rendered" MCs from the MC fragments generated by
-	// the above sub-controllers, which are then consumed by the node controller
-	go render.New(
-		ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
-		ctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(),
-		ctx.ClientBuilder.KubeClientOrDie("render-controller"),
-		ctx.ClientBuilder.MachineConfigClientOrDie("render-controller"),
-	).Run(2, ctx.Stop)
-
-	// The node controller consumes data written by the above
-	go node.New(
-		ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
-		ctx.KubeInformerFactory.Core().V1().Nodes(),
-		ctx.ClientBuilder.KubeClientOrDie("node-update-controller"),
-		ctx.ClientBuilder.MachineConfigClientOrDie("node-update-controller"),
-	).Run(2, ctx.Stop)
-
-	return nil
+func createControllers(ctx *controllercommon.ControllerContext) []controllercommon.Controller {
+	var controllers []controllercommon.Controller
+
+	controllers = append(controllers,
+		// Our primary MCs come from here
+		template.New(
+			rootOpts.templates,
+			ctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
+			ctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(),
+			ctx.ClientBuilder.KubeClientOrDie("template-controller"),
+			ctx.ClientBuilder.MachineConfigClientOrDie("template-controller"),
+		),
+		// Add all "sub-renderers here"
+		kubeletconfig.New(
+			rootOpts.templates,
+			ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
+			ctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
+			ctx.InformerFactory.Machineconfiguration().V1().KubeletConfigs(),
+			ctx.ClientBuilder.KubeClientOrDie("kubelet-config-controller"),
+			ctx.ClientBuilder.MachineConfigClientOrDie("kubelet-config-controller"),
+		),
+		containerruntimeconfig.New(
+			rootOpts.templates,
+			ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
+			ctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
+			ctx.InformerFactory.Machineconfiguration().V1().ContainerRuntimeConfigs(),
+			ctx.ConfigInformerFactory.Config().V1().Images(),
+			ctx.ClientBuilder.KubeClientOrDie("container-runtime-config-controller"),
+			ctx.ClientBuilder.MachineConfigClientOrDie("container-runtime-config-controller"),
+			ctx.ClientBuilder.ConfigClientOrDie("container-runtime-config-controller"),
+		),
+		// The renderer creates "rendered" MCs from the MC fragments generated by
+		// the above sub-controllers, which are then consumed by the node controller
+		render.New(
+			ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
+			ctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(),
+			ctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
+			ctx.ClientBuilder.KubeClientOrDie("render-controller"),
+			ctx.ClientBuilder.MachineConfigClientOrDie("render-controller"),
+		),
+		// The node controller consumes data written by the above
+		node.New(
+			ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
+			ctx.KubeInformerFactory.Core().V1().Nodes(),
+			ctx.ClientBuilder.KubeClientOrDie("node-update-controller"),
+			ctx.ClientBuilder.MachineConfigClientOrDie("node-update-controller"),
+		),
+	)
+
+	return controllers
 }

cmd/machine-config-daemon/start.go

@@ -6,7 +6,8 @@ import (
 	"syscall"
 
 	"github.com/golang/glog"
-	"github.com/openshift/machine-config-operator/cmd/common"
+	"github.com/openshift/machine-config-operator/internal/clients"
+	controllercommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/openshift/machine-config-operator/pkg/daemon"
 	"github.com/openshift/machine-config-operator/pkg/version"
 	"github.com/spf13/cobra"
@@ -82,7 +83,7 @@ func runStartCmd(cmd *cobra.Command, args []string) {
 	defer close(exitCh)
 
 	var dn *daemon.Daemon
-	var ctx *common.ControllerContext
+	var ctx *controllercommon.ControllerContext
 
 	glog.Info("starting node writer")
 	nodeWriter := daemon.NewNodeWriter()
@@ -108,11 +109,11 @@ func runStartCmd(cmd *cobra.Command, args []string) {
 		}
 		// Else we use the cluster driven daemon
 	} else {
-		cb, err := common.NewClientBuilder(startOpts.kubeconfig)
+		cb, err := clients.NewBuilder(startOpts.kubeconfig)
 		if err != nil {
 			glog.Fatalf("failed to initialize ClientBuilder: %v", err)
 		}
-		ctx = common.CreateControllerContext(cb, stopCh, componentName)
+		ctx = controllercommon.CreateControllerContext(cb, stopCh, componentName)
 		// create the daemon instance. this also initializes kube client items
 		// which need to come from the container and not the chroot.
 		dn, err = daemon.NewClusterDrivenDaemon(

cmd/machine-config-operator/start.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/golang/glog"
 	"github.com/openshift/machine-config-operator/cmd/common"
+	"github.com/openshift/machine-config-operator/internal/clients"
+	controllercommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/openshift/machine-config-operator/pkg/operator"
 	"github.com/openshift/machine-config-operator/pkg/version"
 	"github.com/spf13/cobra"
@@ -43,15 +45,35 @@ func runStartCmd(cmd *cobra.Command, args []string) {
 		glog.Fatal("--images-json cannot be empty")
 	}
 
-	cb, err := common.NewClientBuilder(startOpts.kubeconfig)
+	cb, err := clients.NewBuilder(startOpts.kubeconfig)
 	if err != nil {
 		glog.Fatalf("error creating clients: %v", err)
 	}
 	run := func(ctx context.Context) {
-		ctrlctx := common.CreateControllerContext(cb, ctx.Done(), componentNamespace)
-		if err := startControllers(ctrlctx); err != nil {
-			glog.Fatalf("error starting controllers: %v", err)
-		}
+		ctrlctx := controllercommon.CreateControllerContext(cb, ctx.Done(), componentNamespace)
+
+		controller := operator.New(
+			componentNamespace, componentName,
+			startOpts.imagesFile,
+			ctrlctx.NamespacedInformerFactory.Machineconfiguration().V1().MCOConfigs(),
+			ctrlctx.NamespacedInformerFactory.Machineconfiguration().V1().MachineConfigPools(),
+			ctrlctx.NamespacedInformerFactory.Machineconfiguration().V1().ControllerConfigs(),
+			ctrlctx.NamespacedInformerFactory.Machineconfiguration().V1().MachineConfigs(),
+			ctrlctx.NamespacedInformerFactory.Machineconfiguration().V1().ControllerConfigs(),
+			ctrlctx.KubeNamespacedInformerFactory.Core().V1().ServiceAccounts(),
+			ctrlctx.APIExtInformerFactory.Apiextensions().V1beta1().CustomResourceDefinitions(),
+			ctrlctx.KubeNamespacedInformerFactory.Apps().V1().Deployments(),
+			ctrlctx.KubeNamespacedInformerFactory.Apps().V1().DaemonSets(),
+			ctrlctx.KubeNamespacedInformerFactory.Rbac().V1().ClusterRoles(),
+			ctrlctx.KubeNamespacedInformerFactory.Rbac().V1().ClusterRoleBindings(),
+			ctrlctx.KubeNamespacedInformerFactory.Core().V1().ConfigMaps(),
+			ctrlctx.ConfigInformerFactory.Config().V1().Infrastructures(),
+			ctrlctx.ConfigInformerFactory.Config().V1().Networks(),
+			ctrlctx.ClientBuilder.MachineConfigClientOrDie(componentName),
+			ctrlctx.ClientBuilder.KubeClientOrDie(componentName),
+			ctrlctx.ClientBuilder.APIExtClientOrDie(componentName),
+			ctrlctx.ClientBuilder.ConfigClientOrDie(componentName),
+		)
 
 		ctrlctx.NamespacedInformerFactory.Start(ctrlctx.Stop)
 		ctrlctx.KubeInformerFactory.Start(ctrlctx.Stop)
@@ -60,6 +82,8 @@ func runStartCmd(cmd *cobra.Command, args []string) {
 		ctrlctx.ConfigInformerFactory.Start(ctrlctx.Stop)
 		close(ctrlctx.InformersStarted)
 
+		go controller.Run(2, ctrlctx.Stop)
+
 		select {}
 	}
 
@@ -77,30 +101,3 @@ func runStartCmd(cmd *cobra.Command, args []string) {
 	})
 	panic("unreachable")
 }
-
-func startControllers(ctx *common.ControllerContext) error {
-	go operator.New(
-		componentNamespace, componentName,
-		startOpts.imagesFile,
-		ctx.NamespacedInformerFactory.Machineconfiguration().V1().MCOConfigs(),
-		ctx.NamespacedInformerFactory.Machineconfiguration().V1().MachineConfigPools(),
-		ctx.NamespacedInformerFactory.Machineconfiguration().V1().ControllerConfigs(),
-		ctx.NamespacedInformerFactory.Machineconfiguration().V1().MachineConfigs(),
-		ctx.NamespacedInformerFactory.Machineconfiguration().V1().ControllerConfigs(),
-		ctx.KubeNamespacedInformerFactory.Core().V1().ServiceAccounts(),
-		ctx.APIExtInformerFactory.Apiextensions().V1beta1().CustomResourceDefinitions(),
-		ctx.KubeNamespacedInformerFactory.Apps().V1().Deployments(),
-		ctx.KubeNamespacedInformerFactory.Apps().V1().DaemonSets(),
-		ctx.KubeNamespacedInformerFactory.Rbac().V1().ClusterRoles(),
-		ctx.KubeNamespacedInformerFactory.Rbac().V1().ClusterRoleBindings(),
-		ctx.KubeNamespacedInformerFactory.Core().V1().ConfigMaps(),
-		ctx.ConfigInformerFactory.Config().V1().Infrastructures(),
-		ctx.ConfigInformerFactory.Config().V1().Networks(),
-		ctx.ClientBuilder.MachineConfigClientOrDie(componentName),
-		ctx.ClientBuilder.KubeClientOrDie(componentName),
-		ctx.ClientBuilder.APIExtClientOrDie(componentName),
-		ctx.ClientBuilder.ConfigClientOrDie(componentName),
-	).Run(2, ctx.Stop)
-
-	return nil
-}

cmd/common/client_builder.go → internal/clients/builder.go

@@ -1,47 +1,46 @@
-package common
+package clients
 
 import (
 	"os"
 
 	"github.com/golang/glog"
 	configclientset "github.com/openshift/client-go/config/clientset/versioned"
+	mcfgclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned"
 	apiext "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
-
-	mcfgclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned"
 )
 
-// ClientBuilder can create a variety of kubernetes client interface
+// Builder can create a variety of kubernetes client interface
 // with its embeded rest.Config.
-type ClientBuilder struct {
+type Builder struct {
 	config *rest.Config
 }
 
 // MachineConfigClientOrDie returns the kubernetes client interface for machine config.
-func (cb *ClientBuilder) MachineConfigClientOrDie(name string) mcfgclientset.Interface {
+func (cb *Builder) MachineConfigClientOrDie(name string) mcfgclientset.Interface {
 	return mcfgclientset.NewForConfigOrDie(rest.AddUserAgent(cb.config, name))
 }
 
 // KubeClientOrDie returns the kubernetes client interface for general kubernetes objects.
-func (cb *ClientBuilder) KubeClientOrDie(name string) kubernetes.Interface {
+func (cb *Builder) KubeClientOrDie(name string) kubernetes.Interface {
 	return kubernetes.NewForConfigOrDie(rest.AddUserAgent(cb.config, name))
 }
 
 // ConfigClientOrDie returns the kubernetes client interface for security related kubernetes objects
 // such as pod security policy, security context.
-func (cb *ClientBuilder) ConfigClientOrDie(name string) configclientset.Interface {
+func (cb *Builder) ConfigClientOrDie(name string) configclientset.Interface {
 	return configclientset.NewForConfigOrDie(rest.AddUserAgent(cb.config, name))
 }
 
 // APIExtClientOrDie returns the kubernetes client interface for extended kubernetes objects.
-func (cb *ClientBuilder) APIExtClientOrDie(name string) apiext.Interface {
+func (cb *Builder) APIExtClientOrDie(name string) apiext.Interface {
 	return apiext.NewForConfigOrDie(rest.AddUserAgent(cb.config, name))
 }
 
-// NewClientBuilder returns a *ClientBuilder with the given kubeconfig.
-func NewClientBuilder(kubeconfig string) (*ClientBuilder, error) {
+// NewBuilder returns a *ClientBuilder with the given kubeconfig.
+func NewBuilder(kubeconfig string) (*Builder, error) {
 	var config *rest.Config
 	var err error
 
@@ -60,7 +59,7 @@ func NewClientBuilder(kubeconfig string) (*ClientBuilder, error) {
 		return nil, err
 	}
 
-	return &ClientBuilder{
+	return &Builder{
 		config: config,
 	}, nil
 }