From 3f5088ef8557cd9192a5372ea7cd29d2114c3374 Mon Sep 17 00:00:00 2001
From: David Eads <deads@redhat.com>
Date: Mon, 21 May 2018 11:18:19 -0400
Subject: [PATCH] make the docker registry secret always prime

---
 .../controllers/docker_registry_service.go    | 11 ++++-----
 .../docker_registry_service_test.go           | 23 +++++++++++++++----
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/pkg/serviceaccounts/controllers/docker_registry_service.go b/pkg/serviceaccounts/controllers/docker_registry_service.go
index 1837b0a909b6..d70172c04433 100644
--- a/pkg/serviceaccounts/controllers/docker_registry_service.go
+++ b/pkg/serviceaccounts/controllers/docker_registry_service.go
@@ -103,7 +103,7 @@ type DockerRegistryServiceController struct {
 	serviceLister  listers.ServiceLister
 	servicesSynced func() bool
 
-	syncRegistryLocationHandler func(key string) error
+	syncRegistryLocationHandler func() error
 
 	secretCache       cache.Store
 	secretsSynced     func() bool
@@ -157,10 +157,9 @@ func (e *DockerRegistryServiceController) waitForDockerURLs(ready chan<- struct{
 		return
 	}
 
-	// after syncing, determine the current state and assume that we're up to date for it if you don't do this,
-	// you'll get an initial storm as you mess with all the dockercfg secrets every time you startup
+	// after syncing, make sure the dockercfgController is up to date before releasing it
+	// this controller will do a single scan of all existing secrets to be sure that they're up to date
 	urls := e.getDockerRegistryLocations()
-	e.setRegistryURLs(urls...)
 	e.dockercfgController.SetDockerURLs(urls...)
 	close(e.dockerURLsInitialized)
 	close(ready)
@@ -190,7 +189,7 @@ func (e *DockerRegistryServiceController) watchForDockerURLChanges() {
 		}
 		defer e.registryLocationQueue.Done(key)
 
-		if err := e.syncRegistryLocationHandler(key.(string)); err == nil {
+		if err := e.syncRegistryLocationHandler(); err == nil {
 			// this means the request was successfully handled.  We should "forget" the item so that any retry
 			// later on is reset
 			e.registryLocationQueue.Forget(key)
@@ -238,7 +237,7 @@ func getDockerRegistryLocations(lister listers.ServiceLister, location serviceLo
 }
 
 // syncRegistryLocationChange goes through all service account dockercfg secrets and updates them to point at a new docker-registry location
-func (e *DockerRegistryServiceController) syncRegistryLocationChange(key string) error {
+func (e *DockerRegistryServiceController) syncRegistryLocationChange() error {
 	newLocations := e.getDockerRegistryLocations()
 	newDockerRegistryLocations := sets.NewString(newLocations...)
 	existingURLs := e.getRegistryURLs()
diff --git a/pkg/serviceaccounts/controllers/docker_registry_service_test.go b/pkg/serviceaccounts/controllers/docker_registry_service_test.go
index fe8c93a31dfe..88c27376222b 100644
--- a/pkg/serviceaccounts/controllers/docker_registry_service_test.go
+++ b/pkg/serviceaccounts/controllers/docker_registry_service_test.go
@@ -63,7 +63,20 @@ func controllerSetup(startingObjects []runtime.Object, t *testing.T, stopCh <-ch
 	return kubeclient, fakeWatch, controller, informerFactory
 }
 
-func wrapHandler(indicator chan bool, handler func(string) error, t *testing.T) func(string) error {
+func wrapHandler(indicator chan bool, handler func() error, t *testing.T) func() error {
+	return func() error {
+		defer func() { indicator <- true }()
+
+		err := handler()
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+
+		return err
+	}
+}
+
+func wrapStringHandler(indicator chan bool, handler func(string) error, t *testing.T) func(string) error {
 	return func(key string) error {
 		defer func() { indicator <- true }()
 
@@ -129,7 +142,7 @@ func TestUpdateNewStyleSecret(t *testing.T) {
 
 	kubeclient, fakeWatch, controller, informerFactory := controllerSetup([]runtime.Object{newStyleDockercfgSecret}, t, stopChannel)
 	controller.syncRegistryLocationHandler = wrapHandler(received, controller.syncRegistryLocationChange, t)
-	controller.syncSecretHandler = wrapHandler(updatedSecret, controller.syncSecretUpdate, t)
+	controller.syncSecretHandler = wrapStringHandler(updatedSecret, controller.syncSecretUpdate, t)
 	informerFactory.Start(stopChannel)
 	go controller.Run(5, stopChannel)
 
@@ -218,7 +231,7 @@ func TestUpdateOldStyleSecretWithKey(t *testing.T) {
 
 	kubeclient, fakeWatch, controller, informerFactory := controllerSetup([]runtime.Object{oldStyleDockercfgSecret}, t, stopChannel)
 	controller.syncRegistryLocationHandler = wrapHandler(received, controller.syncRegistryLocationChange, t)
-	controller.syncSecretHandler = wrapHandler(updatedSecret, controller.syncSecretUpdate, t)
+	controller.syncSecretHandler = wrapStringHandler(updatedSecret, controller.syncSecretUpdate, t)
 	informerFactory.Start(stopChannel)
 	go controller.Run(5, stopChannel)
 
@@ -309,7 +322,7 @@ func TestUpdateOldStyleSecretWithoutKey(t *testing.T) {
 		return true, tokenSecret, nil
 	})
 	controller.syncRegistryLocationHandler = wrapHandler(received, controller.syncRegistryLocationChange, t)
-	controller.syncSecretHandler = wrapHandler(updatedSecret, controller.syncSecretUpdate, t)
+	controller.syncSecretHandler = wrapStringHandler(updatedSecret, controller.syncSecretUpdate, t)
 	informerFactory.Start(stopChannel)
 	go controller.Run(5, stopChannel)
 
@@ -400,7 +413,7 @@ func TestClearSecretAndRecreate(t *testing.T) {
 
 	kubeclient, fakeWatch, controller, informerFactory := controllerSetup([]runtime.Object{registryService, oldStyleDockercfgSecret}, t, stopChannel)
 	controller.syncRegistryLocationHandler = wrapHandler(received, controller.syncRegistryLocationChange, t)
-	controller.syncSecretHandler = wrapHandler(updatedSecret, controller.syncSecretUpdate, t)
+	controller.syncSecretHandler = wrapStringHandler(updatedSecret, controller.syncSecretUpdate, t)
 	informerFactory.Start(stopChannel)
 	go controller.Run(5, stopChannel)