pkg/asset/machines: Convert Master to a WriteableAsset

Since 3326b6b (pkg/tfvars: Pull from []Machine instead of
InstallConfig, 2018-11-28, openshift#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.

I've dropped the structured Master properties (Machines and
MachineDeprecated) in favor of new methods on the asset.  There are
three possible paths towards recovering the asset that we feed in to
TerraformVariables:

a. After rendering it with Generate.  This information had been
   structured before this commit, so no problems here.
b. After reading from the disk.  This information could be
   unmarshalled in Master.Load(), but...
c. After loading it from the state file.  There are no hooks for
   custom unmarshalling in this pathway, so while the generic YAML
   unmarshal would give us a structured machine object, it would not
   unmarshal the RawExtension that holds the provider spec.

The lack of custom-unmarshal hooks for (c) means there's no reliable
way to use Master properties to feed TerraformVariables structured
provider information.  With this commit, we use Master methods
instead.  That's just as efficient for the (b) and (c) cases, and
while it's an uneccessary (de)serialize cycle for (a), that's not too
expensive.

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, openshift#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.

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]: openshift#1205
[2]: https://github.com/golang/go/blob/go1.11.5/src/path/filepath/match.go#L325

Author: wking

pkg/asset/cluster/tfvars.go

@@ -21,7 +21,7 @@ import (
 	"github.com/openshift/installer/pkg/types/openstack"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	awsprovider "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1"
+	awsprovider "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1beta1"
 	openstackprovider "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1"
 )
 
@@ -76,10 +76,8 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 	bootstrapIgn := string(bootstrapIgnAsset.Files()[0].Data)
 	masterIgn := string(masterIgnAsset.Files()[0].Data)
 
-	masterCount := len(mastersAsset.Machines)
-	if mastersAsset.Machines == nil {
-		masterCount = len(mastersAsset.MachinesDeprecated)
-	}
+	masters := mastersAsset.Machines()
+	masterCount := len(masters)
 	data, err := tfvars.TFVars(
 		clusterID.ClusterID,
 		installConfig.Config.ObjectMeta.Name,
@@ -105,8 +103,12 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 
 	switch platform := installConfig.Config.Platform.Name(); platform {
 	case aws.Name:
+		masters, err := mastersAsset.StructuredMachines()
+		if err != nil {
+			return err
+		}
 		data, err = awstfvars.TFVars(
-			mastersAsset.Machines[0].Spec.ProviderSpec.Value.Object.(*awsprovider.AWSMachineProviderConfig),
+			masters[0].Spec.ProviderSpec.Value.Object.(*awsprovider.AWSMachineProviderConfig),
 		)
 		if err != nil {
 			return errors.Wrapf(err, "failed to get %s Terraform variables", platform)
@@ -116,8 +118,12 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 			Data:     data,
 		})
 	case libvirt.Name:
+		masters, err := mastersAsset.StructuredMachines()
+		if err != nil {
+			return err
+		}
 		data, err = libvirttfvars.TFVars(
-			mastersAsset.Machines[0].Spec.ProviderSpec.Value.Object.(*libvirtprovider.LibvirtMachineProviderConfig),
+			masters[0].Spec.ProviderSpec.Value.Object.(*libvirtprovider.LibvirtMachineProviderConfig),
 			string(*rhcosImage),
 			&installConfig.Config.Networking.MachineCIDR.IPNet,
 			installConfig.Config.Platform.Libvirt.Network.IfName,
@@ -132,8 +138,12 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 		})
 	case none.Name:
 	case openstack.Name:
+		masters, err := mastersAsset.StructuredMachinesDeprecated()
+		if err != nil {
+			return err
+		}
 		data, err = openstacktfvars.TFVars(
-			mastersAsset.MachinesDeprecated[0].Spec.ProviderSpec.Value.Object.(*openstackprovider.OpenstackProviderSpec),
+			masters[0].Spec.ProviderSpec.Value.Object.(*openstackprovider.OpenstackProviderSpec),
 			installConfig.Config.Platform.OpenStack.Region,
 			installConfig.Config.Platform.OpenStack.ExternalNetwork,
 			installConfig.Config.Platform.OpenStack.TrunkSupport,

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/aws/machines.go

@@ -11,7 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/utils/pointer"
-	awsprovider "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1"
+	awsprovider "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1beta1"
 
 	"github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/types/aws"

pkg/asset/machines/master.go

@@ -2,11 +2,13 @@ package machines
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
 
+	"github.com/ghodss/yaml"
+	libvirtapi "github.com/openshift/cluster-api-provider-libvirt/pkg/apis"
+	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 +21,32 @@ 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"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
+	awsapi "sigs.k8s.io/cluster-api-provider-aws/pkg/apis"
+	awsprovider "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1beta1"
+	openstackapi "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis"
+	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 {
@@ -59,12 +77,8 @@ func (m *Master) Generate(dependencies asset.Parents) error {
 	dependencies.Get(clusterID, installconfig, rhcosImage, mign)
 
 	var err error
-	userDataMap := map[string][]byte{"master-user-data": mign.File.Data}
-	m.UserDataSecretRaw, err = userDataList(userDataMap)
-	if err != nil {
-		return errors.Wrap(err, "failed to create user-data secret for master machines")
-	}
-
+	machines := []machineapi.Machine{}
+	machinesDeprecated := []clusterapi.Machine{}
 	ic := installconfig.Config
 	pool := masterPool(ic.Machines)
 	switch ic.Platform.Name() {
@@ -81,42 +95,180 @@ func (m *Master) Generate(dependencies asset.Parents) error {
 			mpool.Zones = azs
 		}
 		pool.Platform.AWS = &mpool
-		m.Machines, err = aws.Machines(clusterID.ClusterID, ic, &pool, string(*rhcosImage), "master", "master-user-data")
+		machines, err = aws.Machines(clusterID.ClusterID, ic, &pool, string(*rhcosImage), "master", "master-user-data")
 		if err != nil {
 			return errors.Wrap(err, "failed to create master machine objects")
 		}
-		aws.ConfigMasters(m.Machines, ic.ObjectMeta.Name)
+		aws.ConfigMasters(machines, ic.ObjectMeta.Name)
 	case libvirttypes.Name:
 		mpool := defaultLibvirtMachinePoolPlatform()
 		mpool.Set(ic.Platform.Libvirt.DefaultMachinePlatform)
 		mpool.Set(pool.Platform.Libvirt)
 		pool.Platform.Libvirt = &mpool
-		m.Machines, err = libvirt.Machines(clusterID.ClusterID, ic, &pool, "master", "master-user-data")
+		machines, err = libvirt.Machines(clusterID.ClusterID, ic, &pool, "master", "master-user-data")
 		if err != nil {
 			return errors.Wrap(err, "failed to create master machine objects")
 		}
 	case nonetypes.Name:
-		// This is needed to ensure that roundtrip generate-load tests pass when
-		// comparing this value. Otherwise, generate will use a nil value while
-		// load will use an empty slice.
-		m.Machines = []machineapi.Machine{}
+		return nil
 	case openstacktypes.Name:
 		mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName)
 		mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform)
 		mpool.Set(pool.Platform.OpenStack)
 		pool.Platform.OpenStack = &mpool
 
-		m.MachinesDeprecated, err = openstack.Machines(clusterID.ClusterID, ic, &pool, string(*rhcosImage), "master", "master-user-data")
+		machinesDeprecated, err = openstack.Machines(clusterID.ClusterID, ic, &pool, string(*rhcosImage), "master", "master-user-data")
 		if err != nil {
 			return errors.Wrap(err, "failed to create master machine objects")
 		}
-		openstack.ConfigMasters(m.MachinesDeprecated, ic.ObjectMeta.Name)
+		openstack.ConfigMasters(machinesDeprecated, ic.ObjectMeta.Name)
 	default:
 		return fmt.Errorf("invalid Platform")
 	}
+
+	userDataMap := map[string][]byte{"master-user-data": mign.File.Data}
+	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,
+	}}
+
+	machinesAll := []runtime.Object{}
+	for _, machine := range machines {
+		machinesAll = append(machinesAll, &machine)
+	}
+	for _, machine := range machinesDeprecated {
+		machinesAll = append(machinesAll, &machine)
+	}
+
+	count := len(machinesAll)
+	if count == 0 {
+		return errors.New("at least one master machine must be configured")
+	}
+
+	padFormat := fmt.Sprintf("%%0%dd", len(fmt.Sprintf("%d", count)))
+	for i, machine := range machinesAll {
+		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}
+
+	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...)
+	return true, nil
+}
+
+// Machines returns master Machine manifest YAML.
+func (m *Master) Machines() [][]byte {
+	machines := [][]byte{}
+	userData := filepath.Join(directory, MasterUserDataFileName)
+	for _, file := range m.FileList {
+		if file.Filename == userData {
+			continue
+		}
+		machines = append(machines, file.Data)
+	}
+	return machines
+}
+
+// StructuredMachines returns master Machine manifest structures.
+func (m *Master) StructuredMachines() ([]machineapi.Machine, error) {
+	scheme := runtime.NewScheme()
+	awsapi.AddToScheme(scheme)
+	libvirtapi.AddToScheme(scheme)
+	openstackapi.AddToScheme(scheme)
+	decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(
+		awsprovider.SchemeGroupVersion,
+		libvirtprovider.SchemeGroupVersion,
+		openstackprovider.SchemeGroupVersion,
+	)
+
+	machines := []machineapi.Machine{}
+	for i, data := range m.Machines() {
+		machine := &machineapi.Machine{}
+		err := yaml.Unmarshal(data, &machine)
+		if err != nil {
+			return machines, errors.Wrapf(err, "unmarshal master %d", i)
+		}
+
+		obj, _, err := decoder.Decode(machine.Spec.ProviderSpec.Value.Raw, nil, nil)
+		if err != nil {
+			return machines, errors.Wrapf(err, "unmarshal master %d", i)
+		}
+
+		machine.Spec.ProviderSpec.Value = &runtime.RawExtension{Object: obj}
+		machines = append(machines, *machine)
+	}
+
+	return machines, nil
+}
+
+// StructuredMachinesDeprecated returns master Machine manifest structures.
+func (m *Master) StructuredMachinesDeprecated() ([]clusterapi.Machine, error) {
+	scheme := runtime.NewScheme()
+	openstackapi.AddToScheme(scheme)
+	decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(
+		openstackprovider.SchemeGroupVersion,
+	)
+
+	machines := []clusterapi.Machine{}
+	for i, data := range m.Machines() {
+		machine := &clusterapi.Machine{}
+		err := yaml.Unmarshal(data, &machine)
+		if err != nil {
+			return machines, errors.Wrapf(err, "unmarshal master %d", i)
+		}
+
+		obj, _, err := decoder.Decode(machine.Spec.ProviderSpec.Value.Raw, nil, nil)
+		if err != nil {
+			return machines, errors.Wrapf(err, "unmarshal master %d", i)
+		}
+
+		machine.Spec.ProviderSpec.Value = &runtime.RawExtension{Object: obj}
+		machines = append(machines, *machine)
+	}
+
+	return machines, nil
+}
+
 func masterPool(pools []types.MachinePool) types.MachinePool {
 	for idx, pool := range pools {
 		if pool.Name == "master" {