Remove OnceAndForever util, create a Run() method on MinionController.

pkg/cloudprovider/controller/minioncontroller.go

@@ -18,10 +18,12 @@ package controller
 
 import (
 	"fmt"
+	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/minion"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/golang/glog"
 )
 
@@ -30,16 +32,30 @@ type MinionController struct {
 	matchRE         string
 	staticResources *api.NodeResources
 	registry        minion.Registry
+	period          time.Duration
 }
 
 // NewMinionController returns a new minion controller to sync instances from cloudprovider.
-func NewMinionController(cloud cloudprovider.Interface, matchRE string, staticResources *api.NodeResources, registry minion.Registry) (*MinionController, error) {
+func NewMinionController(
+	cloud cloudprovider.Interface,
+	matchRE string,
+	staticResources *api.NodeResources,
+	registry minion.Registry,
+	period time.Duration) *MinionController {
 	return &MinionController{
 		cloud:           cloud,
 		matchRE:         matchRE,
 		staticResources: staticResources,
 		registry:        registry,
-	}, nil
+		period:          period,
+	}
+}
+
+// Run starts syncing instances from cloudprovider periodically.
+func (s *MinionController) Run() {
+	// Call Sync() first to warm up minion registry.
+	s.Sync()
+	go util.Forever(func() { s.Sync() }, s.period)
 }
 
 // Sync syncs list of instances from cloudprovider to master etcd registry.

pkg/cloudprovider/controller/minioncontroller_test.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"testing"
+	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
@@ -64,10 +65,7 @@ func TestSyncCreateMinion(t *testing.T) {
 	fakeCloud := fake_cloud.FakeCloud{
 		Machines: instances,
 	}
-	minionController, err := NewMinionController(&fakeCloud, ".*", nil, registry)
+	minionController := NewMinionController(&fakeCloud, ".*", nil, registry, time.Second)
-	if err != nil {
-		t.Errorf("Unexpected error")
-	}
 
 	minion, err := registry.GetMinion(ctx, "m3")
 	if minion != nil {
@@ -112,10 +110,7 @@ func TestSyncDeleteMinion(t *testing.T) {
 	fakeCloud := fake_cloud.FakeCloud{
 		Machines: instances,
 	}
-	minionController, err := NewMinionController(&fakeCloud, ".*", nil, registry)
+	minionController := NewMinionController(&fakeCloud, ".*", nil, registry, time.Second)
-	if err != nil {
-		t.Errorf("Unexpected error")
-	}
 
 	minion, err := registry.GetMinion(ctx, "m3")
 	if minion == nil {
@@ -150,12 +145,9 @@ func TestSyncMinionRegexp(t *testing.T) {
 	fakeCloud := fake_cloud.FakeCloud{
 		Machines: instances,
 	}
-	minionController, err := NewMinionController(&fakeCloud, "m[0-9]+", nil, registry)
+	minionController := NewMinionController(&fakeCloud, "m[0-9]+", nil, registry, time.Second)
-	if err != nil {
-		t.Errorf("Unexpected error")
-	}
 
-	err = minionController.Sync()
+	err := minionController.Sync()
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}

pkg/master/master.go

@@ -40,7 +40,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
-	"github.com/golang/glog"
 )
 
 // Config is a structure used to configure a Master.
@@ -119,12 +118,7 @@ func (m *Master) init(c *Config) {
 
 	if c.Cloud != nil && len(c.MinionRegexp) > 0 {
 		// TODO: Move minion controller to its own code.
-		minionController, err := cloudcontroller.NewMinionController(c.Cloud, c.MinionRegexp, &c.NodeResources, m.minionRegistry)
+		cloudcontroller.NewMinionController(c.Cloud, c.MinionRegexp, &c.NodeResources, m.minionRegistry, c.MinionCacheTTL).Run()
-		if err != nil {
-			glog.Errorf("Failed to initalize minion controller (%#v)", err)
-		}
-		// TODO: Create a Run() method on controller to invoke Sync().
-		go util.OnceAndForever(func() { minionController.Sync() }, c.MinionCacheTTL)
 	} else {
 		for _, minionID := range c.Minions {
 			m.minionRegistry.CreateMinion(nil, &api.Minion{

pkg/util/util.go

@@ -60,19 +60,6 @@ func Forever(f func(), period time.Duration) {
 	}
 }
 
-// OnceAndForever runs f first then loops forever running f every d.  Catches any panics, and keeps going.
-func OnceAndForever(f func(), period time.Duration) {
-	defer HandleCrash()
-	f()
-	for {
-		func() {
-			defer HandleCrash()
-			f()
-		}()
-		time.Sleep(period)
-	}
-}
-
 // EncodeJSON returns obj marshalled as a JSON string, ignoring any errors.
 func EncodeJSON(obj interface{}) string {
 	data, _ := json.Marshal(obj)