Skip to content

Commit

Permalink
pkg/types: Push platform-specific types (AWS, etc.) into subdirs
Browse files Browse the repository at this point in the history
This decouples our platforms a bit and makes it easier to distinguish
between platform-specific and platform-agnostic code.  It also gives
us much more compact struct names, since now we don't need to
distinguish between many flavors of machine pool, etc. in a single
package.

I've also updated pkg/types/doc.go; pkg/types includes more than
user-specified configuration since 78c3118 (pkg: add ClusterMetadata
asset,type that can be used for destroy, 2018-09-25, openshift#324).

I've also added OWNERS files for some OpenStack-specific directories
that were missing them before.

There's still more work to go in this direction (e.g. pushing default
logic into subdirs), but this seems like a reasonable chunk.
  • Loading branch information
wking committed Nov 12, 2018
1 parent 54f341c commit 6c5e904
Show file tree
Hide file tree
Showing 30 changed files with 343 additions and 283 deletions.
9 changes: 6 additions & 3 deletions pkg/asset/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import (
"github.com/openshift/installer/pkg/asset/kubeconfig"
"github.com/openshift/installer/pkg/terraform"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/aws"
"github.com/openshift/installer/pkg/types/libvirt"
"github.com/openshift/installer/pkg/types/openstack"
)

const (
Expand Down Expand Up @@ -87,7 +90,7 @@ func (c *Cluster) Generate(parents asset.Parents) (err error) {

switch {
case installConfig.Config.Platform.AWS != nil:
metadata.ClusterPlatformMetadata.AWS = &types.ClusterAWSPlatformMetadata{
metadata.ClusterPlatformMetadata.AWS = &aws.Metadata{
Region: installConfig.Config.Platform.AWS.Region,
Identifier: []map[string]string{
{
Expand All @@ -99,14 +102,14 @@ func (c *Cluster) Generate(parents asset.Parents) (err error) {
},
}
case installConfig.Config.Platform.OpenStack != nil:
metadata.ClusterPlatformMetadata.OpenStack = &types.ClusterOpenStackPlatformMetadata{
metadata.ClusterPlatformMetadata.OpenStack = &openstack.Metadata{
Region: installConfig.Config.Platform.OpenStack.Region,
Identifier: map[string]string{
"tectonicClusterID": installConfig.Config.ClusterID,
},
}
case installConfig.Config.Platform.Libvirt != nil:
metadata.ClusterPlatformMetadata.Libvirt = &types.ClusterLibvirtPlatformMetadata{
metadata.ClusterPlatformMetadata.Libvirt = &libvirt.Metadata{
URI: installConfig.Config.Platform.Libvirt.URI,
}
default:
Expand Down
3 changes: 2 additions & 1 deletion pkg/asset/ignition/machine/master_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/openshift/installer/pkg/asset/tls"
"github.com/openshift/installer/pkg/ipnet"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/aws"
)

// TestMasterGenerate tests generating the master asset.
Expand All @@ -31,7 +32,7 @@ func TestMasterGenerate(t *testing.T) {
},
},
Platform: types.Platform{
AWS: &types.AWSPlatform{
AWS: &aws.Platform{
Region: "us-east",
},
},
Expand Down
3 changes: 2 additions & 1 deletion pkg/asset/ignition/machine/worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/openshift/installer/pkg/asset/tls"
"github.com/openshift/installer/pkg/ipnet"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/aws"
)

// TestWorkerGenerate tests generating the worker asset.
Expand All @@ -26,7 +27,7 @@ func TestWorkerGenerate(t *testing.T) {
},
},
Platform: types.Platform{
AWS: &types.AWSPlatform{
AWS: &aws.Platform{
Region: "us-east",
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/asset/installconfig/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ func (a *InstallConfig) Generate(parents asset.Parents) error {
switch {
case platform.AWS != nil:
a.Config.AWS = platform.AWS
case platform.Openstack != nil:
a.Config.OpenStack = platform.Openstack
case platform.OpenStack != nil:
a.Config.OpenStack = platform.OpenStack
case platform.Libvirt != nil:
a.Config.Libvirt = platform.Libvirt
a.Config.Libvirt.Network.Name = clusterName.ClusterName
Expand Down
27 changes: 13 additions & 14 deletions pkg/asset/installconfig/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import (
"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/aws"
"github.com/openshift/installer/pkg/types/libvirt"
"github.com/openshift/installer/pkg/types/openstack"
)

const (
Expand Down Expand Up @@ -58,11 +61,7 @@ var (

// Platform is an asset that queries the user for the platform on which to install
// the cluster.
type platform struct {
AWS *types.AWSPlatform
Openstack *types.OpenStackPlatform
Libvirt *types.LibvirtPlatform
}
type platform types.Platform

var _ asset.Asset = (*platform)(nil)

Expand Down Expand Up @@ -90,7 +89,7 @@ func (a *platform) Generate(asset.Parents) error {
if err != nil {
return err
}
a.Openstack = openstack
a.OpenStack = openstack
case LibvirtPlatformType:
libvirt, err := a.libvirtPlatform()
if err != nil {
Expand Down Expand Up @@ -131,7 +130,7 @@ func (a *platform) queryUserForPlatform() (string, error) {
)
}

func (a *platform) awsPlatform() (*types.AWSPlatform, error) {
func (a *platform) awsPlatform() (*aws.Platform, error) {
longRegions := make([]string, 0, len(validAWSRegions))
shortRegions := make([]string, 0, len(validAWSRegions))
for id, location := range validAWSRegions {
Expand Down Expand Up @@ -175,14 +174,14 @@ func (a *platform) awsPlatform() (*types.AWSPlatform, error) {
}
}

return &types.AWSPlatform{
return &aws.Platform{
VPCCIDRBlock: defaultVPCCIDR,
Region: region,
UserTags: userTags,
}, nil
}

func (a *platform) openstackPlatform() (*types.OpenStackPlatform, error) {
func (a *platform) openstackPlatform() (*openstack.Platform, error) {
region, err := asset.GenerateUserProvidedAsset(
"OpenStack Region",
&survey.Question{
Expand Down Expand Up @@ -263,7 +262,7 @@ func (a *platform) openstackPlatform() (*types.OpenStackPlatform, error) {
return nil, errors.Wrapf(err, "failed to Marshal %s platform", OpenStackPlatformType)
}

return &types.OpenStackPlatform{
return &openstack.Platform{
NetworkCIDRBlock: defaultVPCCIDR,
Region: region,
BaseImage: image,
Expand All @@ -272,7 +271,7 @@ func (a *platform) openstackPlatform() (*types.OpenStackPlatform, error) {
}, nil
}

func (a *platform) libvirtPlatform() (*types.LibvirtPlatform, error) {
func (a *platform) libvirtPlatform() (*libvirt.Platform, error) {
uri, err := asset.GenerateUserProvidedAsset(
"Libvirt Connection URI",
&survey.Question{
Expand Down Expand Up @@ -302,12 +301,12 @@ func (a *platform) libvirtPlatform() (*types.LibvirtPlatform, error) {
}
}

return &types.LibvirtPlatform{
Network: types.LibvirtNetwork{
return &libvirt.Platform{
Network: libvirt.Network{
IfName: defaultLibvirtNetworkIfName,
IPRange: defaultLibvirtNetworkIPRange,
},
DefaultMachinePlatform: &types.LibvirtMachinePoolPlatform{
DefaultMachinePlatform: &libvirt.MachinePool{
Image: qcowImage,
},
URI: uri,
Expand Down
3 changes: 2 additions & 1 deletion pkg/asset/machines/aws/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
clusterapi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"

"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/aws"
)

// Machines returns a list of machines for a machinepool.
Expand Down Expand Up @@ -68,7 +69,7 @@ func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDa
return machines, nil
}

func provider(clusterID, clusterName string, platform *types.AWSPlatform, mpool *types.AWSMachinePoolPlatform, azIdx int, role, userDataSecret string) (*awsprovider.AWSMachineProviderConfig, error) {
func provider(clusterID, clusterName string, platform *aws.Platform, mpool *aws.MachinePool, azIdx int, role, userDataSecret string) (*awsprovider.AWSMachineProviderConfig, error) {
az := mpool.Zones[azIdx]
tags, err := tagsFromUserTags(clusterID, clusterName, platform.UserTags)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/asset/machines/libvirt/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
clusterapi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"

"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/libvirt"
)

// Machines returns a list of machines for a machinepool.
Expand Down Expand Up @@ -60,7 +61,7 @@ func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDa
return machines, nil
}

func provider(platform *types.LibvirtPlatform, name string) *libvirtprovider.LibvirtMachineProviderConfig {
func provider(platform *libvirt.Platform, name string) *libvirtprovider.LibvirtMachineProviderConfig {
return &libvirtprovider.LibvirtMachineProviderConfig{
TypeMeta: metav1.TypeMeta{
APIVersion: "libvirtproviderconfig.k8s.io/v1alpha1",
Expand Down
5 changes: 5 additions & 0 deletions pkg/asset/machines/openstack/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# See the OWNERS docs: https://git.k8s.io/community/contributors/guide/owners.md
# This file just uses aliases defined in OWNERS_ALIASES.

approvers:
- openstack-approvers
4 changes: 2 additions & 2 deletions pkg/asset/machines/openstack/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package openstack
import (
"text/template"

"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/openstack"
)

// MasterConfig is used to generate the machine.
Expand All @@ -14,7 +14,7 @@ type MasterConfig struct {
Image string
Tags map[string]string
Region string
Machine types.OpenStackMachinePoolPlatform
Machine openstack.MachinePool
}

// MasterMachinesTmpl is the template for master machines.
Expand Down
4 changes: 2 additions & 2 deletions pkg/asset/machines/openstack/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package openstack
import (
"text/template"

"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/openstack"
)

// Config is used to generate the machine.
Expand All @@ -14,7 +14,7 @@ type Config struct {
Image string
Tags map[string]string
Region string
Machine types.OpenStackMachinePoolPlatform
Machine openstack.MachinePool
}

// WorkerMachineSetTmpl is template for worker machineset.
Expand Down
10 changes: 6 additions & 4 deletions pkg/asset/machines/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,18 @@ import (
"github.com/openshift/installer/pkg/asset/machines/openstack"
"github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/types"
awstypes "github.com/openshift/installer/pkg/types/aws"
openstacktypes "github.com/openshift/installer/pkg/types/openstack"
)

func defaultAWSMachinePoolPlatform() types.AWSMachinePoolPlatform {
return types.AWSMachinePoolPlatform{
func defaultAWSMachinePoolPlatform() awstypes.MachinePool {
return awstypes.MachinePool{
InstanceType: "t2.medium",
}
}

func defaultOpenStackMachinePoolPlatform() types.OpenStackMachinePoolPlatform {
return types.OpenStackMachinePoolPlatform{
func defaultOpenStackMachinePoolPlatform() openstacktypes.MachinePool {
return openstacktypes.MachinePool{
FlavorName: "m1.medium",
}
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/tfvars/openstack/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# See the OWNERS docs: https://git.k8s.io/community/contributors/guide/owners.md
# This file just uses aliases defined in OWNERS_ALIASES.

approvers:
- openstack-approvers
3 changes: 3 additions & 0 deletions pkg/types/aws/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Package aws contains AWS-specific structures for installer
// configuration and management.
package aws
62 changes: 62 additions & 0 deletions pkg/types/aws/machinepool.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package aws

// MachinePool stores the configuration for a machine pool installed
// on AWS.
type MachinePool struct {
// Zones is list of availability zones that can be used.
Zones []string `json:"zones,omitempty"`

// AMIID defines the AMI that should be used.
AMIID string `json:"amiID,omitempty"`

// InstanceType defines the ec2 instance type.
// eg. m4-large
InstanceType string `json:"type"`

// IAMRoleName defines the IAM role associated
// with the ec2 instance.
IAMRoleName string `json:"iamRoleName"`

// EC2RootVolume defines the storage for ec2 instance.
EC2RootVolume `json:"rootVolume"`
}

// Set sets the values from `required` to `a`.
func (a *MachinePool) Set(required *MachinePool) {
if required == nil || a == nil {
return
}

if len(required.Zones) > 0 {
a.Zones = required.Zones
}
if required.AMIID != "" {
a.AMIID = required.AMIID
}
if required.InstanceType != "" {
a.InstanceType = required.InstanceType
}
if required.IAMRoleName != "" {
a.IAMRoleName = required.IAMRoleName
}

if required.EC2RootVolume.IOPS != 0 {
a.EC2RootVolume.IOPS = required.EC2RootVolume.IOPS
}
if required.EC2RootVolume.Size != 0 {
a.EC2RootVolume.Size = required.EC2RootVolume.Size
}
if required.EC2RootVolume.Type != "" {
a.EC2RootVolume.Type = required.EC2RootVolume.Type
}
}

// EC2RootVolume defines the storage for an ec2 instance.
type EC2RootVolume struct {
// IOPS defines the iops for the instance.
IOPS int `json:"iops"`
// Size defines the size of the instance.
Size int `json:"size"`
// Type defines the type of the instance.
Type string `json:"type"`
}
12 changes: 12 additions & 0 deletions pkg/types/aws/metadata.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package aws

// Metadata contains AWS metadata (e.g. for uninstalling the cluster).
type Metadata struct {
Region string `json:"region"`

// Identifier holds a slice of filter maps. The maps hold the
// key/value pairs for the tags we will be matching against. A
// resource matches the map if all of the key/value pairs are in its
// tags. A resource matches Identifier if it matches any of the maps.
Identifier []map[string]string `json:"identifier"`
}
25 changes: 25 additions & 0 deletions pkg/types/aws/platform.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package aws

// Platform stores all the global configuration that all machinesets
// use.
type Platform struct {
// Region specifies the AWS region where the cluster will be created.
Region string `json:"region"`

// UserTags specifies additional tags for AWS resources created for the cluster.
UserTags map[string]string `json:"userTags,omitempty"`

// DefaultMachinePlatform is the default configuration used when
// installing on AWS for machine pools which do not define their own
// platform configuration.
DefaultMachinePlatform *MachinePool `json:"defaultMachinePlatform,omitempty"`

// VPCID specifies the vpc to associate with the cluster.
// If empty, new vpc will be created.
// +optional
VPCID string `json:"vpcID"`

// VPCCIDRBlock
// +optional
VPCCIDRBlock string `json:"vpcCIDRBlock"`
}
Loading

0 comments on commit 6c5e904

Please sign in to comment.