machineDeployment controller races+panics on startup when a machineSet exists

[alvaroaleman]

Currently, the machineDeployment controller sometimes panics on startup when a machineSet already exists:

ERROR: logging before flag.Parse: I0913 19:43:11.940900       1 controller.go:83] Waiting for caches to sync for machine deployment controller
ERROR: logging before flag.Parse: E0913 19:43:12.533934       1 runtime.go:66] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)
/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:72
/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:65
/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:51
/usr/local/go/src/runtime/asm_amd64.s:573
/usr/local/go/src/runtime/panic.go:505
/usr/local/go/src/runtime/panic.go:63
/usr/local/go/src/runtime/signal_unix.go:388
/go/src/sigs.k8s.io/cluster-api/pkg/controller/machinedeployment/controller.go:372
/go/src/sigs.k8s.io/cluster-api/pkg/controller/machinedeployment/controller.go:194
/go/src/sigs.k8s.io/cluster-api/pkg/controller/machinedeployment/controller.go:66
/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache/controller.go:195
/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache/shared_informer.go:547
/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache/shared_informer.go:389
/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache/shared_informer.go:389
/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:71
/usr/local/go/src/runtime/asm_amd64.s:2361
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1268cd6]

goroutine 82 [running]:
sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/runtime.HandleCrash(0x0, 0x0, 0x0)
	/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:58 +0x107
panic(0x13ede20, 0x212c150)
	/usr/local/go/src/runtime/panic.go:505 +0x229
sigs.k8s.io/cluster-api/pkg/controller/machinedeployment.(*MachineDeploymentControllerImpl).enqueue(0xc4204155e0, 0xc420446000)
	/go/src/sigs.k8s.io/cluster-api/pkg/controller/machinedeployment/controller.go:372 +0x146
sigs.k8s.io/cluster-api/pkg/controller/machinedeployment.(*MachineDeploymentControllerImpl).addMachineSet(0xc4204155e0, 0x15b0ea0, 0xc42005ec00)
	/go/src/sigs.k8s.io/cluster-api/pkg/controller/machinedeployment/controller.go:194 +0x1aa
sigs.k8s.io/cluster-api/pkg/controller/machinedeployment.(*MachineDeploymentControllerImpl).(sigs.k8s.io/cluster-api/pkg/controller/machinedeployment.addMachineSet)-fm(0x15b0ea0, 0xc42005ec00)
	/go/src/sigs.k8s.io/cluster-api/pkg/controller/machinedeployment/controller.go:66 +0x3e
sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache.ResourceEventHandlerFuncs.OnAdd(0xc42036d390, 0xc42036d3a0, 0xc42036d3b0, 0x15b0ea0, 0xc42005ec00)
	/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache/controller.go:195 +0x49
sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache.(*processorListener).run(0xc4202ee180)
	/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache/shared_informer.go:547 +0x247
sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache.(*processorListener).(sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache.run)-fm()
	/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/client-go/tools/cache/shared_informer.go:389 +0x2a
sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1(0xc42015fe20, 0xc42036dc60)
	/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:71 +0x4f
created by sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/wait.(*Group).Start
	/go/src/sigs.k8s.io/cluster-api/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:69 +0x62

If I read the code correctly, this is caused by the machineDeployment controller adding an EventHandler for Adds of machineSets in its Init which subsequently causes all machineDeployments that own a machineSet to get enqeued - However the queue gets created after the Init is called:

cluster-api/pkg/controller/machinedeployment/zz_generated.api.register.go

Lines 59 to 71 in ba672fe

	if i, ok := ci.(sharedinformers.LegacyControllerInit); ok {
	i.Init(config, si, c.LookupAndReconcile)
	} else if i, ok := ci.(sharedinformers.ControllerInit); ok {
	i.Init(&sharedinformers.ControllerInitArgumentsImpl{si, config, c.LookupAndReconcile})
	}
	c.controller = uc
	queue.Reconcile = c.reconcile
	if c.Informers.WorkerQueues == nil {
	c.Informers.WorkerQueues = map[string]*controller.QueueWorker{}
	}
	c.Informers.WorkerQueues["MachineDeployment"] = queue

So if the initial List of machineSets is fast enough, this results in a NPE.