From 6454d10f4db4c528f66d47649bd5dde318e00d6b Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 7 Feb 2019 22:21:13 -0800 Subject: [PATCH] pkg/asset/machines: Convert Master to a WriteableAsset Since 3326b6b3 (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 yalm 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 cf60daa1 (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. [1]: https://github.com/openshift/installer/issues/1205 --- pkg/asset/ignition/bootstrap/bootstrap.go | 3 + pkg/asset/machines/master.go | 134 ++++++++++++++++++++-- pkg/asset/manifests/openshift.go | 65 +++-------- pkg/asset/targets/targets.go | 2 + 4 files changed, 150 insertions(+), 54 deletions(-) diff --git a/pkg/asset/ignition/bootstrap/bootstrap.go b/pkg/asset/ignition/bootstrap/bootstrap.go index a6850b12302..8266d171e60 100644 --- a/pkg/asset/ignition/bootstrap/bootstrap.go +++ b/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{}, diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 11d88e03676..4b636afc7d0 100644 --- a/pkg/asset/machines/master.go +++ b/pkg/asset/machines/master.go @@ -2,11 +2,13 @@ package machines import ( "fmt" + "os" + "path/filepath" + "strconv" + "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 +21,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 { @@ -58,13 +75,17 @@ func (m *Master) Generate(dependencies asset.Parents) error { 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 +135,108 @@ func (m *Master) Generate(dependencies asset.Parents) error { default: return fmt.Errorf("invalid Platform") } + + machines := []interface{}{} + if m.Machines == nil { + for _, machine := range m.MachinesDeprecated { + machines = append(machines, machine) + } + } else { + for _, machine := range m.Machines { + machines = append(machines, machine) + } + } + + for i, machine := range machines { + data, err := yaml.Marshal(machine) + if err != nil { + return errors.Wrapf(err, "marshal master %d", i) + } + + m.FileList = append(m.FileList, &asset.File{ + Filename: filepath.Join(directory, fmt.Sprintf(MasterMachineFileName, strconv.Itoa(i))), + Data: data, + }) + } + + asset.SortFiles(m.FileList) + 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 + } + 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 "openstack.cluster.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) + } + } + + asset.SortFiles(m.FileList) + return true, nil +} + func masterPool(pools []types.MachinePool) types.MachinePool { for idx, pool := range pools { if pool.Name == "master" { diff --git a/pkg/asset/manifests/openshift.go b/pkg/asset/manifests/openshift.go index c01bf5c2cf6..cd11d3dae20 100644 --- a/pkg/asset/manifests/openshift.go +++ b/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(fileList) > 0, nil } diff --git a/pkg/asset/targets/targets.go b/pkg/asset/targets/targets.go index db253ccee39..bb153c322b4 100644 --- a/pkg/asset/targets/targets.go +++ b/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{}, }