pkg/asset/machines: Convert Master to a WriteableAsset

Since 3326b6b (pkg/tfvars: Pull from []Machine instead of
InstallConfig, 2018-11-28, #792), we've been consuming structured
master configurations to generate Terraform variables.  But before
this commit, the Openshift asset was the WriteableAsset responsible
for the master configs, giving you the following issue:

1. openshift-install create manifests
  i. Master asset populated with structured data
  ii. Openshift asset pulls in Master, flattens to YAML, and writes to
      disk.

2. openshift-install create cluster
  i. Openshift asset reads master YAML from disk.
  ii. TerraformVariables asset pulls in Master, but it's empty.
  iii. Panic [1].

With this commit, responsibility for reading and writing master
manifests to disk gets shifted to the Master asset itself.

Unpacking the YAML data into the appropriate structures is a bit
hairy.  I'm not familiar with this code though, maybe there's a better
way.  It will help once we complete the shift to the OpenShift
machine-API types started in cf60daa (Pivot aws from cluster.k8s.io
into machine.openshift.io, 2019-02-01, #1175).

I've also taken the opportunity to drop the list.  It saves us a few
lines of code for (de)serializing, and there's no upside to collecting
the Machine configs together in a single file.

The "glob, but not the master files" logic in the Openshift loader is
also a bit ugly.  Moving forward, I expect us to push the remaining
Openshift assets out into their own assets as well, which would allow
us to tighten down on the wildcard there.

The empty-array sets (vs. using nil zero values):

  m.Machines = []machineapi.Machine{}
  m.MachinesDeprecated = []clusterapi.Machine{}

avoid issues with round-tripping through the asset store.  To support
that, I also had to change some:

  if m.Machines == nil {

to:

  if len(m.Machines) == 0 {

which we will be able to drop once we get rid of MachinesDeprecated.
For similar changes in earlier work, see the comments about
generate-load tests in 75ab106 (assets: add tests for validating
asset fetching of targets, 2018-12-12, #890).

There's also a bit of dancing to ensure our Machine filenames are
ordered by increasing index.  I'm padding the filenames with %02d (for
example) to get ...-01.yaml, etc. so they come back in the right order
on load (filepath.Glob sorts its output [2]).  To avoid hard-coding a
pad size, I format the largest index, measure its length, and use that
length to construct padFormat.

[1]: #1205
[2]: https://github.com/golang/go/blob/go1.11.5/src/path/filepath/match.go#L325

pkg/asset/cluster/tfvars.go

@@ -77,7 +77,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 	masterIgn := string(masterIgnAsset.Files()[0].Data)
 
 	masterCount := len(mastersAsset.Machines)
-	if mastersAsset.Machines == nil {
+	if len(mastersAsset.Machines) == 0 {
 		masterCount = len(mastersAsset.MachinesDeprecated)
 	}
 	data, err := tfvars.TFVars(

pkg/asset/ignition/bootstrap/bootstrap.go

@@ -22,6 +22,7 @@ import (
 	"github.com/openshift/installer/pkg/asset/ignition"
 	"github.com/openshift/installer/pkg/asset/installconfig"
 	"github.com/openshift/installer/pkg/asset/kubeconfig"
+	"github.com/openshift/installer/pkg/asset/machines"
 	"github.com/openshift/installer/pkg/asset/manifests"
 	"github.com/openshift/installer/pkg/asset/tls"
 	"github.com/openshift/installer/pkg/types"
@@ -75,6 +76,7 @@ func (a *Bootstrap) Dependencies() []asset.Asset {
 		&tls.JournalCertKey{},
 		&kubeconfig.Admin{},
 		&kubeconfig.Kubelet{},
+		&machines.Master{},
 		&manifests.Manifests{},
 		&manifests.Openshift{},
 	}
@@ -347,6 +349,7 @@ func (a *Bootstrap) addParentFiles(dependencies asset.Parents) {
 	for _, asset := range []asset.WritableAsset{
 		&kubeconfig.Admin{},
 		&kubeconfig.Kubelet{},
+		&machines.Master{},
 		&tls.KubeCA{},
 		&tls.AggregatorCA{},
 		&tls.EtcdCA{},

pkg/asset/machines/master.go

@@ -2,11 +2,12 @@ package machines
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
 
+	"github.com/ghodss/yaml"
+	libvirtprovider "github.com/openshift/cluster-api-provider-libvirt/pkg/apis/libvirtproviderconfig/v1alpha1"
 	machineapi "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1"
-	"github.com/pkg/errors"
-	clusterapi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
-
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/asset/ignition/machine"
 	"github.com/openshift/installer/pkg/asset/installconfig"
@@ -19,16 +20,31 @@ import (
 	libvirttypes "github.com/openshift/installer/pkg/types/libvirt"
 	nonetypes "github.com/openshift/installer/pkg/types/none"
 	openstacktypes "github.com/openshift/installer/pkg/types/openstack"
+	"github.com/pkg/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	awsprovider "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1"
+	openstackprovider "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1"
+	clusterapi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
 )
 
 // Master generates the machines for the `master` machine pool.
 type Master struct {
 	Machines           []machineapi.Machine
 	MachinesDeprecated []clusterapi.Machine
-	UserDataSecretRaw  []byte
+	FileList           []*asset.File
 }
 
-var _ asset.Asset = (*Master)(nil)
+var (
+	directory = "openshift"
+
+	// MasterMachineFileName is the format string for constucting the master Machine filenames.
+	MasterMachineFileName = "99_openshift-cluster-api_master-machines-%s.yaml"
+
+	// MasterUserDataFileName is the filename used for the master user-data secret.
+	MasterUserDataFileName = "99_openshift-cluster-api_master-user-data-secret.yaml"
+
+	_ asset.WritableAsset = (*Master)(nil)
+)
 
 // Name returns a human friendly name for the Master Asset.
 func (m *Master) Name() string {
@@ -52,19 +68,26 @@ func (m *Master) Dependencies() []asset.Asset {
 
 // Generate generates the Master asset.
 func (m *Master) Generate(dependencies asset.Parents) error {
+	m.Machines = []machineapi.Machine{}
+	m.MachinesDeprecated = []clusterapi.Machine{}
+
 	clusterID := &installconfig.ClusterID{}
 	installconfig := &installconfig.InstallConfig{}
 	rhcosImage := new(rhcos.Image)
 	mign := &machine.Master{}
 	dependencies.Get(clusterID, installconfig, rhcosImage, mign)
 
-	var err error
 	userDataMap := map[string][]byte{"master-user-data": mign.File.Data}
-	m.UserDataSecretRaw, err = userDataList(userDataMap)
+	data, err := userDataList(userDataMap)
 	if err != nil {
 		return errors.Wrap(err, "failed to create user-data secret for master machines")
 	}
 
+	m.FileList = []*asset.File{{
+		Filename: filepath.Join(directory, MasterUserDataFileName),
+		Data:     data,
+	}}
+
 	ic := installconfig.Config
 	pool := masterPool(ic.Machines)
 	switch ic.Platform.Name() {
@@ -114,9 +137,114 @@ func (m *Master) Generate(dependencies asset.Parents) error {
 	default:
 		return fmt.Errorf("invalid Platform")
 	}
+
+	machines := []interface{}{}
+	if len(m.Machines) == 0 {
+		for _, machine := range m.MachinesDeprecated {
+			machines = append(machines, machine)
+		}
+	} else {
+		for _, machine := range m.Machines {
+			machines = append(machines, machine)
+		}
+	}
+
+	padFormat := fmt.Sprintf("%%0%dd", len(fmt.Sprintf("%d", len(machines))))
+	for i, machine := range machines {
+		data, err := yaml.Marshal(machine)
+		if err != nil {
+			return errors.Wrapf(err, "marshal master %d", i)
+		}
+
+		padded := fmt.Sprintf(padFormat, i)
+		m.FileList = append(m.FileList, &asset.File{
+			Filename: filepath.Join(directory, fmt.Sprintf(MasterMachineFileName, padded)),
+			Data:     data,
+		})
+	}
+
 	return nil
 }
 
+// Files returns the files generated by the asset.
+func (m *Master) Files() []*asset.File {
+	return m.FileList
+}
+
+// Load reads the asset files from disk.
+func (m *Master) Load(f asset.FileFetcher) (found bool, err error) {
+	file, err := f.FetchByName(filepath.Join(directory, MasterUserDataFileName))
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false, nil
+		}
+		return false, err
+	}
+	m.FileList = []*asset.File{file}
+
+	m.Machines = []machineapi.Machine{}
+	m.MachinesDeprecated = []clusterapi.Machine{}
+	fileList, err := f.FetchByPattern(filepath.Join(directory, fmt.Sprintf(MasterMachineFileName, "*")))
+	if err != nil {
+		return true, err
+	}
+
+	if len(fileList) == 0 {
+		return true, errors.Errorf("master machine manifests are required if you also provide %s", file.Filename)
+	}
+
+	m.FileList = append(m.FileList, fileList...)
+	for _, file := range fileList {
+		machine := &machineapi.Machine{}
+		err = yaml.Unmarshal(file.Data, &machine)
+		if err != nil {
+			return true, errors.Wrapf(err, "unmarshal %q", file.Filename)
+		}
+
+		rawProvider := machine.Spec.ProviderSpec.Value.Raw
+		providerType := &runtime.TypeMeta{}
+		err = yaml.Unmarshal(rawProvider, &providerType)
+		if err != nil {
+			return true, errors.Wrapf(err, "unmarshal %q provider type", file.Filename)
+		}
+
+		switch providerType.APIVersion {
+		case "awsproviderconfig.k8s.io/v1alpha1":
+			provider := &awsprovider.AWSMachineProviderConfig{}
+			err = yaml.Unmarshal(rawProvider, &provider)
+			if err != nil {
+				return true, errors.Wrapf(err, "unmarshal %s AWS provider", file.Filename)
+			}
+			machine.Spec.ProviderSpec.Value = &runtime.RawExtension{Object: provider}
+			m.Machines = append(m.Machines, *machine)
+		case "libvirtproviderconfig.k8s.io/v1alpha1":
+			provider := &libvirtprovider.LibvirtMachineProviderConfig{}
+			err = yaml.Unmarshal(rawProvider, &provider)
+			if err != nil {
+				return true, errors.Wrapf(err, "unmarshal %s libvirt provider", file.Filename)
+			}
+			m.Machines = append(m.Machines, *machine)
+		case "openstackproviderconfig.k8s.io/v1alpha1":
+			machine := &clusterapi.Machine{}
+			err = yaml.Unmarshal(file.Data, &machine)
+			if err != nil {
+				return true, errors.Wrapf(err, "unmarshal %q", file.Filename)
+			}
+
+			provider := &openstackprovider.OpenstackProviderSpec{}
+			err = yaml.Unmarshal(rawProvider, &provider)
+			if err != nil {
+				return true, errors.Wrapf(err, "unmarshal %s OpenStack provider", file.Filename)
+			}
+			m.MachinesDeprecated = append(m.MachinesDeprecated, *machine)
+		default:
+			return true, errors.Errorf("unrecognized %q provider %q", file.Filename, providerType.APIVersion)
+		}
+	}
+
+	return true, nil
+}
+
 func masterPool(pools []types.MachinePool) types.MachinePool {
 	for idx, pool := range pools {
 		if pool.Name == "master" {

pkg/asset/manifests/openshift.go

@@ -2,6 +2,7 @@ package manifests
 
 import (
 	"encoding/base64"
+	"fmt"
 	"path/filepath"
 
 	"github.com/aws/aws-sdk-go/aws/session"
@@ -14,18 +15,13 @@ import (
 	// https://github.com/openshift/installer/pull/854
 	goyaml "gopkg.in/yaml.v2"
 
-	"github.com/ghodss/yaml"
 	"github.com/gophercloud/utils/openstack/clientconfig"
-	machineapi "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1"
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/asset/installconfig"
 	"github.com/openshift/installer/pkg/asset/machines"
 	osmachine "github.com/openshift/installer/pkg/asset/machines/openstack"
 	"github.com/openshift/installer/pkg/asset/password"
 	"github.com/openshift/installer/pkg/asset/templates/content/openshift"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	clusterapi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
 )
 
 const (
@@ -53,7 +49,6 @@ func (o *Openshift) Dependencies() []asset.Asset {
 		&installconfig.InstallConfig{},
 		&ClusterK8sIO{},
 		&machines.Worker{},
-		&machines.Master{},
 		&password.KubeadminPassword{},
 
 		&openshift.BindingDiscovery{},
@@ -69,8 +64,7 @@ func (o *Openshift) Generate(dependencies asset.Parents) error {
 	kubeadminPassword := &password.KubeadminPassword{}
 	clusterk8sio := &ClusterK8sIO{}
 	worker := &machines.Worker{}
-	master := &machines.Master{}
-	dependencies.Get(installConfig, clusterk8sio, worker, master, kubeadminPassword)
+	dependencies.Get(installConfig, clusterk8sio, worker, kubeadminPassword)
 	var cloudCreds cloudCredsSecretData
 	platform := installConfig.Config.Platform.Name()
 	switch platform {
@@ -128,23 +122,10 @@ func (o *Openshift) Generate(dependencies asset.Parents) error {
 		kubeadminPasswordSecret,
 		roleCloudCredsSecretReader)
 
-	var masterMachines []byte
-	var err error
-	if master.Machines != nil {
-		masterMachines, err = listFromMachines(master.Machines)
-	} else {
-		masterMachines, err = listFromMachinesDeprecated(master.MachinesDeprecated)
-	}
-	if err != nil {
-		return err
-	}
-
 	assetData := map[string][]byte{
 		"99_binding-discovery.yaml":                             []byte(bindingDiscovery.Files()[0].Data),
 		"99_kubeadmin-password-secret.yaml":                     applyTemplateData(kubeadminPasswordSecret.Files()[0].Data, templateData),
 		"99_openshift-cluster-api_cluster.yaml":                 clusterk8sio.Raw,
-		"99_openshift-cluster-api_master-machines.yaml":         masterMachines,
-		"99_openshift-cluster-api_master-user-data-secret.yaml": master.UserDataSecretRaw,
 		"99_openshift-cluster-api_worker-machineset.yaml":       worker.MachineSetRaw,
 		"99_openshift-cluster-api_worker-user-data-secret.yaml": worker.UserDataSecretRaw,
 	}
@@ -179,35 +160,25 @@ func (o *Openshift) Load(f asset.FileFetcher) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	o.FileList = fileList
-	asset.SortFiles(o.FileList)
-	return len(fileList) > 0, nil
-}
 
-func listFromMachines(objs []machineapi.Machine) ([]byte, error) {
-	list := &metav1.List{
-		TypeMeta: metav1.TypeMeta{
-			APIVersion: "v1",
-			Kind:       "List",
-		},
-	}
-	for idx := range objs {
-		list.Items = append(list.Items, runtime.RawExtension{Object: &objs[idx]})
-	}
+	masterMachinePattern := fmt.Sprintf(machines.MasterMachineFileName, "*")
+	for _, file := range fileList {
+		filename := filepath.Base(file.Filename)
+		if filename == machines.MasterUserDataFileName {
+			continue
+		}
 
-	return yaml.Marshal(list)
-}
+		matched, err := filepath.Match(masterMachinePattern, filename)
+		if err != nil {
+			return true, err
+		}
+		if matched {
+			continue
+		}
 
-func listFromMachinesDeprecated(objs []clusterapi.Machine) ([]byte, error) {
-	list := &metav1.List{
-		TypeMeta: metav1.TypeMeta{
-			APIVersion: "v1",
-			Kind:       "List",
-		},
-	}
-	for idx := range objs {
-		list.Items = append(list.Items, runtime.RawExtension{Object: &objs[idx]})
+		o.FileList = append(o.FileList, file)
 	}
 
-	return yaml.Marshal(list)
+	asset.SortFiles(o.FileList)
+	return len(o.FileList) > 0, nil
 }

pkg/asset/targets/targets.go

@@ -7,6 +7,7 @@ import (
 	"github.com/openshift/installer/pkg/asset/ignition/machine"
 	"github.com/openshift/installer/pkg/asset/installconfig"
 	"github.com/openshift/installer/pkg/asset/kubeconfig"
+	"github.com/openshift/installer/pkg/asset/machines"
 	"github.com/openshift/installer/pkg/asset/manifests"
 	"github.com/openshift/installer/pkg/asset/templates/content/bootkube"
 	"github.com/openshift/installer/pkg/asset/templates/content/openshift"
@@ -21,6 +22,7 @@ var (
 
 	// Manifests are the manifests targeted assets.
 	Manifests = []asset.WritableAsset{
+		&machines.Master{},
 		&manifests.Manifests{},
 		&manifests.Openshift{},
 	}