update docker config secret to include image-registry.openshift-image-registry.svc

deads2k · deads2k · commit 7d51eaca7978 · 2018-04-25T12:57:54.000-04:00
diff --git a/pkg/cmd/openshift-controller-manager/controller/serviceaccount.go b/pkg/cmd/openshift-controller-manager/controller/serviceaccount.go
@@ -68,8 +68,6 @@ func RunServiceAccountPullSecretsController(ctx ControllerContext) (bool, error)
 	go dockercfgController.Run(5, ctx.Stop)
 
 	dockerRegistryControllerOptions := serviceaccountcontrollers.DockerRegistryServiceControllerOptions{
-		RegistryNamespace:     "default",
-		RegistryServiceName:   "docker-registry",
 		DockercfgController:   dockercfgController,
 		DockerURLsInitialized: dockerURLsInitialized,
 	}
diff --git a/pkg/serviceaccounts/controllers/docker_registry_service.go b/pkg/serviceaccounts/controllers/docker_registry_service.go
@@ -31,9 +31,6 @@ type DockerRegistryServiceControllerOptions struct {
 	// If zero, re-list will be delayed as long as possible
 	Resync time.Duration
 
-	RegistryNamespace   string
-	RegistryServiceName string
-
 	DockercfgController *DockercfgController
 
 	// DockerURLsInitialized is used to send a signal to the DockercfgController that it has the correct set of docker urls
@@ -43,29 +40,43 @@ type DockerRegistryServiceControllerOptions struct {
 // NewDockerRegistryServiceController returns a new *DockerRegistryServiceController.
 func NewDockerRegistryServiceController(secrets informers.SecretInformer, cl kclientset.Interface, options DockerRegistryServiceControllerOptions) *DockerRegistryServiceController {
 	e := &DockerRegistryServiceController{
-		client:                cl,
-		dockercfgController:   options.DockercfgController,
-		registryLocationQueue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
-		secretsToUpdate:       workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
-		serviceName:           options.RegistryServiceName,
-		serviceNamespace:      options.RegistryNamespace,
-		dockerURLsInitialized: options.DockerURLsInitialized,
+		client:                 cl,
+		dockercfgController:    options.DockercfgController,
+		registryLocationQueue:  workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
+		secretsToUpdate:        workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
+		legacyServiceLocation:  serviceLocation{namespace: "default", name: "docker-registry"},
+		currentServiceLocation: serviceLocation{namespace: "openshift-image-registry", name: "image-registry"},
+		dockerURLsInitialized:  options.DockerURLsInitialized,
 	}
 
 	// does not use shared informers because we're only watching one item
-	e.serviceCache, e.serviceController = cache.NewInformer(
+	e.legacyServiceCache = newServiceCache(e, e.legacyServiceLocation, options.Resync)
+	e.currentServiceCache = newServiceCache(e, e.currentServiceLocation, options.Resync)
+
+	e.syncRegistryLocationHandler = e.syncRegistryLocationChange
+
+	e.secretCache = secrets.Informer().GetIndexer()
+	e.secretsSynced = secrets.Informer().GetController().HasSynced
+	e.syncSecretHandler = e.syncSecretUpdate
+
+	return e
+}
+
+func newServiceCache(e *DockerRegistryServiceController, location serviceLocation, resync time.Duration) serviceCache {
+	ret := serviceCache{}
+	ret.serviceCache, ret.serviceController = cache.NewInformer(
 		&cache.ListWatch{
 			ListFunc: func(opts metav1.ListOptions) (runtime.Object, error) {
-				opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", options.RegistryServiceName).String()
-				return e.client.Core().Services(options.RegistryNamespace).List(opts)
+				opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", location.name).String()
+				return e.client.Core().Services(location.namespace).List(opts)
 			},
 			WatchFunc: func(opts metav1.ListOptions) (watch.Interface, error) {
-				opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", options.RegistryServiceName).String()
-				return e.client.Core().Services(options.RegistryNamespace).Watch(opts)
+				opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", location.name).String()
+				return e.client.Core().Services(location.namespace).Watch(opts)
 			},
 		},
 		&v1.Service{},
-		options.Resync,
+		resync,
 		cache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
 				e.enqueueRegistryLocationQueue()
@@ -78,28 +89,34 @@ func NewDockerRegistryServiceController(secrets informers.SecretInformer, cl kcl
 			},
 		},
 	)
-	e.servicesSynced = e.serviceController.HasSynced
-	e.syncRegistryLocationHandler = e.syncRegistryLocationChange
+	ret.servicesSynced = ret.serviceController.HasSynced
 
-	e.secretCache = secrets.Informer().GetIndexer()
-	e.secretsSynced = secrets.Informer().GetController().HasSynced
-	e.syncSecretHandler = e.syncSecretUpdate
+	return ret
+}
 
-	return e
+type serviceLocation struct {
+	namespace string
+	name      string
+}
+
+type serviceCache struct {
+	serviceController cache.Controller
+	serviceCache      cache.Store
+	servicesSynced    func() bool
 }
 
 // DockerRegistryServiceController manages ServiceToken secrets for Service objects
 type DockerRegistryServiceController struct {
 	client kclientset.Interface
 
-	serviceName      string
-	serviceNamespace string
+	legacyServiceLocation  serviceLocation
+	currentServiceLocation serviceLocation
 
 	dockercfgController *DockercfgController
 
-	serviceController           cache.Controller
-	serviceCache                cache.Store
-	servicesSynced              func() bool
+	legacyServiceCache  serviceCache
+	currentServiceCache serviceCache
+
 	syncRegistryLocationHandler func(key string) error
 
 	secretCache       cache.Store
@@ -119,7 +136,8 @@ func (e *DockerRegistryServiceController) Run(workers int, stopCh <-chan struct{
 	defer utilruntime.HandleCrash()
 	defer e.registryLocationQueue.ShutDown()
 
-	go e.serviceController.Run(stopCh)
+	go e.legacyServiceCache.serviceController.Run(stopCh)
+	go e.currentServiceCache.serviceController.Run(stopCh)
 
 	// Wait for the store to sync before starting any work in this controller.
 	ready := make(chan struct{})
@@ -152,7 +170,7 @@ func (e *DockerRegistryServiceController) waitForDockerURLs(ready chan<- struct{
 	defer utilruntime.HandleCrash()
 
 	// Wait for the stores to fill
-	if !cache.WaitForCacheSync(stopCh, e.servicesSynced, e.secretsSynced) {
+	if !cache.WaitForCacheSync(stopCh, e.legacyServiceCache.servicesSynced, e.currentServiceCache.servicesSynced, e.secretsSynced) {
 		return
 	}
 
@@ -212,12 +230,19 @@ func (e *DockerRegistryServiceController) watchForDockerURLChanges() {
 
 // getDockerRegistryLocations returns the dns form and the ip form of the secret
 func (e *DockerRegistryServiceController) getDockerRegistryLocations() []string {
-	key, err := controller.KeyFunc(&v1.Service{ObjectMeta: metav1.ObjectMeta{Name: e.serviceName, Namespace: e.serviceNamespace}})
+	ret := []string{}
+	ret = append(ret, getDockerRegistryLocations(e.legacyServiceCache, e.legacyServiceLocation)...)
+	ret = append(ret, getDockerRegistryLocations(e.currentServiceCache, e.currentServiceLocation)...)
+	return nil
+}
+
+func getDockerRegistryLocations(cache serviceCache, location serviceLocation) []string {
+	key, err := controller.KeyFunc(&v1.Service{ObjectMeta: metav1.ObjectMeta{Name: location.name, Namespace: location.namespace}})
 	if err != nil {
 		return []string{}
 	}
 
-	obj, exists, err := e.serviceCache.GetByKey(key)
+	obj, exists, err := cache.serviceCache.GetByKey(key)
 	if err != nil {
 		return []string{}
 	}
@@ -239,7 +264,8 @@ func (e *DockerRegistryServiceController) getDockerRegistryLocations() []string
 
 // 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 {
-	newDockerRegistryLocations := sets.NewString(e.getDockerRegistryLocations()...)
+	newLocations := e.getDockerRegistryLocations()
+	newDockerRegistryLocations := sets.NewString(newLocations...)
 	if e.getRegistryURLs().Equal(newDockerRegistryLocations) {
 		glog.V(4).Infof("No effective update: %v", newDockerRegistryLocations)
 		return nil
diff --git a/pkg/serviceaccounts/controllers/docker_registry_service_test.go b/pkg/serviceaccounts/controllers/docker_registry_service_test.go
@@ -19,8 +19,8 @@ import (
 )
 
 const (
-	registryNamespace = "ns"
-	registryName      = "registry"
+	registryNamespace = "default"
+	registryName      = "docker-registry"
 )
 
 var (
@@ -42,7 +42,14 @@ func controllerSetup(startingObjects []runtime.Object, t *testing.T, stopCh <-ch
 	kubeclient.PrependReactor("update", "*", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
 		return true, action.(clientgotesting.UpdateAction).GetObject(), nil
 	})
-	kubeclient.PrependWatchReactor("services", clientgotesting.DefaultWatchReactor(fakeWatch, nil))
+	kubeclient.PrependWatchReactor("services",
+		func(action clientgotesting.Action) (handled bool, ret watch.Interface, err error) {
+			if action.GetNamespace() != registryNamespace {
+				return true, watch.NewFake(), nil
+			}
+
+			return true, fakeWatch, nil
+		})
 
 	informerFactory := informers.NewSharedInformerFactory(kubeclient, controller.NoResyncPeriodFunc())
 
@@ -51,8 +58,6 @@ func controllerSetup(startingObjects []runtime.Object, t *testing.T, stopCh <-ch
 		kubeclient,
 		DockerRegistryServiceControllerOptions{
 			Resync:                10 * time.Minute,
-			RegistryNamespace:     registryNamespace,
-			RegistryServiceName:   registryName,
 			DockercfgController:   &DockercfgController{},
 			DockerURLsInitialized: make(chan struct{}),
 		},
@@ -153,7 +158,7 @@ func TestUpdateNewStyleSecret(t *testing.T) {
 	}
 
 	expectedDockercfgMap := credentialprovider.DockerConfig{}
-	for _, key := range []string{"172.16.123.123:1235", "registry.ns.svc:1235"} {
+	for _, key := range []string{"172.16.123.123:1235", "docker-registry.default.svc:1235"} {
 		expectedDockercfgMap[key] = credentialprovider.DockerConfigEntry{
 			Username: "serviceaccount",
 			Password: newStyleDockercfgSecret.Annotations[ServiceAccountTokenValueAnnotation],
@@ -243,7 +248,7 @@ func TestUpdateOldStyleSecretWithKey(t *testing.T) {
 	}
 
 	expectedDockercfgMap := credentialprovider.DockerConfig{}
-	for _, key := range []string{"172.16.123.123:1235", "registry.ns.svc:1235"} {
+	for _, key := range []string{"172.16.123.123:1235", "docker-registry.default.svc:1235"} {
 		expectedDockercfgMap[key] = credentialprovider.DockerConfigEntry{
 			Username: "serviceaccount",
 			Password: "token-value",
@@ -334,7 +339,7 @@ func TestUpdateOldStyleSecretWithoutKey(t *testing.T) {
 	}
 
 	expectedDockercfgMap := credentialprovider.DockerConfig{}
-	for _, key := range []string{"172.16.123.123:1235", "registry.ns.svc:1235"} {
+	for _, key := range []string{"172.16.123.123:1235", "docker-registry.default.svc:1235"} {
 		expectedDockercfgMap[key] = credentialprovider.DockerConfigEntry{
 			Username: "serviceaccount",
 			Password: "the-sa-bearer-token",
@@ -463,7 +468,7 @@ func TestClearSecretAndRecreate(t *testing.T) {
 	}
 
 	expectedDockercfgMap := credentialprovider.DockerConfig{}
-	for _, key := range []string{"172.16.123.123:1235", "registry.ns.svc:1235"} {
+	for _, key := range []string{"172.16.123.123:1235", "docker-registry.default.svc:1235"} {
 		expectedDockercfgMap[key] = credentialprovider.DockerConfigEntry{
 			Username: "serviceaccount",
 			Password: "the-token",

Original file line number	Diff line number	Diff line change
`@@ -68,8 +68,6 @@ func RunServiceAccountPullSecretsController(ctx ControllerContext) (bool, error)`
`68`	`68`	`go dockercfgController.Run(5, ctx.Stop)`
`69`	`69`
`70`	`70`	`dockerRegistryControllerOptions := serviceaccountcontrollers.DockerRegistryServiceControllerOptions{`
`71`		`- RegistryNamespace: "default",`
`72`		`- RegistryServiceName: "docker-registry",`
`73`	`71`	`DockercfgController: dockercfgController,`
`74`	`72`	`DockerURLsInitialized: dockerURLsInitialized,`
`75`	`73`	`}`