Skip to content

Commit

Permalink
pkg/tfvars: Pull from []Machine instead of InstallConfig
Browse files Browse the repository at this point in the history 
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.

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 d19cad5 (destroy/bootstrap: explicit pass
`disable-bootstrap.tfvars` for libvirt, 2018-12-13, openshift#900), the
installer runs outside the Terraform directory and needs to pass this
through to Terraform explicitly.

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 [2].

[1]: https://www.terraform.io/docs/configuration/variables.html#variable-files
[2]: openshift#792 (comment)
  • Loading branch information
@wking
wking committed Feb 4, 2019
1 parent da6d45b commit e5707ed
Show file tree
Hide file tree
Showing 12 changed files with 280 additions and 261 deletions.
12 changes: 8 additions & 4 deletions pkg/asset/cluster/cluster.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cluster

import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -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{
Expand All @@ -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")
}
Expand Down
119 changes: 102 additions & 17 deletions pkg/asset/cluster/tfvars.go
View file
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -39,39 +59,98 @@ func (t *TerraformVariables) Dependencies() []asset.Asset {
new(rhcos.Image),
&bootstrap.Bootstrap{},
&machine.Master{},
&machines.Master{},
}
}

// Generate generates the terraform.tfvars file.
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() {
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
}

// 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.
Expand All @@ -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
}
49 changes: 6 additions & 43 deletions pkg/asset/machines/master.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -25,7 +22,7 @@ import (

// Master generates the machines for the `master` machine pool.
type Master struct {
MachinesRaw []byte
Machines []clusterapi.Machine
UserDataSecretRaw []byte
}

Expand Down Expand Up @@ -82,34 +79,20 @@ 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
Expand All @@ -121,18 +104,11 @@ func (m *Master) Generate(dependencies asset.Parents) error {
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")
}
Expand All @@ -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
}
30 changes: 27 additions & 3 deletions pkg/asset/manifests/openshift.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,19 @@ 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"
"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 (
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Loading

0 comments on commit e5707ed

Please sign in to comment.