Merge pull request #36581 from wojtek-t/fix_service_alloc

pkg/master/controller.go

@@ -45,6 +45,7 @@ import (
 // loops, which manage creating the "kubernetes" service, the "default" and "kube-system"
 // namespace, and provide the IP repair check on service IPs
 type Controller struct {
+	ServiceClient     coreclient.ServicesGetter
 	NamespaceRegistry namespace.Registry
 	ServiceRegistry   service.Registry
 
@@ -75,8 +76,9 @@ type Controller struct {
 }
 
 // NewBootstrapController returns a controller for watching the core capabilities of the master
-func (c *Config) NewBootstrapController(legacyRESTStorage corerest.LegacyRESTStorage) *Controller {
+func (c *Config) NewBootstrapController(legacyRESTStorage corerest.LegacyRESTStorage, serviceClient coreclient.ServicesGetter) *Controller {
 	return &Controller{
+		ServiceClient:     serviceClient,
 		NamespaceRegistry: legacyRESTStorage.NamespaceRegistry,
 		ServiceRegistry:   legacyRESTStorage.ServiceRegistry,
 
@@ -240,12 +242,12 @@ func createEndpointPortSpec(endpointPort int, endpointPortName string, extraEndp
 // doesn't already exist.
 func (c *Controller) CreateOrUpdateMasterServiceIfNeeded(serviceName string, serviceIP net.IP, servicePorts []api.ServicePort, serviceType api.ServiceType, reconcile bool) error {
 	ctx := api.NewDefaultContext()
-	if s, err := c.ServiceRegistry.GetService(ctx, serviceName); err == nil {
+	if s, err := c.ServiceClient.Services(api.NamespaceDefault).Get(serviceName); err == nil {
 		// The service already exists.
 		if reconcile {
 			if svc, updated := getMasterServiceUpdateIfNeeded(s, servicePorts, serviceType); updated {
 				glog.Warningf("Resetting master service %q to %#v", serviceName, svc)
-				_, err := c.ServiceRegistry.UpdateService(ctx, svc)
+				_, err := c.ServiceClient.Services(api.NamespaceDefault).Update(svc)
 				return err
 			}
 		}
@@ -270,7 +272,7 @@ func (c *Controller) CreateOrUpdateMasterServiceIfNeeded(serviceName string, ser
 		return err
 	}
 
-	_, err := c.ServiceRegistry.CreateService(ctx, svc)
+	_, err := c.ServiceClient.Services(api.NamespaceDefault).Create(svc)
 	if err != nil && errors.IsAlreadyExists(err) {
 		err = nil
 	}

pkg/master/controller_test.go

@@ -582,15 +582,26 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 			Err: errors.New("unable to get svc"),
 		}
 		master.ServiceRegistry = registry
+		fakeClient := fake.NewSimpleClientset()
+		master.ServiceClient = fakeClient.Core()
 		master.CreateOrUpdateMasterServiceIfNeeded(test.serviceName, net.ParseIP("1.2.3.4"), test.servicePorts, test.serviceType, false)
+		creates := []core.CreateAction{}
+		for _, action := range fakeClient.Actions() {
+			if action.GetVerb() == "create" {
+				creates = append(creates, action.(core.CreateAction))
+			}
+		}
 		if test.expectCreate != nil {
-			if len(registry.List.Items) != 1 {
-				t.Errorf("case %q: unexpected creations: %v", test.testName, registry.List.Items)
-			} else if e, a := test.expectCreate.Spec, registry.List.Items[0].Spec; !reflect.DeepEqual(e, a) {
-				t.Errorf("case %q: expected create:\n%#v\ngot:\n%#v\n", test.testName, e, a)
+			if len(creates) != 1 {
+				t.Errorf("case %q: unexpected creations: %v", test.testName, creates)
+			} else {
+				obj := creates[0].GetObject()
+				if e, a := test.expectCreate.Spec, obj.(*api.Service).Spec; !reflect.DeepEqual(e, a) {
+					t.Errorf("case %q: expected create:\n%#v\ngot:\n%#v\n", test.testName, e, a)
+				}
 			}
 		}
-		if test.expectCreate == nil && len(registry.List.Items) > 1 {
+		if test.expectCreate == nil && len(creates) > 1 {
 			t.Errorf("case %q: no create expected, yet saw: %v", test.testName, registry.List.Items)
 		}
 	}
@@ -857,18 +868,29 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 			Service: test.service,
 		}
 		master.ServiceRegistry = registry
+		fakeClient := fake.NewSimpleClientset(test.service)
+		master.ServiceClient = fakeClient.Core()
 		err := master.CreateOrUpdateMasterServiceIfNeeded(test.serviceName, net.ParseIP("1.2.3.4"), test.servicePorts, test.serviceType, true)
 		if err != nil {
 			t.Errorf("case %q: unexpected error: %v", test.testName, err)
 		}
+		updates := []core.UpdateAction{}
+		for _, action := range fakeClient.Actions() {
+			if action.GetVerb() == "update" {
+				updates = append(updates, action.(core.UpdateAction))
+			}
+		}
 		if test.expectUpdate != nil {
-			if len(registry.Updates) != 1 {
-				t.Errorf("case %q: unexpected updates: %v", test.testName, registry.Updates)
-			} else if e, a := test.expectUpdate, &registry.Updates[0]; !reflect.DeepEqual(e, a) {
-				t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a)
+			if len(updates) != 1 {
+				t.Errorf("case %q: unexpected updates: %v", test.testName, updates)
+			} else {
+				obj := updates[0].GetObject()
+				if e, a := test.expectUpdate.Spec, obj.(*api.Service).Spec; !reflect.DeepEqual(e, a) {
+					t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a)
+				}
 			}
 		}
-		if test.expectUpdate == nil && len(registry.Updates) > 0 {
+		if test.expectUpdate == nil && len(updates) > 0 {
 			t.Errorf("case %q: no update expected, yet saw: %v", test.testName, registry.Updates)
 		}
 	}
@@ -909,18 +931,29 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 			Service: test.service,
 		}
 		master.ServiceRegistry = registry
+		fakeClient := fake.NewSimpleClientset(test.service)
+		master.ServiceClient = fakeClient.Core()
 		err := master.CreateOrUpdateMasterServiceIfNeeded(test.serviceName, net.ParseIP("1.2.3.4"), test.servicePorts, test.serviceType, false)
 		if err != nil {
 			t.Errorf("case %q: unexpected error: %v", test.testName, err)
 		}
+		updates := []core.UpdateAction{}
+		for _, action := range fakeClient.Actions() {
+			if action.GetVerb() == "update" {
+				updates = append(updates, action.(core.UpdateAction))
+			}
+		}
 		if test.expectUpdate != nil {
-			if len(registry.Updates) != 1 {
-				t.Errorf("case %q: unexpected updates: %v", test.testName, registry.Updates)
-			} else if e, a := test.expectUpdate, &registry.Updates[0]; !reflect.DeepEqual(e, a) {
-				t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a)
+			if len(updates) != 1 {
+				t.Errorf("case %q: unexpected updates: %v", test.testName, updates)
+			} else {
+				obj := updates[0].GetObject()
+				if e, a := test.expectUpdate.Spec, obj.(*api.Service).Spec; !reflect.DeepEqual(e, a) {
+					t.Errorf("case %q: expected update:\n%#v\ngot:\n%#v\n", test.testName, e, a)
+				}
 			}
 		}
-		if test.expectUpdate == nil && len(registry.Updates) > 0 {
+		if test.expectUpdate == nil && len(updates) > 0 {
 			t.Errorf("case %q: no update expected, yet saw: %v", test.testName, registry.Updates)
 		}
 	}

pkg/master/master.go

@@ -271,7 +271,8 @@ func (m *Master) InstallLegacyAPI(c *Config, restOptionsGetter genericapiserver.
 	}
 
 	if c.EnableCoreControllers {
-		bootstrapController := c.NewBootstrapController(legacyRESTStorage)
+		serviceClient := coreclient.NewForConfigOrDie(c.GenericConfig.LoopbackClientConfig)
+		bootstrapController := c.NewBootstrapController(legacyRESTStorage, serviceClient)
 		if err := m.GenericAPIServer.AddPostStartHook("bootstrap-controller", bootstrapController.PostStartHook); err != nil {
 			glog.Fatalf("Error registering PostStartHook %q: %v", "bootstrap-controller", err)
 		}