From 6d449c5376db1e409a2d64ac815ea880ee1bc206 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 28 Nov 2018 23:20:22 -0800 Subject: [PATCH] pkg/tfvars: Pull from []Machine instead of InstallConfig The previous implementation pulled from the install-config, but that missed downstream changes like AWS instance type defaults being applied in pkg/asset/machines. With this commit, we pull that information from the cluster-API types, since they're the last touch point for that data. Some remaining information still comes from the InstallConfig, but I've split it into explicit arguments to avoid confusion about where data is coming from when InstallConfig's machine pools, etc. overlap with clusterapi.Machine fields. Unfortunately, Master.Machines is not loadable. This commit fixes the "Go defaults applied to Master.Machines do not affect Terraform" issue (which is good), but it will not fix the "I ran create manifests and edited openshift/99_openshift-cluster-api_master-machines.yaml" because that is loaded back in downstream of the Master asset. We'll address that in follow-up work. This commit also splits the platform-specific Terraform variable generation into separate functions generating separate files. I've used *.auto.tfvars filenames because Terraform loads those automatically from the current directory [1]. But that only helps folks who are trying to run our generated Terraform by hand; as described in d19cad5d (destroy/bootstrap: explicit pass `disable-bootstrap.tfvars` for libvirt, 2018-12-13, #900), the installer runs outside the Terraform directory and needs to pass this through to Terraform explicitly. The code copying the platform-specific Terraform variables file down into the temporary directory for bootstrap deletion has an IsNotExist check. At the moment, all of our platforms except for 'none' generate a platform-specific Terraform variables file. But we're starting to move towards "treat platforms you don't recognize as no-ops" in most locations (e.g. [2]), so we have the check to make "I guess there wasn't a platform-specific Terraform variables file for this platform" non-fatal. I'd prefer if FileList could be an internal property (.files?), but we currently require public fields for reloading from disk during multiple-invocation creation [3]. [1]: https://www.terraform.io/docs/configuration/variables.html#variable-files [2]: https://github.com/openshift/installer/pull/1112/files#diff-6532666297c6115f7774f93ebab396c4R156 [3]: https://github.com/openshift/installer/pull/792#discussion_r253618310 --- pkg/asset/cluster/cluster.go | 12 ++- pkg/asset/cluster/tfvars.go | 119 ++++++++++++++++++++++++----- pkg/asset/machines/master.go | 53 ++----------- pkg/asset/manifests/openshift.go | 30 +++++++- pkg/destroy/bootstrap/bootstrap.go | 25 ++++-- pkg/terraform/terraform.go | 2 - pkg/tfvars/aws/aws.go | 69 +++++++++++++---- pkg/tfvars/libvirt/cache.go | 33 ++++---- pkg/tfvars/libvirt/libvirt.go | 57 +++++++------- pkg/tfvars/openstack/openstack.go | 64 ++++++---------- pkg/tfvars/tfvars.go | 88 +++------------------ pkg/types/libvirt/platform.go | 8 -- 12 files changed, 297 insertions(+), 263 deletions(-) diff --git a/pkg/asset/cluster/cluster.go b/pkg/asset/cluster/cluster.go index d88d5d76ca9..69b15a324e7 100644 --- a/pkg/asset/cluster/cluster.go +++ b/pkg/asset/cluster/cluster.go @@ -1,6 +1,7 @@ package cluster import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -66,9 +67,12 @@ func (c *Cluster) Generate(parents asset.Parents) (err error) { } defer os.RemoveAll(tmpDir) - terraformVariablesFile := terraformVariables.Files()[0] - if err := ioutil.WriteFile(filepath.Join(tmpDir, terraformVariablesFile.Filename), terraformVariablesFile.Data, 0600); err != nil { - return errors.Wrap(err, "failed to write terraform.tfvars file") + extraArgs := []string{} + for _, file := range terraformVariables.Files() { + if err := ioutil.WriteFile(filepath.Join(tmpDir, file.Filename), file.Data, 0600); err != nil { + return err + } + extraArgs = append(extraArgs, fmt.Sprintf("-var-file=%s", filepath.Join(tmpDir, file.Filename))) } c.FileList = []*asset.File{ @@ -79,7 +83,7 @@ func (c *Cluster) Generate(parents asset.Parents) (err error) { } logrus.Infof("Creating cluster...") - stateFile, err := terraform.Apply(tmpDir, installConfig.Config.Platform.Name()) + stateFile, err := terraform.Apply(tmpDir, installConfig.Config.Platform.Name(), extraArgs...) if err != nil { err = errors.Wrap(err, "failed to create cluster") } diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 95cfbd7fae2..70e11624f18 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -1,27 +1,47 @@ package cluster import ( + "fmt" "os" + libvirtprovider "github.com/openshift/cluster-api-provider-libvirt/pkg/apis/libvirtproviderconfig/v1alpha1" "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/ignition/bootstrap" "github.com/openshift/installer/pkg/asset/ignition/machine" "github.com/openshift/installer/pkg/asset/installconfig" + "github.com/openshift/installer/pkg/asset/machines" "github.com/openshift/installer/pkg/asset/rhcos" "github.com/openshift/installer/pkg/tfvars" + awstfvars "github.com/openshift/installer/pkg/tfvars/aws" + libvirttfvars "github.com/openshift/installer/pkg/tfvars/libvirt" + openstacktfvars "github.com/openshift/installer/pkg/tfvars/openstack" + "github.com/openshift/installer/pkg/types/aws" + "github.com/openshift/installer/pkg/types/libvirt" + "github.com/openshift/installer/pkg/types/none" + "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" + openstackprovider "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1" ) const ( // TfVarsFileName is the filename for Terraform variables. - TfVarsFileName = "terraform.tfvars" + TfVarsFileName = "terraform.tfvars" + + // TfPlatformVarsFileName is a template for platform-specific + // Terraform variable files. + // + // https://www.terraform.io/docs/configuration/variables.html#variable-files + TfPlatformVarsFileName = "terraform.%s.auto.tfvars" + tfvarsAssetName = "Terraform Variables" ) // TerraformVariables depends on InstallConfig and // Ignition to generate the terrafor.tfvars. type TerraformVariables struct { - File *asset.File + FileList []*asset.File } var _ asset.WritableAsset = (*TerraformVariables)(nil) @@ -39,6 +59,7 @@ func (t *TerraformVariables) Dependencies() []asset.Asset { new(rhcos.Image), &bootstrap.Bootstrap{}, &machine.Master{}, + &machines.Master{}, } } @@ -46,21 +67,82 @@ func (t *TerraformVariables) Dependencies() []asset.Asset { func (t *TerraformVariables) Generate(parents asset.Parents) error { clusterID := &installconfig.ClusterID{} installConfig := &installconfig.InstallConfig{} - bootstrap := &bootstrap.Bootstrap{} - master := &machine.Master{} + bootstrapIgnAsset := &bootstrap.Bootstrap{} + masterIgnAsset := &machine.Master{} + mastersAsset := &machines.Master{} rhcosImage := new(rhcos.Image) - parents.Get(clusterID, installConfig, bootstrap, master, rhcosImage) + parents.Get(clusterID, installConfig, bootstrapIgnAsset, masterIgnAsset, mastersAsset, rhcosImage) - bootstrapIgn := string(bootstrap.Files()[0].Data) - masterIgn := string(master.Files()[0].Data) + bootstrapIgn := string(bootstrapIgnAsset.Files()[0].Data) + masterIgn := string(masterIgnAsset.Files()[0].Data) - data, err := tfvars.TFVars(clusterID.ClusterID, installConfig.Config, string(*rhcosImage), bootstrapIgn, masterIgn) + data, err := tfvars.TFVars( + clusterID.ClusterID, + installConfig.Config.ObjectMeta.Name, + installConfig.Config.BaseDomain, + &installConfig.Config.Networking.MachineCIDR.IPNet, + bootstrapIgn, + masterIgn, + len(mastersAsset.Machines), + ) if err != nil { - return errors.Wrap(err, "failed to get Tfvars") + return errors.Wrap(err, "failed to get Terraform variables") } - t.File = &asset.File{ - Filename: TfVarsFileName, - Data: data, + t.FileList = []*asset.File{ + { + Filename: TfVarsFileName, + Data: data, + }, + } + + if len(mastersAsset.Machines) == 0 { + return errors.Errorf("master slice cannot be empty") + } + + switch platform := installConfig.Config.Platform.Name(); platform { + case aws.Name: + data, err = awstfvars.TFVars( + mastersAsset.Machines[0].Spec.ProviderSpec.Value.Object.(*awsprovider.AWSMachineProviderConfig), + ) + if err != nil { + return errors.Wrapf(err, "failed to get %s Terraform variables", platform) + } + t.FileList = append(t.FileList, &asset.File{ + Filename: fmt.Sprintf(TfPlatformVarsFileName, platform), + Data: data, + }) + case libvirt.Name: + data, err = libvirttfvars.TFVars( + mastersAsset.Machines[0].Spec.ProviderSpec.Value.Object.(*libvirtprovider.LibvirtMachineProviderConfig), + string(*rhcosImage), + &installConfig.Config.Networking.MachineCIDR.IPNet, + installConfig.Config.Platform.Libvirt.Network.IfName, + len(mastersAsset.Machines), + ) + if err != nil { + return errors.Wrapf(err, "failed to get %s Terraform variables", platform) + } + t.FileList = append(t.FileList, &asset.File{ + Filename: fmt.Sprintf(TfPlatformVarsFileName, platform), + Data: data, + }) + case none.Name: + case openstack.Name: + data, err = openstacktfvars.TFVars( + mastersAsset.Machines[0].Spec.ProviderSpec.Value.Object.(*openstackprovider.OpenstackProviderSpec), + installConfig.Config.Platform.OpenStack.Region, + installConfig.Config.Platform.OpenStack.ExternalNetwork, + installConfig.Config.Platform.OpenStack.TrunkSupport, + ) + if err != nil { + return errors.Wrapf(err, "failed to get %s Terraform variables", platform) + } + t.FileList = append(t.FileList, &asset.File{ + Filename: fmt.Sprintf(TfPlatformVarsFileName, platform), + Data: data, + }) + default: + logrus.Warnf("unrecognized platform %s", platform) } return nil @@ -68,10 +150,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { // Files returns the files generated by the asset. func (t *TerraformVariables) Files() []*asset.File { - if t.File != nil { - return []*asset.File{t.File} - } - return []*asset.File{} + return t.FileList } // Load reads the terraform.tfvars from disk. @@ -83,7 +162,13 @@ func (t *TerraformVariables) Load(f asset.FileFetcher) (found bool, err error) { } return false, err } + t.FileList = []*asset.File{file} + + fileList, err := f.FetchByPattern(fmt.Sprintf(TfPlatformVarsFileName, "*")) + if err != nil { + return false, err + } + t.FileList = append(t.FileList, fileList...) - t.File = file return true, nil } diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 609f53e67ef..0aca410f987 100644 --- a/pkg/asset/machines/master.go +++ b/pkg/asset/machines/master.go @@ -3,10 +3,7 @@ package machines import ( "fmt" - "github.com/ghodss/yaml" "github.com/pkg/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" clusterapi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" "github.com/openshift/installer/pkg/asset" @@ -25,7 +22,7 @@ import ( // Master generates the machines for the `master` machine pool. type Master struct { - MachinesRaw []byte + Machines []clusterapi.Machine UserDataSecretRaw []byte } @@ -82,57 +79,36 @@ func (m *Master) Generate(dependencies asset.Parents) error { mpool.Zones = azs } pool.Platform.AWS = &mpool - machines, err := aws.Machines(clusterID.ClusterID, ic, &pool, string(*rhcosImage), "master", "master-user-data") + m.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(machines, ic.ObjectMeta.Name) - - list := listFromMachines(machines) - raw, err := yaml.Marshal(list) - if err != nil { - return errors.Wrap(err, "failed to marshal") - } - m.MachinesRaw = raw + aws.ConfigMasters(m.Machines, ic.ObjectMeta.Name) case libvirttypes.Name: mpool := defaultLibvirtMachinePoolPlatform() mpool.Set(ic.Platform.Libvirt.DefaultMachinePlatform) mpool.Set(pool.Platform.Libvirt) pool.Platform.Libvirt = &mpool - machines, err := libvirt.Machines(clusterID.ClusterID, ic, &pool, "master", "master-user-data") + m.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") } - - list := listFromMachines(machines) - raw, err := yaml.Marshal(list) - if err != nil { - return errors.Wrap(err, "failed to marshal") - } - m.MachinesRaw = raw 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 byte slice. - m.MachinesRaw = []byte{} + // load will use an empty slice. + m.Machines = []clusterapi.Machine{} case openstacktypes.Name: mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName) mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform) mpool.Set(pool.Platform.OpenStack) pool.Platform.OpenStack = &mpool - machines, err := openstack.Machines(clusterID.ClusterID, ic, &pool, string(*rhcosImage), "master", "master-user-data") + m.Machines, 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(machines, ic.ObjectMeta.Name) - - list := listFromMachines(machines) - m.MachinesRaw, err = yaml.Marshal(list) - if err != nil { - return errors.Wrap(err, "failed to marshal") - } - + openstack.ConfigMasters(m.Machines, ic.ObjectMeta.Name) default: return fmt.Errorf("invalid Platform") } @@ -147,16 +123,3 @@ func masterPool(pools []types.MachinePool) types.MachinePool { } return types.MachinePool{} } - -func listFromMachines(objs []clusterapi.Machine) *metav1.List { - list := &metav1.List{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "List", - }, - } - for idx := range objs { - list.Items = append(list.Items, runtime.RawExtension{Object: &objs[idx]}) - } - return list -} diff --git a/pkg/asset/manifests/openshift.go b/pkg/asset/manifests/openshift.go index 84c70ffd1a1..1a62f5d1847 100644 --- a/pkg/asset/manifests/openshift.go +++ b/pkg/asset/manifests/openshift.go @@ -12,8 +12,9 @@ import ( // clientconfig out of go-yaml. We'll use it here // until that happens. // https://github.com/openshift/installer/pull/854 - "gopkg.in/yaml.v2" + goyaml "gopkg.in/yaml.v2" + "github.com/ghodss/yaml" "github.com/gophercloud/utils/openstack/clientconfig" "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/installconfig" @@ -21,6 +22,9 @@ import ( 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 ( @@ -95,7 +99,7 @@ func (o *Openshift) Generate(dependencies asset.Parents) error { osmachine.CloudName: cloud, } - marshalled, err := yaml.Marshal(clouds) + marshalled, err := goyaml.Marshal(clouds) if err != nil { return err } @@ -122,11 +126,17 @@ func (o *Openshift) Generate(dependencies asset.Parents) error { cloudCredsSecret, kubeadminPasswordSecret, roleCloudCredsSecretReader) + + masterMachines, err := listFromMachines(master.Machines) + 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": master.MachinesRaw, + "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, @@ -166,3 +176,17 @@ func (o *Openshift) Load(f asset.FileFetcher) (bool, error) { asset.SortFiles(o.FileList) return len(fileList) > 0, nil } + +func listFromMachines(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]}) + } + + return yaml.Marshal(list) +} diff --git a/pkg/destroy/bootstrap/bootstrap.go b/pkg/destroy/bootstrap/bootstrap.go index 19c8229a82b..b11cf87d196 100644 --- a/pkg/destroy/bootstrap/bootstrap.go +++ b/pkg/destroy/bootstrap/bootstrap.go @@ -6,9 +6,11 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "github.com/openshift/installer/pkg/asset/cluster" "github.com/openshift/installer/pkg/terraform" + "github.com/openshift/installer/pkg/types/libvirt" "github.com/pkg/errors" ) @@ -24,9 +26,10 @@ func Destroy(dir string) (err error) { return errors.New("no platform configured in metadata") } - copyNames := []string{terraform.StateFileName, cluster.TfVarsFileName} + tfPlatformVarsFileName := fmt.Sprintf(cluster.TfPlatformVarsFileName, platform) + copyNames := []string{terraform.StateFileName, cluster.TfVarsFileName, tfPlatformVarsFileName} - if platform == "libvirt" { + if platform == libvirt.Name { err = ioutil.WriteFile(filepath.Join(dir, "disable-bootstrap.tfvars"), []byte(`{ "bootstrap_dns": false } @@ -43,21 +46,31 @@ func Destroy(dir string) (err error) { } defer os.RemoveAll(tempDir) + extraArgs := []string{} for _, filename := range copyNames { - err = copy(filepath.Join(dir, filename), filepath.Join(tempDir, filename)) + sourcePath := filepath.Join(dir, filename) + targetPath := filepath.Join(tempDir, filename) + err = copy(sourcePath, targetPath) if err != nil { + if os.IsNotExist(err) && err.(*os.PathError).Path == sourcePath && filename == tfPlatformVarsFileName { + continue // platform may not need platform-specific Terraform variables + } return errors.Wrapf(err, "failed to copy %s to the temporary directory", filename) } + if strings.HasSuffix(filename, ".tfvars") { + extraArgs = append(extraArgs, fmt.Sprintf("-var-file=%s", targetPath)) + } } - if platform == "libvirt" { - _, err = terraform.Apply(tempDir, platform, fmt.Sprintf("-var-file=%s", filepath.Join(tempDir, "disable-bootstrap.tfvars"))) + if platform == libvirt.Name { + _, err = terraform.Apply(tempDir, platform, extraArgs...) if err != nil { return errors.Wrap(err, "Terraform apply") } } - err = terraform.Destroy(tempDir, platform, "-target=module.bootstrap") + extraArgs = append(extraArgs, "-target=module.bootstrap") + err = terraform.Destroy(tempDir, platform, extraArgs...) if err != nil { return errors.Wrap(err, "Terraform destroy") } diff --git a/pkg/terraform/terraform.go b/pkg/terraform/terraform.go index f566d3491f7..70ea3e8871f 100644 --- a/pkg/terraform/terraform.go +++ b/pkg/terraform/terraform.go @@ -38,7 +38,6 @@ func Apply(dir string, platform string, extraArgs ...string) (path string, err e "-input=false", fmt.Sprintf("-state=%s", filepath.Join(dir, StateFileName)), fmt.Sprintf("-state-out=%s", filepath.Join(dir, StateFileName)), - fmt.Sprintf("-var-file=%s", filepath.Join(dir, VarFileName)), } args := append(defaultArgs, extraArgs...) args = append(args, dir) @@ -71,7 +70,6 @@ func Destroy(dir string, platform string, extraArgs ...string) (err error) { "-input=false", fmt.Sprintf("-state=%s", filepath.Join(dir, StateFileName)), fmt.Sprintf("-state-out=%s", filepath.Join(dir, StateFileName)), - fmt.Sprintf("-var-file=%s", filepath.Join(dir, VarFileName)), } args := append(defaultArgs, extraArgs...) args = append(args, dir) diff --git a/pkg/tfvars/aws/aws.go b/pkg/tfvars/aws/aws.go index 77b66edbcf9..411e99a0644 100644 --- a/pkg/tfvars/aws/aws.go +++ b/pkg/tfvars/aws/aws.go @@ -1,22 +1,63 @@ +// Package aws contains AWS-specific Terraform-variable logic. package aws -// AWS converts AWS related config. -type AWS struct { +import ( + "encoding/json" + + "github.com/pkg/errors" + "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsproviderconfig/v1alpha1" +) + +type config struct { EC2AMIOverride string `json:"aws_ec2_ami_override,omitempty"` ExtraTags map[string]string `json:"aws_extra_tags,omitempty"` - Master `json:",inline"` - Region string `json:"aws_region,omitempty"` + EC2Type string `json:"aws_master_ec2_type,omitempty"` + IOPS int64 `json:"aws_master_root_volume_iops,omitempty"` + Size int64 `json:"aws_master_root_volume_size,omitempty"` + Type string `json:"aws_master_root_volume_type,omitempty"` + Region string `json:"aws_region,omitempty"` } -// Master converts master related config. -type Master struct { - EC2Type string `json:"aws_master_ec2_type,omitempty"` - MasterRootVolume `json:",inline"` -} +// TFVars generates AWS-specific Terraform variables launching the cluster. +func TFVars(masterConfig *v1alpha1.AWSMachineProviderConfig) ([]byte, error) { + tags := make(map[string]string, len(masterConfig.Tags)) + for _, tag := range masterConfig.Tags { + tags[tag.Name] = tag.Value + } + + if len(masterConfig.BlockDevices) == 0 { + return nil, errors.New("block device slice cannot be empty") + } + + rootVolume := masterConfig.BlockDevices[0] + if rootVolume.EBS == nil { + return nil, errors.New("EBS information must be configured for the root volume") + } + + if rootVolume.EBS.VolumeType == nil { + return nil, errors.New("EBS volume type must be configured for the root volume") + } + + if rootVolume.EBS.VolumeSize == nil { + return nil, errors.New("EBS volume size must be configured for the root volume") + } + + if *rootVolume.EBS.VolumeType == "io1" && rootVolume.EBS.Iops == nil { + return nil, errors.New("EBS IOPS must be configured for the io1 root volume") + } + + cfg := &config{ + Region: masterConfig.Placement.Region, + ExtraTags: tags, + EC2AMIOverride: *masterConfig.AMI.ID, + EC2Type: masterConfig.InstanceType, + Size: *rootVolume.EBS.VolumeSize, + Type: *rootVolume.EBS.VolumeType, + } + + if rootVolume.EBS.Iops != nil { + cfg.IOPS = *rootVolume.EBS.Iops + } -// MasterRootVolume converts master rool volume related config. -type MasterRootVolume struct { - IOPS int `json:"aws_master_root_volume_iops,omitempty"` - Size int `json:"aws_master_root_volume_size,omitempty"` - Type string `json:"aws_master_root_volume_type,omitempty"` + return json.MarshalIndent(cfg, "", " ") } diff --git a/pkg/tfvars/libvirt/cache.go b/pkg/tfvars/libvirt/cache.go index 84c198d71bb..602019e3703 100644 --- a/pkg/tfvars/libvirt/cache.go +++ b/pkg/tfvars/libvirt/cache.go @@ -16,7 +16,7 @@ import ( "golang.org/x/sys/unix" ) -// UseCachedImage leaves non-file:// image URIs unalterered. +// cachedImage leaves non-file:// image URIs unalterered. // Other URIs are retrieved with a local cache at // $XDG_CACHE_HOME/openshift-install/libvirt [1]. This allows you to // use the same remote image URI multiple times without needing to @@ -24,25 +24,25 @@ import ( // periodically blow away your cache. // // [1]: https://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html -func (libvirt *Libvirt) UseCachedImage() (err error) { - if strings.HasPrefix(libvirt.Image, "file://") { - return nil +func cachedImage(uri string) (string, error) { + if strings.HasPrefix(uri, "file://") { + return uri, nil } - logrus.Infof("Fetching OS image: %s", filepath.Base(libvirt.Image)) + logrus.Infof("Fetching OS image: %s", filepath.Base(uri)) // FIXME: Use os.UserCacheDir() once we bump to Go 1.11 // baseCacheDir, err := os.UserCacheDir() // if err != nil { - // return err + // return uri, err // } baseCacheDir := filepath.Join(os.Getenv("HOME"), ".cache") cacheDir := filepath.Join(baseCacheDir, "openshift-install", "libvirt") httpCacheDir := filepath.Join(cacheDir, "http") - err = os.MkdirAll(httpCacheDir, 0777) + err := os.MkdirAll(httpCacheDir, 0777) if err != nil { - return err + return uri, err } cache := diskcache.NewWithDiskv(diskv.New(diskv.Options{ @@ -50,24 +50,24 @@ func (libvirt *Libvirt) UseCachedImage() (err error) { CacheSizeMax: 0, // This stops the diskcache from caching the resp in memory. })) transport := httpcache.NewTransport(cache) - resp, err := transport.Client().Get(libvirt.Image) + resp, err := transport.Client().Get(uri) if err != nil { - return err + return uri, err } if resp.StatusCode != 200 { - return fmt.Errorf("%s while getting %s", resp.Status, libvirt.Image) + return uri, fmt.Errorf("%s while getting %s", resp.Status, uri) } defer resp.Body.Close() key, err := cacheKey(resp.Header.Get("ETag")) if err != nil { - return fmt.Errorf("invalid ETag for %s: %v", libvirt.Image, err) + return uri, fmt.Errorf("invalid ETag for %s: %v", uri, err) } imageCacheDir := filepath.Join(cacheDir, "image") err = os.MkdirAll(imageCacheDir, 0777) if err != nil { - return err + return uri, err } imagePath := filepath.Join(imageCacheDir, key) @@ -76,17 +76,16 @@ func (libvirt *Libvirt) UseCachedImage() (err error) { logrus.Debugf("Using cached OS image %q", imagePath) } else { if !os.IsNotExist(err) { - return err + return uri, err } err = cacheImage(resp.Body, imagePath) if err != nil { - return err + return uri, err } } - libvirt.Image = fmt.Sprintf("file://%s", filepath.ToSlash(imagePath)) - return nil + return fmt.Sprintf("file://%s", filepath.ToSlash(imagePath)), nil } func cacheKey(etag string) (key string, err error) { diff --git a/pkg/tfvars/libvirt/libvirt.go b/pkg/tfvars/libvirt/libvirt.go index 97e17ee78b3..dc9710fa1e6 100644 --- a/pkg/tfvars/libvirt/libvirt.go +++ b/pkg/tfvars/libvirt/libvirt.go @@ -1,49 +1,50 @@ +// Package libvirt contains libvirt-specific Terraform-variable logic. package libvirt import ( + "encoding/json" "fmt" "net" "github.com/apparentlymart/go-cidr/cidr" + "github.com/openshift/cluster-api-provider-libvirt/pkg/apis/libvirtproviderconfig/v1alpha1" + "github.com/pkg/errors" ) -// Libvirt encompasses configuration specific to libvirt. -type Libvirt struct { - URI string `json:"libvirt_uri,omitempty"` - Image string `json:"os_image,omitempty"` - Network `json:",inline"` +type config struct { + URI string `json:"libvirt_uri,omitempty"` + Image string `json:"os_image,omitempty"` + IfName string `json:"libvirt_network_if"` MasterIPs []string `json:"libvirt_master_ips,omitempty"` BootstrapIP string `json:"libvirt_bootstrap_ip,omitempty"` } -// Network describes a libvirt network configuration. -type Network struct { - IfName string `json:"libvirt_network_if"` -} +// TFVars generates libvirt-specific Terraform variables. +func TFVars(masterConfig *v1alpha1.LibvirtMachineProviderConfig, osImage string, machineCIDR *net.IPNet, bridge string, masterCount int) ([]byte, error) { + bootstrapIP, err := cidr.Host(machineCIDR, 10) + if err != nil { + return nil, errors.Errorf("failed to generate bootstrap IP: %v", err) + } -// TFVars fills in computed Terraform variables. -func (l *Libvirt) TFVars(machineCIDR *net.IPNet, masterCount int) error { - if l.BootstrapIP == "" { - ip, err := cidr.Host(machineCIDR, 10) - if err != nil { - return fmt.Errorf("failed to generate bootstrap IP: %v", err) - } - l.BootstrapIP = ip.String() + masterIPs, err := generateIPs("master", machineCIDR, masterCount, 11) + if err != nil { + return nil, err } - if len(l.MasterIPs) > 0 { - if len(l.MasterIPs) != masterCount { - return fmt.Errorf("length of MasterIPs doesn't match master count") - } - } else { - if ips, err := generateIPs("master", machineCIDR, masterCount, 11); err == nil { - l.MasterIPs = ips - } else { - return err - } + osImage, err = cachedImage(osImage) + if err != nil { + return nil, errors.Wrap(err, "failed to use cached libvirt image") + } + + cfg := &config{ + URI: masterConfig.URI, + Image: osImage, + IfName: bridge, + BootstrapIP: bootstrapIP.String(), + MasterIPs: masterIPs, } - return nil + return json.MarshalIndent(cfg, "", " ") } func generateIPs(name string, network *net.IPNet, count int, offset int) ([]string, error) { diff --git a/pkg/tfvars/openstack/openstack.go b/pkg/tfvars/openstack/openstack.go index 587973abe75..9b6931f2dc1 100644 --- a/pkg/tfvars/openstack/openstack.go +++ b/pkg/tfvars/openstack/openstack.go @@ -1,49 +1,31 @@ +// Package openstack contains OpenStack-specific Terraform-variable logic. package openstack -// OpenStack converts OpenStack related config. -type OpenStack struct { - BaseImage string `json:"openstack_base_image,omitempty"` - Credentials `json:",inline"` - External `json:",inline"` - ExternalNetwork string `json:"openstack_external_network,omitempty"` - ExtraTags map[string]string `json:"openstack_extra_tags,omitempty"` - Master `json:",inline"` +import ( + "encoding/json" + + "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1" +) + +type config struct { Region string `json:"openstack_region,omitempty"` + BaseImage string `json:"openstack_base_image,omitempty"` + ExternalNetwork string `json:"openstack_external_network,omitempty"` + Cloud string `json:"openstack_credentials_cloud,omitempty"` + FlavorName string `json:"openstack_master_flavor_name,omitempty"` TrunkSupport string `json:"openstack_trunk_support,omitempty"` } -// External converts external related config. -type External struct { - MasterSubnetIDs []string `json:"openstack_external_master_subnet_ids,omitempty"` -} - -// Master converts master related config. -type Master struct { - FlavorName string `json:"openstack_master_flavor_name,omitempty"` - ExtraSGIDs []string `json:"openstack_master_extra_sg_ids,omitempty"` -} +// TFVars generates OpenStack-specific Terraform variables. +func TFVars(masterConfig *v1alpha1.OpenstackProviderSpec, region string, externalNetwork string, trunkSupport string) ([]byte, error) { + cfg := &config{ + Region: region, + BaseImage: masterConfig.Image, + ExternalNetwork: externalNetwork, + Cloud: masterConfig.CloudName, + FlavorName: masterConfig.Flavor, + TrunkSupport: trunkSupport, + } -// Credentials converts credentials related config. -type Credentials struct { - AuthURL string `json:"openstack_credentials_auth_url,omitempty"` - Cert string `json:"openstack_credentials_cert,omitempty"` - Cloud string `json:"openstack_credentials_cloud,omitempty"` - DomainID string `json:"openstack_credentials_domain_id,omitempty"` - DomainName string `json:"openstack_credentials_domain_name,omitempty"` - EndpointType string `json:"openstack_credentials_endpoint_type,omitempty"` - Insecure bool `json:"openstack_credentials_insecure,omitempty"` - Key string `json:"openstack_credentials_key,omitempty"` - Password string `json:"openstack_credentials_password,omitempty"` - ProjectDomainID string `json:"openstack_credentials_project_domain_id,omitempty"` - ProjectDomainName string `json:"openstack_credentials_project_domain_name,omitempty"` - Region string `json:"openstack_credentials_region,omitempty"` - Swauth bool `json:"openstack_credentials_swauth,omitempty"` - TenantID string `json:"openstack_credentials_tenant_id,omitempty"` - TenantName string `json:"openstack_credentials_tenant_name,omitempty"` - Token string `json:"openstack_credentials_token,omitempty"` - UseOctavia bool `json:"openstack_credentials_use_octavia,omitempty"` - UserDomainID string `json:"openstack_credentials_user_domain_id,omitempty"` - UserDomainName string `json:"openstack_credentials_user_domain_name,omitempty"` - UserID string `json:"openstack_credentials_user_id,omitempty"` - UserName string `json:"openstack_credentials_user_name,omitempty"` + return json.MarshalIndent(cfg, "", " ") } diff --git a/pkg/tfvars/tfvars.go b/pkg/tfvars/tfvars.go index 1717db4f016..c61a422b83f 100644 --- a/pkg/tfvars/tfvars.go +++ b/pkg/tfvars/tfvars.go @@ -1,14 +1,9 @@ -// Package tfvars converts an InstallConfig to Terraform variables. +// Package tfvars generates Terraform variables for launching the cluster. package tfvars import ( "encoding/json" - - "github.com/openshift/installer/pkg/tfvars/aws" - "github.com/openshift/installer/pkg/tfvars/libvirt" - "github.com/openshift/installer/pkg/tfvars/openstack" - "github.com/openshift/installer/pkg/types" - "github.com/pkg/errors" + "net" ) type config struct { @@ -20,81 +15,18 @@ type config struct { IgnitionBootstrap string `json:"ignition_bootstrap,omitempty"` IgnitionMaster string `json:"ignition_master,omitempty"` - - aws.AWS `json:",inline"` - libvirt.Libvirt `json:",inline"` - openstack.OpenStack `json:",inline"` } -// TFVars converts the InstallConfig and Ignition content to -// terraform.tfvar JSON. -func TFVars(clusterID string, cfg *types.InstallConfig, osImage, bootstrapIgn, masterIgn string) ([]byte, error) { +// TFVars generates terraform.tfvar JSON for launching the cluster. +func TFVars(clusterID string, clusterName string, baseDomain string, machineCIDR *net.IPNet, bootstrapIgn string, masterIgn string, masterCount int) ([]byte, error) { config := &config{ - ClusterID: clusterID, - Name: cfg.ObjectMeta.Name, - BaseDomain: cfg.BaseDomain, - MachineCIDR: cfg.Networking.MachineCIDR.String(), - - IgnitionMaster: masterIgn, + ClusterID: clusterID, + Name: clusterName, + BaseDomain: baseDomain, + MachineCIDR: machineCIDR.String(), + Masters: masterCount, IgnitionBootstrap: bootstrapIgn, - } - - for _, m := range cfg.Machines { - switch m.Name { - case "master": - var replicas int - if m.Replicas == nil { - replicas = 1 - } else { - replicas = int(*m.Replicas) - } - - config.Masters += replicas - if m.Platform.AWS != nil { - config.AWS.Master = aws.Master{ - EC2Type: m.Platform.AWS.InstanceType, - MasterRootVolume: aws.MasterRootVolume{ - IOPS: m.Platform.AWS.EC2RootVolume.IOPS, - Size: m.Platform.AWS.EC2RootVolume.Size, - Type: m.Platform.AWS.EC2RootVolume.Type, - }, - } - } - } - } - - if cfg.Platform.AWS != nil { - config.AWS.Region = cfg.Platform.AWS.Region - config.AWS.ExtraTags = cfg.Platform.AWS.UserTags - config.AWS.EC2AMIOverride = osImage - } else if cfg.Platform.Libvirt != nil { - masterIPs := make([]string, len(cfg.Platform.Libvirt.MasterIPs)) - for i, ip := range cfg.Platform.Libvirt.MasterIPs { - masterIPs[i] = ip.String() - } - config.Libvirt = libvirt.Libvirt{ - URI: cfg.Platform.Libvirt.URI, - Network: libvirt.Network{ - IfName: cfg.Platform.Libvirt.Network.IfName, - }, - Image: osImage, - MasterIPs: masterIPs, - } - if err := config.Libvirt.TFVars(&cfg.Networking.MachineCIDR.IPNet, config.Masters); err != nil { - return nil, errors.Wrap(err, "failed to insert libvirt variables") - } - if err := config.Libvirt.UseCachedImage(); err != nil { - return nil, errors.Wrap(err, "failed to use cached libvirt image") - } - } else if cfg.Platform.OpenStack != nil { - config.OpenStack = openstack.OpenStack{ - Region: cfg.Platform.OpenStack.Region, - BaseImage: osImage, - } - config.OpenStack.Credentials.Cloud = cfg.Platform.OpenStack.Cloud - config.OpenStack.ExternalNetwork = cfg.Platform.OpenStack.ExternalNetwork - config.OpenStack.Master.FlavorName = cfg.Platform.OpenStack.FlavorName - config.OpenStack.TrunkSupport = cfg.Platform.OpenStack.TrunkSupport + IgnitionMaster: masterIgn, } return json.MarshalIndent(config, "", " ") diff --git a/pkg/types/libvirt/platform.go b/pkg/types/libvirt/platform.go index d4f161708cf..d7ee7aa3008 100644 --- a/pkg/types/libvirt/platform.go +++ b/pkg/types/libvirt/platform.go @@ -1,9 +1,5 @@ package libvirt -import ( - "net" -) - // Platform stores all the global configuration that all // machinesets use. type Platform struct { @@ -24,8 +20,4 @@ type Platform struct { // Network // +optional Network *Network `json:"network,omitempty"` - - // MasterIPs - // +optional - MasterIPs []net.IP `json:"masterIPs,omitempty"` }