Skip to content

Commit

Permalink
pkg/asset/installconfig/platform: Drop *PlatformType for types.{platf…
Browse files Browse the repository at this point in the history
…orm}.Name

The old *PlatformType are from cccbb37 (Generate installation assets
via a dependency graph, 2018-08-10, #120), but since 476be07
(pkg/asset: use vendored cluster-api instead of go templates,
2018-10-30, #573), we've had variables for the name strings in the
more central pkg/types.  With this commit, we drop the more peripheral
forms.  I've also pushed the types.PlatformName{Platform} variables
down into types.{platform}.Name at Ahbinav's suggestion [1].

I've added a unit test to enforce sorting in PlatformNames, because
the order is required by sort.SearchStrings in queryUserForPlatform.

[1]: #659 (comment)
  • Loading branch information
wking committed Nov 13, 2018
1 parent a6bddcd commit 1e129fe
Show file tree
Hide file tree
Showing 11 changed files with 55 additions and 40 deletions.
26 changes: 7 additions & 19 deletions pkg/asset/installconfig/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,7 @@ import (
"github.com/openshift/installer/pkg/types/openstack"
)

const (
// AWSPlatformType is used to install on AWS.
AWSPlatformType = "aws"
// OpenStackPlatformType is used to install on OpenStack.
OpenStackPlatformType = "openstack"
// LibvirtPlatformType is used to install of libvirt.
LibvirtPlatformType = "libvirt"
)

var (
validPlatforms = []string{AWSPlatformType, OpenStackPlatformType, LibvirtPlatformType}

validAWSRegions = map[string]string{
"ap-northeast-1": "Tokyo",
"ap-northeast-2": "Seoul",
Expand Down Expand Up @@ -78,19 +67,19 @@ func (a *platform) Generate(asset.Parents) error {
}

switch platform {
case AWSPlatformType:
case aws.Name:
aws, err := a.awsPlatform()
if err != nil {
return err
}
a.AWS = aws
case OpenStackPlatformType:
case openstack.Name:
openstack, err := a.openstackPlatform()
if err != nil {
return err
}
a.OpenStack = openstack
case LibvirtPlatformType:
case libvirt.Name:
libvirt, err := a.libvirtPlatform()
if err != nil {
return err
Expand All @@ -109,18 +98,17 @@ func (a *platform) Name() string {
}

func (a *platform) queryUserForPlatform() (string, error) {
sort.Strings(validPlatforms)
return asset.GenerateUserProvidedAsset(
"Platform",
&survey.Question{
Prompt: &survey.Select{
Message: "Platform",
Options: validPlatforms,
Options: types.PlatformNames,
},
Validate: survey.ComposeValidators(survey.Required, func(ans interface{}) error {
choice := ans.(string)
i := sort.SearchStrings(validPlatforms, choice)
if i == len(validPlatforms) || validPlatforms[i] != choice {
i := sort.SearchStrings(types.PlatformNames, choice)
if i == len(types.PlatformNames) || types.PlatformNames[i] != choice {
return errors.Errorf("invalid platform %q", choice)
}
return nil
Expand Down Expand Up @@ -259,7 +247,7 @@ func (a *platform) openstackPlatform() (*openstack.Platform, error) {
"OPENSHIFT_INSTALL_OPENSTACK_EXTERNAL_NETWORK",
)
if err != nil {
return nil, errors.Wrapf(err, "failed to Marshal %s platform", OpenStackPlatformType)
return nil, errors.Wrapf(err, "failed to Marshal %s platform", openstack.Name)
}

return &openstack.Platform{
Expand Down
4 changes: 2 additions & 2 deletions pkg/asset/machines/aws/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import (

// Machines returns a list of machines for a machinepool.
func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDataSecret string) ([]clusterapi.Machine, error) {
if configPlatform := config.Platform.Name(); configPlatform != types.PlatformNameAWS {
if configPlatform := config.Platform.Name(); configPlatform != aws.Name {
return nil, fmt.Errorf("non-AWS configuration: %q", configPlatform)
}
if poolPlatform := pool.Platform.Name(); poolPlatform != types.PlatformNameAWS {
if poolPlatform := pool.Platform.Name(); poolPlatform != aws.Name {
return nil, fmt.Errorf("non-AWS machine-pool: %q", poolPlatform)
}
clustername := config.ObjectMeta.Name
Expand Down
5 changes: 3 additions & 2 deletions pkg/asset/machines/aws/machinesets.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ import (
clusterapi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"

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

// MachineSets returns a list of machinesets for a machinepool.
func MachineSets(config *types.InstallConfig, pool *types.MachinePool, role, userDataSecret string) ([]clusterapi.MachineSet, error) {
if configPlatform := config.Platform.Name(); configPlatform != types.PlatformNameAWS {
if configPlatform := config.Platform.Name(); configPlatform != aws.Name {
return nil, fmt.Errorf("non-AWS configuration: %q", configPlatform)
}
if poolPlatform := pool.Platform.Name(); poolPlatform != types.PlatformNameAWS {
if poolPlatform := pool.Platform.Name(); poolPlatform != aws.Name {
return nil, fmt.Errorf("non-AWS machine-pool: %q", poolPlatform)
}
clustername := config.ObjectMeta.Name
Expand Down
4 changes: 2 additions & 2 deletions pkg/asset/machines/libvirt/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ import (

// Machines returns a list of machines for a machinepool.
func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDataSecret string) ([]clusterapi.Machine, error) {
if configPlatform := config.Platform.Name(); configPlatform != types.PlatformNameLibvirt {
if configPlatform := config.Platform.Name(); configPlatform != libvirt.Name {
return nil, fmt.Errorf("non-Libvirt configuration: %q", configPlatform)
}
// FIXME: empty is a valid case for Libvirt as we don't use it.
if poolPlatform := pool.Platform.Name(); poolPlatform != "" && poolPlatform != types.PlatformNameLibvirt {
if poolPlatform := pool.Platform.Name(); poolPlatform != "" && poolPlatform != libvirt.Name {
return nil, fmt.Errorf("non-Libvirt machine-pool: %q", poolPlatform)
}
clustername := config.ObjectMeta.Name
Expand Down
5 changes: 3 additions & 2 deletions pkg/asset/machines/libvirt/machinesets.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ import (
clusterapi "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"

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

// MachineSets returns a list of machinesets for a machinepool.
func MachineSets(config *types.InstallConfig, pool *types.MachinePool, role, userDataSecret string) ([]clusterapi.MachineSet, error) {
if configPlatform := config.Platform.Name(); configPlatform != types.PlatformNameLibvirt {
if configPlatform := config.Platform.Name(); configPlatform != libvirt.Name {
return nil, fmt.Errorf("non-Libvirt configuration: %q", configPlatform)
}
// FIXME: empty is a valid case for Libvirt as we don't use it.
if poolPlatform := pool.Platform.Name(); poolPlatform != "" && poolPlatform != types.PlatformNameLibvirt {
if poolPlatform := pool.Platform.Name(); poolPlatform != "" && poolPlatform != libvirt.Name {
return nil, fmt.Errorf("non-Libvirt machine-pool: %q", poolPlatform)
}
clustername := config.ObjectMeta.Name
Expand Down
3 changes: 3 additions & 0 deletions pkg/types/aws/doc.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Package aws contains AWS-specific structures for installer
// configuration and management.
package aws

// Name is name for the AWS platform.
const Name string = "aws"
21 changes: 11 additions & 10 deletions pkg/types/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
// PlatformNameAWS is name for AWS platform.
PlatformNameAWS string = "aws"
// PlatformNameOpenstack is name for Openstack platform.
PlatformNameOpenstack string = "openstack"
// PlatformNameLibvirt is name for Libvirt platform.
PlatformNameLibvirt string = "libvirt"
var (
// PlatformNames is a slice with all the supported platform names in
// alphabetical order.
PlatformNames = []string{
aws.Name,
libvirt.Name,
openstack.Name,
}
)

// InstallConfig is the configuration for an OpenShift install.
Expand Down Expand Up @@ -90,13 +91,13 @@ func (p *Platform) Name() string {
return ""
}
if p.AWS != nil {
return PlatformNameAWS
return aws.Name
}
if p.Libvirt != nil {
return PlatformNameLibvirt
return libvirt.Name
}
if p.OpenStack != nil {
return PlatformNameOpenstack
return openstack.Name
}
return ""
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/types/installconfig_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package types

import (
"sort"
"testing"

"github.com/stretchr/testify/assert"
)

func TestPlatformNamesSorted(t *testing.T) {
sorted := make([]string, len(PlatformNames))
copy(sorted, PlatformNames)
sort.Strings(sorted)
assert.Equal(t, sorted, PlatformNames)
}
3 changes: 3 additions & 0 deletions pkg/types/libvirt/doc.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Package libvirt contains libvirt-specific structures for
// installer configuration and management.
package libvirt

// Name is the name for the libvirt platform.
const Name string = "libvirt"
6 changes: 3 additions & 3 deletions pkg/types/machinepools.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ func (p *MachinePoolPlatform) Name() string {
return ""
}
if p.AWS != nil {
return PlatformNameAWS
return aws.Name
}
if p.Libvirt != nil {
return PlatformNameLibvirt
return libvirt.Name
}
if p.OpenStack != nil {
return PlatformNameOpenstack
return openstack.Name
}
return ""
}
3 changes: 3 additions & 0 deletions pkg/types/openstack/doc.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Package openstack contains OpenStack-specific structures for
// installer configuration and management.
package openstack

// Name is the name for the Openstack platform.
const Name string = "openstack"

0 comments on commit 1e129fe

Please sign in to comment.