router: simplify detection of initial sync

Author: marun

pkg/router/controller/controller.go

@@ -79,12 +79,6 @@ func (c *RouterController) handleFirstSync() bool {
 		return false
 	}
 
-	err := c.Plugin.SetSyncedAtLeastOnce()
-	if err != nil {
-		utilruntime.HandleError(err)
-		return false
-	}
-
 	// If either of the event queues were empty after the initial
 	// List, the tracking listConsumed variable's default value of
 	// 'false' may prevent the router from committing the readiness

pkg/router/controller/controller_test.go

@@ -32,10 +32,6 @@ func (p *fakeRouterPlugin) Commit() error {
 	return nil
 }
 
-func (p *fakeRouterPlugin) SetSyncedAtLeastOnce() error {
-	return nil
-}
-
 type fakeNamespaceLister struct {
 }
 

pkg/router/controller/extended_validator.go

@@ -82,7 +82,3 @@ func (p *ExtendedValidator) HandleNamespaces(namespaces sets.String) error {
 func (p *ExtendedValidator) Commit() error {
 	return p.plugin.Commit()
 }
-
-func (p *ExtendedValidator) SetSyncedAtLeastOnce() error {
-	return p.plugin.SetSyncedAtLeastOnce()
-}

pkg/router/controller/host_admitter.go

@@ -152,10 +152,6 @@ func (p *HostAdmitter) Commit() error {
 	return p.plugin.Commit()
 }
 
-func (p *HostAdmitter) SetSyncedAtLeastOnce() error {
-	return p.plugin.SetSyncedAtLeastOnce()
-}
-
 // addRoute admits routes based on subdomain ownership - returns errors if the route is not admitted.
 func (p *HostAdmitter) addRoute(route *routeapi.Route) error {
 	// Find displaced routes (or error if an existing route displaces us)

pkg/router/controller/status.go

@@ -315,7 +315,3 @@ func (a *StatusAdmitter) HandleNamespaces(namespaces sets.String) error {
 func (a *StatusAdmitter) Commit() error {
 	return a.plugin.Commit()
 }
-
-func (a *StatusAdmitter) SetSyncedAtLeastOnce() error {
-	return a.plugin.SetSyncedAtLeastOnce()
-}

pkg/router/controller/status_test.go

@@ -43,10 +43,6 @@ func (p *fakePlugin) Commit() error {
 	return fmt.Errorf("not expected")
 }
 
-func (p *fakePlugin) SetSyncedAtLeastOnce() error {
-	return fmt.Errorf("not expected")
-}
-
 func TestStatusNoOp(t *testing.T) {
 	now := nowFn()
 	touched := unversioned.Time{Time: now.Add(-time.Minute)}

pkg/router/controller/unique_host.go

@@ -259,10 +259,6 @@ func (p *UniqueHost) Commit() error {
 	return p.plugin.Commit()
 }
 
-func (p *UniqueHost) SetSyncedAtLeastOnce() error {
-	return p.plugin.SetSyncedAtLeastOnce()
-}
-
 // routeKeys returns the internal router key to use for the given Route.
 func routeKeys(route *routeapi.Route) []string {
 	keys := make([]string, 1+len(route.Spec.AlternateBackends))

pkg/router/f5/plugin.go

@@ -633,8 +633,3 @@ func (p *F5Plugin) HandleRoute(eventType watch.EventType,
 func (p *F5Plugin) Commit() error {
 	return nil
 }
-
-// No-op since f5 has its own concept of what 'ready' means
-func (p *F5Plugin) SetSyncedAtLeastOnce() error {
-	return nil
-}

pkg/router/interfaces.go

@@ -17,5 +17,4 @@ type Plugin interface {
 	HandleNamespaces(namespaces sets.String) error
 	HandleNode(watch.EventType, *kapi.Node) error
 	Commit() error
-	SetSyncedAtLeastOnce() error
 }

pkg/router/template/plugin.go

@@ -83,9 +83,6 @@ type routerInterface interface {
 	// Commit applies the changes in the background. It kicks off a rate-limited
 	// commit (persist router state + refresh the backend) that coalesces multiple changes.
 	Commit()
-
-	// SetSyncedAtLeastOnce indicates to the router that state has been read from the api at least once
-	SetSyncedAtLeastOnce()
 }
 
 func env(name, defaultValue string) string {
@@ -227,11 +224,6 @@ func (p *TemplatePlugin) Commit() error {
 	return nil
 }
 
-func (p *TemplatePlugin) SetSyncedAtLeastOnce() error {
-	p.Router.SetSyncedAtLeastOnce()
-	return nil
-}
-
 // routeKeys returns the internal router keys to use for the given Route.
 // A route can have several services that it can point to, now
 func routeKeys(route *routeapi.Route) ([]string, []int32) {

pkg/router/template/plugin_test.go

@@ -245,9 +245,6 @@ func (r *TestRouter) Commit() {
 	r.Committed = true
 }
 
-func (r *TestRouter) SetSyncedAtLeastOnce() {
-}
-
 // TestHandleEndpoints test endpoint watch events
 func TestHandleEndpoints(t *testing.T) {
 	testCases := []struct {

pkg/router/template/router.go

@@ -87,7 +87,7 @@ type templateRouter struct {
 	// If true, haproxy should only bind ports when it has route and endpoint state
 	bindPortsAfterSync bool
 	// whether the router state has been read from the api at least once
-	syncedAtLeastOnce bool
+	synced bool
 	// whether a state change has occurred
 	stateChanged bool
 }
@@ -351,6 +351,12 @@ func (r *templateRouter) Commit() {
 	r.lock.Lock()
 	defer r.lock.Unlock()
 
+	if !r.synced {
+		glog.V(4).Infof("Router state synchronized for the first time")
+		r.synced = true
+		r.stateChanged = true
+	}
+
 	if r.stateChanged {
 		r.rateLimitedCommitFunction.Invoke(r.rateLimitedCommitFunction)
 		r.stateChanged = false
@@ -418,7 +424,7 @@ func (r *templateRouter) writeConfig() error {
 			StatsUser:          r.statsUser,
 			StatsPassword:      r.statsPassword,
 			StatsPort:          r.statsPort,
-			BindPorts:          !r.bindPortsAfterSync || r.syncedAtLeastOnce,
+			BindPorts:          !r.bindPortsAfterSync || r.synced,
 		}
 		if err := template.Execute(file, data); err != nil {
 			file.Close()
@@ -745,17 +751,6 @@ func (r *templateRouter) shouldWriteCerts(cfg *ServiceAliasConfig) bool {
 	return false
 }
 
-// SetSyncedAtLeastOnce indicates to the router that state has been
-// read from the api.
-func (r *templateRouter) SetSyncedAtLeastOnce() {
-	r.lock.Lock()
-	defer r.lock.Unlock()
-
-	glog.V(4).Infof("Router state synchronized for the first time")
-	r.syncedAtLeastOnce = true
-	r.stateChanged = true
-}
-
 // HasRoute indicates whether the given route is known to this router.
 func (r *templateRouter) HasRoute(route *routeapi.Route) bool {
 	r.lock.Lock()

test/integration/router_stress_test.go

@@ -288,13 +288,6 @@ func (p *DelayPlugin) Commit() error {
 	return p.plugin.Commit()
 }
 
-func (p *DelayPlugin) SetSyncedAtLeastOnce() error {
-	if p.committed {
-		return nil
-	}
-	return p.plugin.SetSyncedAtLeastOnce()
-}
-
 // launchRouter launches a template router that communicates with the
 // api via the provided clients.
 func launchRouter(oc osclient.Interface, kc kclientset.Interface, maxDelay int32, name string, reloadInterval int, reloadedMap map[string]bool) (templatePlugin *templateplugin.TemplatePlugin) {