Refactor GCEClient: wrap ValidConfigs in an interface so that unit te…

…sts using machineactuator.go can inject their Configs instead of having a dependency on machined_setup_configs.yaml

cloud/google/cmd/gce-machine-controller/main.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/apiserver/pkg/util/logs"
 
 	"sigs.k8s.io/cluster-api/cloud/google"
+	"sigs.k8s.io/cluster-api/cloud/google/machinesetup"
 	"sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset"
 	"sigs.k8s.io/cluster-api/pkg/controller/config"
 	"sigs.k8s.io/cluster-api/pkg/controller/machine"
@@ -55,7 +56,12 @@ func main() {
 		glog.Fatalf("Could not create client for talking to the apiserver: %v", err)
 	}
 
-	actuator, err := google.NewMachineActuator(*kubeadmToken, client.ClusterV1alpha1().Machines(corev1.NamespaceDefault), *machineSetupConfigsPath)
+	configWatch, err := machinesetup.NewConfigWatch(*machineSetupConfigsPath)
+	if err != nil {
+		glog.Fatalf("Could not create config watch: %v", err)
+	}
+
+	actuator, err := google.NewMachineActuator(*kubeadmToken, client.ClusterV1alpha1().Machines(corev1.NamespaceDefault), configWatch)
 	if err != nil {
 		glog.Fatalf("Could not create Google machine actuator: %v", err)
 	}

cloud/google/machineactuator.go

@@ -66,6 +66,10 @@ type SshCreds struct {
 	privateKeyPath string
 }
 
+type GCEClientMachineSetupConfigGetter interface {
+	GetMachineSetupConfig() (machinesetup.MachineSetupConfig, error)
+}
+
 type GCEClientComputeService interface {
 	ImagesGet(project string, image string) (*compute.Image, error)
 	ImagesGetFromFamily(project string, family string) (*compute.Image, error)
@@ -76,21 +80,21 @@ type GCEClientComputeService interface {
 }
 
 type GCEClient struct {
-	computeService GCEClientComputeService
-	scheme         *runtime.Scheme
-	codecFactory   *serializer.CodecFactory
-	kubeadmToken   string
-	sshCreds       SshCreds
-	machineClient  client.MachineInterface
-	configWatch    *machinesetup.ConfigWatch
+	computeService           GCEClientComputeService
+	scheme                   *runtime.Scheme
+	codecFactory             *serializer.CodecFactory
+	kubeadmToken             string
+	sshCreds                 SshCreds
+	machineClient            client.MachineInterface
+	machineSetupConfigGetter GCEClientMachineSetupConfigGetter
 }
 
 const (
 	gceTimeout   = time.Minute * 10
 	gceWaitSleep = time.Second * 5
 )
 
-func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterface, configListPath string) (*GCEClient, error) {
+func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterface, machineSetupConfigGetter GCEClientMachineSetupConfigGetter) (*GCEClient, error) {
 	// The default GCP client expects the environment variable
 	// GOOGLE_APPLICATION_CREDENTIALS to point to a file with service credentials.
 	client, err := google.DefaultClient(context.TODO(), compute.ComputeScope)
@@ -121,15 +125,6 @@ func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterfa
 		}
 	}
 
-	// TODO: get rid of empty string check when we switch to the new bootstrapping method.
-	var configWatch *machinesetup.ConfigWatch
-	if configListPath != "" {
-		configWatch, err = machinesetup.NewConfigWatch(configListPath)
-		if err != nil {
-			glog.Errorf("Error creating config watch: %v", err)
-		}
-	}
-
 	return &GCEClient{
 		computeService: computeService,
 		scheme:         scheme,
@@ -139,12 +134,15 @@ func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterfa
 			privateKeyPath: privateKeyPath,
 			user:           user,
 		},
-		machineClient: machineClient,
-		configWatch:   configWatch,
+		machineClient:            machineClient,
+		machineSetupConfigGetter: machineSetupConfigGetter,
 	}, nil
 }
 
 func (gce *GCEClient) CreateMachineController(cluster *clusterv1.Cluster, initialMachines []*clusterv1.Machine, clientSet kubernetes.Clientset) error {
+	if gce.machineSetupConfigGetter == nil {
+		return errors.New("a valid machineSetupConfigGetter is required")
+	}
 	if err := gce.CreateMachineControllerServiceAccount(cluster, initialMachines); err != nil {
 		return err
 	}
@@ -158,13 +156,11 @@ func (gce *GCEClient) CreateMachineController(cluster *clusterv1.Cluster, initia
 		return err
 	}
 
-	// Create the configmap so the machine setup configs can be mounted into the node.
-	// TODO: create the configmap during bootstrapping instead of being buried in the machine actuator code.
-	machineSetupConfigs, err := gce.configWatch.ValidConfigs()
+	machineSetupConfig, err := gce.machineSetupConfigGetter.GetMachineSetupConfig()
 	if err != nil {
 		return err
 	}
-	yaml, err := machineSetupConfigs.GetYaml()
+	yaml, err := machineSetupConfig.GetYaml()
 	if err != nil {
 		return err
 	}
@@ -186,6 +182,9 @@ func (gce *GCEClient) CreateMachineController(cluster *clusterv1.Cluster, initia
 }
 
 func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Machine) error {
+	if gce.machineSetupConfigGetter == nil {
+		return errors.New("a valid machineSetupConfigGetter is required")
+	}
 	config, err := gce.providerconfig(machine.Spec.ProviderConfig)
 	if err != nil {
 		return gce.handleMachineError(machine, apierrors.InvalidMachineConfiguration(
@@ -201,7 +200,7 @@ func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Mach
 		return errors.New("invalid master configuration: missing Machine.Spec.Versions.Kubelet")
 	}
 
-	machineSetupConfigs, err := gce.configWatch.ValidConfigs()
+	machineSetupConfig, err := gce.machineSetupConfigGetter.GetMachineSetupConfig()
 	if err != nil {
 		return err
 	}
@@ -210,13 +209,13 @@ func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Mach
 		Roles:    machine.Spec.Roles,
 		Versions: machine.Spec.Versions,
 	}
-	image, err := machineSetupConfigs.GetImage(configParams)
+	image, err := machineSetupConfig.GetImage(configParams)
 	if err != nil {
 		return err
 	}
 	imagePath := gce.getImagePath(image)
 
-	machineSetupMetadata, err := machineSetupConfigs.GetMetadata(configParams)
+	machineSetupMetadata, err := machineSetupConfig.GetMetadata(configParams)
 	if err != nil {
 		return err
 	}

cloud/google/machinesetup/config_types.go

@@ -26,6 +26,12 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
 )
 
+type MachineSetupConfig interface {
+	GetYaml() (string, error)
+	GetImage(params *ConfigParams) (string, error)
+	GetMetadata(params *ConfigParams) (Metadata, error)
+}
+
 // Config Watch holds the path to the machine setup configs yaml file.
 // This works directly with a yaml file is used instead of a ConfigMap object so that we don't take a dependency on the API Server.
 type ConfigWatch struct {
@@ -71,7 +77,7 @@ func NewConfigWatch(path string) (*ConfigWatch, error) {
 	return &ConfigWatch{path: path}, nil
 }
 
-func (cw *ConfigWatch) ValidConfigs() (*ValidConfigs, error) {
+func (cw *ConfigWatch) GetMachineSetupConfig() (MachineSetupConfig, error) {
 	file, err := os.Open(cw.path)
 	if err != nil {
 		return nil, err

gcp-deployer/deploy/deploy.go

@@ -24,6 +24,7 @@ import (
 
 	"k8s.io/client-go/kubernetes"
 	"sigs.k8s.io/cluster-api/cloud/google"
+	"sigs.k8s.io/cluster-api/cloud/google/machinesetup"
 	clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
 	"sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset"
 	"sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset/typed/cluster/v1alpha1"
@@ -56,7 +57,11 @@ func NewDeployer(provider string, kubeConfigPath string, machineSetupConfigPath
 			glog.Exit(fmt.Sprintf("Failed to set Kubeconfig path err %v\n", err))
 		}
 	}
-	ma, err := google.NewMachineActuator(token, nil, machineSetupConfigPath)
+	configWatch, err := newConfigWatchOrNil(machineSetupConfigPath)
+	if err != nil {
+		glog.Exit(fmt.Sprintf("Could not create config watch: %v\n", err))
+	}
+	ma, err := google.NewMachineActuator(token, nil, configWatch)
 	if err != nil {
 		glog.Exit(err)
 	}
@@ -133,3 +138,10 @@ func (d *deployer) deleteMasterVM(machines []*clusterv1.Machine) error {
 	}
 	return nil
 }
+
+func newConfigWatchOrNil(machineSetupConfigPath string) (*machinesetup.ConfigWatch, error) {
+	if machineSetupConfigPath == "" {
+		return nil, nil
+	}
+	return machinesetup.NewConfigWatch(machineSetupConfigPath)
+}