improve default_servicecidr_controller startup

The default service-cidr controller blocks the apiserver because it
needs to create the default ServiceCIDR so Services can be allocated.

If the apiserver is started without the default ServiceCIDR any attempt
to createa  new Service will fail, and this is a breaking change for
users and installers that does not retry on this operation.

Instead of using a channel to signal the controller is ready, just
implement two loops, a first one that verifies that is ready and that
polls with a shorted interval, and leave the second loop with the
existing interval.

Change-Id: I54303af9faeaa9c5cce2a840b6b7b0320cd2f4ad

Author: aojea

pkg/controlplane/controller/defaultservicecidr/default_servicecidr_controller.go

@@ -82,8 +82,6 @@ func NewController(
 	c.eventBroadcaster = broadcaster
 	c.eventRecorder = recorder
 
-	c.readyCh = make(chan struct{})
-
 	return c
 }
 
@@ -99,8 +97,6 @@ type Controller struct {
 	serviceCIDRLister   networkingv1alpha1listers.ServiceCIDRLister
 	serviceCIDRsSynced  cache.InformerSynced
 
-	readyCh chan struct{} // channel to block until the default ServiceCIDR exists
-
 	interval time.Duration
 }
 
@@ -120,28 +116,40 @@ func (c *Controller) Start(stopCh <-chan struct{}) {
 		return
 	}
 
-	go wait.Until(c.sync, c.interval, stopCh)
+	// derive a context from the stopCh so we can cancel the poll loop
+	ctx := wait.ContextForChannel(stopCh)
+	// wait until first successfully sync
+	// this blocks apiserver startup so poll with a short interval
+	err := wait.PollUntilContextCancel(ctx, 100*time.Millisecond, true, func(ctx context.Context) (bool, error) {
+		syncErr := c.sync()
+		return syncErr == nil, nil
+	})
+	if err != nil {
+		klog.Infof("error initializing the default ServiceCIDR: %v", err)
 
-	select {
-	case <-stopCh:
-	case <-c.readyCh:
 	}
+
+	// run the sync loop in the background with the defined interval
+	go wait.Until(func() {
+		err := c.sync()
+		if err != nil {
+			klog.Infof("error trying to sync the default ServiceCIDR: %v", err)
+		}
+	}, c.interval, stopCh)
 }
 
-func (c *Controller) sync() {
+func (c *Controller) sync() error {
 	// check if the default ServiceCIDR already exist
 	serviceCIDR, err := c.serviceCIDRLister.Get(DefaultServiceCIDRName)
 	// if exists
 	if err == nil {
-		c.setReady()
 		c.syncStatus(serviceCIDR)
-		return
+		return nil
 	}
 
 	// unknown error
 	if !apierrors.IsNotFound(err) {
-		klog.Infof("error trying to obtain the default ServiceCIDR: %v", err)
-		return
+		return err
 	}
 
 	// default ServiceCIDR does not exist
@@ -156,21 +164,11 @@ func (c *Controller) sync() {
 	}
 	serviceCIDR, err = c.client.NetworkingV1alpha1().ServiceCIDRs().Create(context.Background(), serviceCIDR, metav1.CreateOptions{})
 	if err != nil && !apierrors.IsAlreadyExists(err) {
-		klog.Infof("error creating default ServiceCIDR: %v", err)
 		c.eventRecorder.Eventf(serviceCIDR, v1.EventTypeWarning, "KubernetesDefaultServiceCIDRError", "The default ServiceCIDR can not be created")
-		return
+		return err
 	}
-
-	c.setReady()
 	c.syncStatus(serviceCIDR)
-}
-
-func (c *Controller) setReady() {
-	select {
-	case <-c.readyCh:
-	default:
-		close(c.readyCh)
-	}
+	return nil
 }
 
 func (c *Controller) syncStatus(serviceCIDR *networkingapiv1alpha1.ServiceCIDR) {

pkg/controlplane/controller/defaultservicecidr/default_servicecidr_controller_test.go

@@ -56,7 +56,6 @@ func newController(t *testing.T, objects []*networkingapiv1alpha1.ServiceCIDR) (
 		eventRecorder:      record.NewFakeRecorder(100),
 		serviceCIDRLister:  serviceCIDRInformer.Lister(),
 		serviceCIDRsSynced: func() bool { return true },
-		readyCh:            make(chan struct{}),
 	}
 
 	return client, c