diff --git a/pkg/asset/ignition/machine/master_test.go b/pkg/asset/ignition/machine/master_test.go index ae4da49d3b9..3d7b65426ab 100644 --- a/pkg/asset/ignition/machine/master_test.go +++ b/pkg/asset/ignition/machine/master_test.go @@ -22,8 +22,8 @@ func TestMasterGenerate(t *testing.T) { Name: "test-cluster", }, BaseDomain: "test-domain", - Networking: types.Networking{ - ServiceCIDR: *ipnet.MustParseCIDR("10.0.1.0/24"), + Networking: &types.Networking{ + ServiceCIDR: ipnet.MustParseCIDR("10.0.1.0/24"), }, Platform: types.Platform{ AWS: &aws.Platform{ diff --git a/pkg/asset/ignition/machine/worker_test.go b/pkg/asset/ignition/machine/worker_test.go index ba79aaa2813..d9f225a3d06 100644 --- a/pkg/asset/ignition/machine/worker_test.go +++ b/pkg/asset/ignition/machine/worker_test.go @@ -17,8 +17,8 @@ import ( func TestWorkerGenerate(t *testing.T) { installConfig := &installconfig.InstallConfig{ Config: &types.InstallConfig{ - Networking: types.Networking{ - ServiceCIDR: *ipnet.MustParseCIDR("10.0.1.0/24"), + Networking: &types.Networking{ + ServiceCIDR: ipnet.MustParseCIDR("10.0.1.0/24"), }, Platform: types.Platform{ AWS: &aws.Platform{ diff --git a/pkg/asset/installconfig/installconfig.go b/pkg/asset/installconfig/installconfig.go index a5bd081ec2d..f40455c3404 100644 --- a/pkg/asset/installconfig/installconfig.go +++ b/pkg/asset/installconfig/installconfig.go @@ -7,11 +7,9 @@ import ( "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "github.com/openshift/installer/pkg/asset" - "github.com/openshift/installer/pkg/asset/installconfig/libvirt" - "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/defaults" openstackvalidation "github.com/openshift/installer/pkg/types/openstack/validation" "github.com/openshift/installer/pkg/types/validation" ) @@ -20,13 +18,6 @@ const ( installConfigFilename = "install-config.yaml" ) -var ( - defaultMachineCIDR = ipnet.MustParseCIDR("10.0.0.0/16") - defaultServiceCIDR = ipnet.MustParseCIDR("172.30.0.0/16") - defaultClusterCIDR = "10.128.0.0/14" - defaultHostSubnetLength = 9 // equivalent to a /23 per node -) - // InstallConfig generates the install-config.yaml file. type InstallConfig struct { Config *types.InstallConfig `json:"config"` @@ -68,47 +59,20 @@ func (a *InstallConfig) Generate(parents asset.Parents) error { }, SSHKey: sshPublicKey.Key, BaseDomain: baseDomain.BaseDomain, - Networking: types.Networking{ - Type: "OpenshiftSDN", - MachineCIDR: *defaultMachineCIDR, - ServiceCIDR: *defaultServiceCIDR, - ClusterNetworks: []netopv1.ClusterNetwork{ - { - CIDR: defaultClusterCIDR, - HostSubnetLength: uint32(defaultHostSubnetLength), - }, - }, - }, PullSecret: pullSecret.PullSecret, } - numberOfMasters := int64(3) - numberOfWorkers := int64(3) - switch { - case platform.AWS != nil: - a.Config.AWS = platform.AWS - case platform.Libvirt != nil: - a.Config.Libvirt = platform.Libvirt - a.Config.Networking.MachineCIDR = *libvirt.DefaultMachineCIDR - numberOfMasters = 1 - numberOfWorkers = 1 - case platform.None != nil: - a.Config.None = platform.None - case platform.OpenStack != nil: - a.Config.OpenStack = platform.OpenStack - default: - panic("unknown platform type") + a.Config.AWS = platform.AWS + a.Config.Libvirt = platform.Libvirt + a.Config.None = platform.None + a.Config.OpenStack = platform.OpenStack + + if err := a.setDefaults(); err != nil { + return errors.Wrapf(err, "failed to set defaults for install config") } - a.Config.Machines = []types.MachinePool{ - { - Name: "master", - Replicas: func(x int64) *int64 { return &x }(numberOfMasters), - }, - { - Name: "worker", - Replicas: func(x int64) *int64 { return &x }(numberOfWorkers), - }, + if err := validation.ValidateInstallConfig(a.Config, openstackvalidation.NewValidValuesFetcher()).ToAggregate(); err != nil { + return errors.Wrap(err, "invalid install config") } data, err := yaml.Marshal(a.Config) @@ -119,7 +83,6 @@ func (a *InstallConfig) Generate(parents asset.Parents) error { Filename: installConfigFilename, Data: data, } - return nil } @@ -150,11 +113,29 @@ func (a *InstallConfig) Load(f asset.FileFetcher) (found bool, err error) { if err := yaml.Unmarshal(file.Data, config); err != nil { return false, errors.Wrapf(err, "failed to unmarshal") } + a.Config = config - if err := validation.ValidateInstallConfig(config, openstackvalidation.NewValidValuesFetcher()).ToAggregate(); err != nil { + if err := a.setDefaults(); err != nil { + return false, errors.Wrapf(err, "failed to set defaults for install config") + } + + if err := validation.ValidateInstallConfig(a.Config, openstackvalidation.NewValidValuesFetcher()).ToAggregate(); err != nil { return false, errors.Wrapf(err, "invalid %q file", installConfigFilename) } - a.File, a.Config = file, config + data, err := yaml.Marshal(a.Config) + if err != nil { + return false, errors.Wrap(err, "failed to Marshal InstallConfig") + } + a.File = &asset.File{ + Filename: installConfigFilename, + Data: data, + } + return true, nil } + +func (a *InstallConfig) setDefaults() error { + defaults.SetInstallConfigDefaults(a.Config) + return nil +} diff --git a/pkg/asset/installconfig/installconfig_test.go b/pkg/asset/installconfig/installconfig_test.go index d9a413156a6..63e1541b087 100644 --- a/pkg/asset/installconfig/installconfig_test.go +++ b/pkg/asset/installconfig/installconfig_test.go @@ -5,7 +5,6 @@ import ( "os" "testing" - "github.com/ghodss/yaml" "github.com/golang/mock/gomock" netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "github.com/stretchr/testify/assert" @@ -13,8 +12,10 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/mock" + "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/aws" + "github.com/openshift/installer/pkg/types/none" ) func validInstallConfig() *types.InstallConfig { @@ -23,32 +24,67 @@ func validInstallConfig() *types.InstallConfig { Name: "test-cluster", }, BaseDomain: "test-domain", - Networking: types.Networking{ + Platform: types.Platform{ + AWS: &aws.Platform{ + Region: "us-east-1", + }, + }, + PullSecret: `{"auths":{"example.com":{"auth":"authorization value"}}}`, + } +} + +func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) { + sshPublicKey := &sshPublicKey{} + baseDomain := &baseDomain{"test-domain"} + clusterName := &clusterName{"test-cluster"} + pullSecret := &pullSecret{`{"auths":{"example.com":{"auth":"authorization value"}}}`} + platform := &platform{ + None: &none.Platform{}, + } + installConfig := &InstallConfig{} + parents := asset.Parents{} + parents.Add( + sshPublicKey, + baseDomain, + clusterName, + pullSecret, + platform, + ) + if err := installConfig.Generate(parents); err != nil { + t.Errorf("unexpected error generating install config: %v", err) + } + expected := &types.InstallConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + BaseDomain: "test-domain", + Networking: &types.Networking{ + MachineCIDR: ipnet.MustParseCIDR("10.0.0.0/16"), Type: "OpenshiftSDN", - MachineCIDR: *defaultMachineCIDR, - ServiceCIDR: *defaultServiceCIDR, + ServiceCIDR: ipnet.MustParseCIDR("172.30.0.0/16"), ClusterNetworks: []netopv1.ClusterNetwork{ { - CIDR: "192.168.1.0/24", - HostSubnetLength: 4, + CIDR: "10.128.0.0/14", + HostSubnetLength: 9, }, }, }, Machines: []types.MachinePool{ { - Name: "master", + Name: "master", + Replicas: func(x int64) *int64 { return &x }(3), }, { - Name: "worker", + Name: "worker", + Replicas: func(x int64) *int64 { return &x }(3), }, }, Platform: types.Platform{ - AWS: &aws.Platform{ - Region: "us-east-1", - }, + None: &none.Platform{}, }, PullSecret: `{"auths":{"example.com":{"auth":"authorization value"}}}`, } + assert.Equal(t, expected, installConfig.Config, "unexpected config generated") } func TestInstallConfigLoad(t *testing.T) { @@ -62,13 +98,49 @@ func TestInstallConfigLoad(t *testing.T) { }{ { name: "valid InstallConfig", - data: func() string { - ic := validInstallConfig() - data, _ := yaml.Marshal(ic) - return string(data) - }(), - expectedFound: true, - expectedConfig: validInstallConfig(), + data: ` +metadata: + name: test-cluster +baseDomain: test-domain +platform: + aws: + region: us-east-1 +pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}" +`, + expectedFound: true, + expectedConfig: &types.InstallConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + BaseDomain: "test-domain", + Networking: &types.Networking{ + MachineCIDR: ipnet.MustParseCIDR("10.0.0.0/16"), + Type: "OpenshiftSDN", + ServiceCIDR: ipnet.MustParseCIDR("172.30.0.0/16"), + ClusterNetworks: []netopv1.ClusterNetwork{ + { + CIDR: "10.128.0.0/14", + HostSubnetLength: 9, + }, + }, + }, + Machines: []types.MachinePool{ + { + Name: "master", + Replicas: func(x int64) *int64 { return &x }(3), + }, + { + Name: "worker", + Replicas: func(x int64) *int64 { return &x }(3), + }, + }, + Platform: types.Platform{ + AWS: &aws.Platform{ + Region: "us-east-1", + }, + }, + PullSecret: `{"auths":{"example.com":{"auth":"authorization value"}}}`, + }, }, { name: "invalid InstallConfig", @@ -107,7 +179,7 @@ metadata: fileFetcher.EXPECT().FetchByName(installConfigFilename). Return( &asset.File{ - Filename: "test-filename", + Filename: installConfigFilename, Data: []byte(tc.data)}, tc.fetchError, ) @@ -121,12 +193,8 @@ metadata: assert.NoError(t, err, "unexpected error from Load") } if tc.expectedFound { - assert.Equal(t, "test-filename", ic.File.Filename, "unexpected File.Filename in InstallConfig") - assert.Equal(t, tc.data, string(ic.File.Data), "unexpected File.Data in InstallConfig") - } else { - assert.Nil(t, ic.File, "expected File in InstallConfig to be nil") + assert.Equal(t, tc.expectedConfig, ic.Config, "unexpected Config in InstallConfig") } - assert.Equal(t, tc.expectedConfig, ic.Config, "unexpected Config in InstallConfig") }) } } diff --git a/pkg/asset/installconfig/libvirt/libvirt.go b/pkg/asset/installconfig/libvirt/libvirt.go index d4fb8530ad7..b7cb82e6fff 100644 --- a/pkg/asset/installconfig/libvirt/libvirt.go +++ b/pkg/asset/installconfig/libvirt/libvirt.go @@ -4,21 +4,11 @@ package libvirt import ( survey "gopkg.in/AlecAivazis/survey.v1" - "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types/libvirt" + libvirtdefaults "github.com/openshift/installer/pkg/types/libvirt/defaults" "github.com/openshift/installer/pkg/validate" ) -const ( - defaultNetworkIfName = "tt0" -) - -var ( - // DefaultMachineCIDR is the libvirt default IP address space from - // which to assign machine IPs. - DefaultMachineCIDR = ipnet.MustParseCIDR("192.168.126.0/24") -) - // Platform collects libvirt-specific configuration. func Platform() (*libvirt.Platform, error) { var uri string @@ -27,7 +17,7 @@ func Platform() (*libvirt.Platform, error) { Prompt: &survey.Input{ Message: "Libvirt Connection URI", Help: "The libvirt connection URI to be used. This must be accessible from the running cluster.", - Default: "qemu+tcp://192.168.122.1/system", + Default: libvirtdefaults.DefaultURI, }, Validate: survey.ComposeValidators(survey.Required, uriValidator), }, @@ -37,9 +27,6 @@ func Platform() (*libvirt.Platform, error) { } return &libvirt.Platform{ - Network: libvirt.Network{ - IfName: defaultNetworkIfName, - }, URI: uri, }, nil } diff --git a/pkg/types/aws/defaults/platform.go b/pkg/types/aws/defaults/platform.go new file mode 100644 index 00000000000..273ae07bdc9 --- /dev/null +++ b/pkg/types/aws/defaults/platform.go @@ -0,0 +1,9 @@ +package defaults + +import ( + "github.com/openshift/installer/pkg/types/aws" +) + +// SetPlatformDefaults sets the defaults for the platform. +func SetPlatformDefaults(p *aws.Platform) { +} diff --git a/pkg/types/aws/platform.go b/pkg/types/aws/platform.go index 0a70b6c9d3a..f1d5042c255 100644 --- a/pkg/types/aws/platform.go +++ b/pkg/types/aws/platform.go @@ -7,10 +7,12 @@ type Platform struct { Region string `json:"region"` // UserTags specifies additional tags for AWS resources created for the cluster. + // +optional 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. + // +optional DefaultMachinePlatform *MachinePool `json:"defaultMachinePlatform,omitempty"` } diff --git a/pkg/types/defaults/installconfig.go b/pkg/types/defaults/installconfig.go new file mode 100644 index 00000000000..a249eed11e8 --- /dev/null +++ b/pkg/types/defaults/installconfig.go @@ -0,0 +1,74 @@ +package defaults + +import ( + netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" + + "github.com/openshift/installer/pkg/ipnet" + "github.com/openshift/installer/pkg/types" + awsdefaults "github.com/openshift/installer/pkg/types/aws/defaults" + libvirtdefaults "github.com/openshift/installer/pkg/types/libvirt/defaults" + nonedefaults "github.com/openshift/installer/pkg/types/none/defaults" + openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults" +) + +var ( + defaultMachineCIDR = ipnet.MustParseCIDR("10.0.0.0/16") + defaultServiceCIDR = ipnet.MustParseCIDR("172.30.0.0/16") + defaultClusterCIDR = "10.128.0.0/14" + defaultHostSubnetLength = 9 // equivalent to a /23 per node +) + +// SetInstallConfigDefaults sets the defaults for the install config. +func SetInstallConfigDefaults(c *types.InstallConfig) { + if c.Networking == nil { + c.Networking = &types.Networking{} + } + if c.Networking.MachineCIDR == nil { + c.Networking.MachineCIDR = defaultMachineCIDR + if c.Platform.Libvirt != nil { + c.Networking.MachineCIDR = libvirtdefaults.DefaultMachineCIDR + } + } + if c.Networking.Type == "" { + c.Networking.Type = netopv1.NetworkTypeOpenshiftSDN + } + if c.Networking.ServiceCIDR == nil { + c.Networking.ServiceCIDR = defaultServiceCIDR + } + if len(c.Networking.ClusterNetworks) == 0 && c.Networking.PodCIDR == nil { + c.Networking.ClusterNetworks = []netopv1.ClusterNetwork{ + { + CIDR: defaultClusterCIDR, + HostSubnetLength: uint32(defaultHostSubnetLength), + }, + } + } + if len(c.Machines) == 0 { + numberOfMasters := int64(3) + numberOfWorkers := int64(3) + if c.Platform.Libvirt != nil { + numberOfMasters = 1 + numberOfWorkers = 1 + } + c.Machines = []types.MachinePool{ + { + Name: "master", + Replicas: func(x int64) *int64 { return &x }(numberOfMasters), + }, + { + Name: "worker", + Replicas: func(x int64) *int64 { return &x }(numberOfWorkers), + }, + } + } + switch { + case c.Platform.AWS != nil: + awsdefaults.SetPlatformDefaults(c.Platform.AWS) + case c.Platform.Libvirt != nil: + libvirtdefaults.SetPlatformDefaults(c.Platform.Libvirt) + case c.Platform.OpenStack != nil: + openstackdefaults.SetPlatformDefaults(c.Platform.OpenStack) + case c.Platform.None != nil: + nonedefaults.SetPlatformDefaults(c.Platform.None) + } +} diff --git a/pkg/types/defaults/installconfig_test.go b/pkg/types/defaults/installconfig_test.go new file mode 100644 index 00000000000..2a4216cdd84 --- /dev/null +++ b/pkg/types/defaults/installconfig_test.go @@ -0,0 +1,257 @@ +package defaults + +import ( + "testing" + + netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" + "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/ipnet" + "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" + awsdefaults "github.com/openshift/installer/pkg/types/aws/defaults" + "github.com/openshift/installer/pkg/types/libvirt" + libvirtdefaults "github.com/openshift/installer/pkg/types/libvirt/defaults" + "github.com/openshift/installer/pkg/types/none" + nonedefaults "github.com/openshift/installer/pkg/types/none/defaults" + "github.com/openshift/installer/pkg/types/openstack" + openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults" +) + +func defaultInstallConfig() *types.InstallConfig { + return &types.InstallConfig{ + Networking: &types.Networking{ + MachineCIDR: defaultMachineCIDR, + Type: netopv1.NetworkTypeOpenshiftSDN, + ServiceCIDR: defaultServiceCIDR, + ClusterNetworks: []netopv1.ClusterNetwork{ + { + CIDR: defaultClusterCIDR, + HostSubnetLength: uint32(defaultHostSubnetLength), + }, + }, + }, + Machines: []types.MachinePool{ + { + Name: "master", + Replicas: func(x int64) *int64 { return &x }(3), + }, + { + Name: "worker", + Replicas: func(x int64) *int64 { return &x }(3), + }, + }, + } +} + +func defaultAWSInstallConfig() *types.InstallConfig { + c := defaultInstallConfig() + c.Platform.AWS = &aws.Platform{} + awsdefaults.SetPlatformDefaults(c.Platform.AWS) + return c +} + +func defaultLibvirtInstallConfig() *types.InstallConfig { + c := defaultInstallConfig() + c.Networking.MachineCIDR = libvirtdefaults.DefaultMachineCIDR + c.Platform.Libvirt = &libvirt.Platform{} + libvirtdefaults.SetPlatformDefaults(c.Platform.Libvirt) + for i, m := range c.Machines { + m.Replicas = func(x int64) *int64 { return &x }(1) + c.Machines[i] = m + } + return c +} + +func defaultOpenStackInstallConfig() *types.InstallConfig { + c := defaultInstallConfig() + c.Platform.OpenStack = &openstack.Platform{} + openstackdefaults.SetPlatformDefaults(c.Platform.OpenStack) + return c +} + +func defaultNoneInstallConfig() *types.InstallConfig { + c := defaultInstallConfig() + c.Platform.None = &none.Platform{} + nonedefaults.SetPlatformDefaults(c.Platform.None) + return c +} + +func TestSetInstallConfigDefaults(t *testing.T) { + cases := []struct { + name string + config *types.InstallConfig + expected *types.InstallConfig + }{ + { + name: "empty", + config: &types.InstallConfig{}, + expected: defaultInstallConfig(), + }, + { + name: "empty AWS", + config: &types.InstallConfig{ + Platform: types.Platform{ + AWS: &aws.Platform{}, + }, + }, + expected: defaultAWSInstallConfig(), + }, + { + name: "empty Libvirt", + config: &types.InstallConfig{ + Platform: types.Platform{ + Libvirt: &libvirt.Platform{}, + }, + }, + expected: defaultLibvirtInstallConfig(), + }, + { + name: "empty OpenStack", + config: &types.InstallConfig{ + Platform: types.Platform{ + OpenStack: &openstack.Platform{}, + }, + }, + expected: defaultOpenStackInstallConfig(), + }, + { + name: "Networking present", + config: &types.InstallConfig{ + Networking: &types.Networking{}, + }, + expected: defaultInstallConfig(), + }, + { + name: "Networking types present", + config: &types.InstallConfig{ + Networking: &types.Networking{ + Type: netopv1.NetworkType("test-networking-type"), + }, + }, + expected: func() *types.InstallConfig { + c := defaultInstallConfig() + c.Networking.Type = netopv1.NetworkType("test-networking-type") + return c + }(), + }, + { + name: "Service CIDR present", + config: &types.InstallConfig{ + Networking: &types.Networking{ + ServiceCIDR: ipnet.MustParseCIDR("1.2.3.4/8"), + }, + }, + expected: func() *types.InstallConfig { + c := defaultInstallConfig() + c.Networking.ServiceCIDR = ipnet.MustParseCIDR("1.2.3.4/8") + return c + }(), + }, + { + name: "Cluster Networks present", + config: &types.InstallConfig{ + Networking: &types.Networking{ + ClusterNetworks: []netopv1.ClusterNetwork{ + { + CIDR: "test-cidr", + HostSubnetLength: 10, + }, + }, + }, + }, + expected: func() *types.InstallConfig { + c := defaultInstallConfig() + c.Networking.ClusterNetworks = []netopv1.ClusterNetwork{ + { + CIDR: "test-cidr", + HostSubnetLength: 10, + }, + } + return c + }(), + }, + { + name: "Pod CIDR present", + config: &types.InstallConfig{ + Networking: &types.Networking{ + PodCIDR: ipnet.MustParseCIDR("1.2.3.4/8"), + }, + }, + expected: func() *types.InstallConfig { + c := defaultInstallConfig() + c.Networking.ClusterNetworks = nil + c.Networking.PodCIDR = ipnet.MustParseCIDR("1.2.3.4/8") + return c + }(), + }, + { + name: "Machines present", + config: &types.InstallConfig{ + Machines: []types.MachinePool{{Name: "test-machine"}}, + }, + expected: func() *types.InstallConfig { + c := defaultInstallConfig() + c.Machines = []types.MachinePool{{Name: "test-machine"}} + return c + }(), + }, + { + name: "AWS platform present", + config: &types.InstallConfig{ + Platform: types.Platform{ + AWS: &aws.Platform{}, + }, + }, + expected: func() *types.InstallConfig { + c := defaultAWSInstallConfig() + return c + }(), + }, + { + name: "Libvirt platform present", + config: &types.InstallConfig{ + Platform: types.Platform{ + Libvirt: &libvirt.Platform{ + URI: "test-uri", + }, + }, + }, + expected: func() *types.InstallConfig { + c := defaultLibvirtInstallConfig() + c.Platform.Libvirt.URI = "test-uri" + return c + }(), + }, + { + name: "OpenStack platform present", + config: &types.InstallConfig{ + Platform: types.Platform{ + OpenStack: &openstack.Platform{}, + }, + }, + expected: func() *types.InstallConfig { + c := defaultOpenStackInstallConfig() + return c + }(), + }, + { + name: "None platform present", + config: &types.InstallConfig{ + Platform: types.Platform{ + None: &none.Platform{}, + }, + }, + expected: func() *types.InstallConfig { + c := defaultNoneInstallConfig() + return c + }(), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + SetInstallConfigDefaults(tc.config) + assert.Equal(t, tc.expected, tc.config, "unexpected install config") + }) + } +} diff --git a/pkg/types/installconfig.go b/pkg/types/installconfig.go index 4ddecf1e3fc..8aba2ea22de 100644 --- a/pkg/types/installconfig.go +++ b/pkg/types/installconfig.go @@ -34,16 +34,20 @@ type InstallConfig struct { metav1.ObjectMeta `json:"metadata"` // SSHKey is the public ssh key to provide access to instances. - SSHKey string `json:"sshKey"` + // +optional + SSHKey string `json:"sshKey,omitempty"` // BaseDomain is the base domain to which the cluster should belong. BaseDomain string `json:"baseDomain"` // Networking defines the pod network provider in the cluster. - Networking `json:"networking"` + *Networking `json:"networking,omitempty"` // Machines is the list of MachinePools that need to be installed. - Machines []MachinePool `json:"machines"` + // +optional + // Default on AWS and OpenStack is 3 masters and 3 workers. + // Default on Libvirt is 1 master and 1 worker. + Machines []MachinePool `json:"machines,omitempty"` // Platform is the configuration for the specific platform upon which to // perform the installation. @@ -68,9 +72,11 @@ func (c *InstallConfig) MasterCount() int { // the installation. Only one of the platform configuration should be set. type Platform struct { // AWS is the configuration used when installing on AWS. + // +optional AWS *aws.Platform `json:"aws,omitempty"` // Libvirt is the configuration used when installing on libvirt. + // +optional Libvirt *libvirt.Platform `json:"libvirt,omitempty"` // None is the empty configuration used when installing on an unsupported @@ -78,6 +84,7 @@ type Platform struct { None *none.Platform `json:"none,omitempty"` // OpenStack is the configuration used when installing on OpenStack. + // +optional OpenStack *openstack.Platform `json:"openstack,omitempty"` } @@ -106,20 +113,32 @@ func (p *Platform) Name() string { // Networking defines the pod network provider in the cluster. type Networking struct { // MachineCIDR is the IP address space from which to assign machine IPs. - MachineCIDR ipnet.IPNet `json:"machineCIDR"` + // +optional + // Default is 10.0.0.0/16 for all platforms other than Libvirt. + // For Libvirt, the default is 192.168.126.0/24. + MachineCIDR *ipnet.IPNet `json:"machineCIDR,omitempty"` // Type is the network type to install - Type netopv1.NetworkType `json:"type"` + // +optional + // Default is OpenshiftSDN. + Type netopv1.NetworkType `json:"type,omitempty"` // ServiceCIDR is the IP address space from which to assign service IPs. - ServiceCIDR ipnet.IPNet `json:"serviceCIDR"` + // +optional + // Default is 172.30.0.0/16. + ServiceCIDR *ipnet.IPNet `json:"serviceCIDR,omitempty"` // ClusterNetworks is the IP address space from which to assign pod IPs. + // +optional + // Default is a single cluster network with a CIDR of 10.128.0.0/14 + // and a host subnet length of 9. The default is only applicable if PodCIDR + // is not present. ClusterNetworks []netopv1.ClusterNetwork `json:"clusterNetworks,omitempty"` // PodCIDR is deprecated (and badly named; it should have always // been called ClusterCIDR. If no ClusterNetworks are specified, // we will fall back to the PodCIDR // TODO(cdc) remove this. + // +optional PodCIDR *ipnet.IPNet `json:"podCIDR,omitempty"` } diff --git a/pkg/types/libvirt/defaults/network.go b/pkg/types/libvirt/defaults/network.go new file mode 100644 index 00000000000..494188b54db --- /dev/null +++ b/pkg/types/libvirt/defaults/network.go @@ -0,0 +1,24 @@ +package defaults + +import ( + "github.com/openshift/installer/pkg/ipnet" + + "github.com/openshift/installer/pkg/types/libvirt" +) + +const ( + defaultIfName = "tt0" +) + +var ( + // DefaultMachineCIDR is the libvirt default IP address space from + // which to assign machine IPs. + DefaultMachineCIDR = ipnet.MustParseCIDR("192.168.126.0/24") +) + +// SetNetworkDefaults sets the defaults for the network. +func SetNetworkDefaults(n *libvirt.Network) { + if n.IfName == "" { + n.IfName = defaultIfName + } +} diff --git a/pkg/types/libvirt/defaults/network_test.go b/pkg/types/libvirt/defaults/network_test.go new file mode 100644 index 00000000000..d3a379ac886 --- /dev/null +++ b/pkg/types/libvirt/defaults/network_test.go @@ -0,0 +1,46 @@ +package defaults + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/types/libvirt" +) + +func defaultNetwork() *libvirt.Network { + return &libvirt.Network{ + IfName: defaultIfName, + } +} + +func TestSetNetworkDefaults(t *testing.T) { + cases := []struct { + name string + network *libvirt.Network + expected *libvirt.Network + }{ + { + name: "empty", + network: &libvirt.Network{}, + expected: defaultNetwork(), + }, + { + name: "IfName present", + network: &libvirt.Network{ + IfName: "test-if", + }, + expected: func() *libvirt.Network { + n := defaultNetwork() + n.IfName = "test-if" + return n + }(), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + SetNetworkDefaults(tc.network) + assert.Equal(t, tc.expected, tc.network, "unexpected network") + }) + } +} diff --git a/pkg/types/libvirt/defaults/platform.go b/pkg/types/libvirt/defaults/platform.go new file mode 100644 index 00000000000..b52da44b725 --- /dev/null +++ b/pkg/types/libvirt/defaults/platform.go @@ -0,0 +1,21 @@ +package defaults + +import ( + "github.com/openshift/installer/pkg/types/libvirt" +) + +const ( + // DefaultURI is the default URI of the libvirtd connection. + DefaultURI = "qemu+tcp://192.168.122.1/system" +) + +// SetPlatformDefaults sets the defaults for the platform. +func SetPlatformDefaults(p *libvirt.Platform) { + if p.URI == "" { + p.URI = DefaultURI + } + if p.Network == nil { + p.Network = &libvirt.Network{} + } + SetNetworkDefaults(p.Network) +} diff --git a/pkg/types/libvirt/defaults/platform_test.go b/pkg/types/libvirt/defaults/platform_test.go new file mode 100644 index 00000000000..083fa411097 --- /dev/null +++ b/pkg/types/libvirt/defaults/platform_test.go @@ -0,0 +1,67 @@ +package defaults + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/types/libvirt" +) + +func defaultPlatform() *libvirt.Platform { + n := &libvirt.Network{} + SetNetworkDefaults(n) + return &libvirt.Platform{ + URI: DefaultURI, + Network: n, + } +} + +func TestSetPlatformDefaults(t *testing.T) { + cases := []struct { + name string + platform *libvirt.Platform + expected *libvirt.Platform + }{ + { + name: "empty", + platform: &libvirt.Platform{}, + expected: defaultPlatform(), + }, + { + name: "URI present", + platform: &libvirt.Platform{ + URI: "test-uri", + }, + expected: func() *libvirt.Platform { + p := defaultPlatform() + p.URI = "test-uri" + return p + }(), + }, + { + name: "Network present", + platform: &libvirt.Platform{ + Network: func() *libvirt.Network { + n := &libvirt.Network{} + SetNetworkDefaults(n) + n.IfName = "test-if" + return n + }(), + }, + expected: func() *libvirt.Platform { + p := defaultPlatform() + p.Network = &libvirt.Network{} + SetNetworkDefaults(p.Network) + p.Network.IfName = "test-if" + return p + }(), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + SetPlatformDefaults(tc.platform) + assert.Equal(t, tc.expected, tc.platform, "unexpected platform") + }) + } +} diff --git a/pkg/types/libvirt/network.go b/pkg/types/libvirt/network.go index 5580fbfba01..b00e0ddafa7 100644 --- a/pkg/types/libvirt/network.go +++ b/pkg/types/libvirt/network.go @@ -2,6 +2,7 @@ package libvirt // Network is the configuration of the libvirt network. type Network struct { - // IfName is the name of the network interface. - IfName string `json:"if"` + // +optional + // Default is tt0. + IfName string `json:"if,omitempty"` } diff --git a/pkg/types/libvirt/platform.go b/pkg/types/libvirt/platform.go index e9dbe6fbac9..d4f161708cf 100644 --- a/pkg/types/libvirt/platform.go +++ b/pkg/types/libvirt/platform.go @@ -10,16 +10,22 @@ type Platform struct { // URI is the identifier for the libvirtd connection. It must be // reachable from both the host (where the installer is run) and the // cluster (where the cluster-API controller pod will be running). - URI string `json:"URI"` + // +optional + // Default is qemu+tcp://192.168.122.1/system + URI string `json:"URI,omitempty"` // DefaultMachinePlatform is the default configuration used when // installing on libvirt for machine pools which do not define their // own platform configuration. + // +optional + // Default will set the image field to the latest RHCOS image. DefaultMachinePlatform *MachinePool `json:"defaultMachinePlatform,omitempty"` // Network - Network Network `json:"network"` + // +optional + Network *Network `json:"network,omitempty"` // MasterIPs + // +optional MasterIPs []net.IP `json:"masterIPs,omitempty"` } diff --git a/pkg/types/libvirt/validation/platform.go b/pkg/types/libvirt/validation/platform.go index c058c223d95..1a63979c8d1 100644 --- a/pkg/types/libvirt/validation/platform.go +++ b/pkg/types/libvirt/validation/platform.go @@ -16,8 +16,12 @@ func ValidatePlatform(p *libvirt.Platform, fldPath *field.Path) field.ErrorList if p.DefaultMachinePlatform != nil { allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...) } - if p.Network.IfName == "" { - allErrs = append(allErrs, field.Required(fldPath.Child("network").Child("if"), p.Network.IfName)) + if p.Network != nil { + if p.Network.IfName == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("network").Child("if"), p.Network.IfName)) + } + } else { + allErrs = append(allErrs, field.Required(fldPath.Child("network"), "network is required")) } return allErrs } diff --git a/pkg/types/libvirt/validation/platform_test.go b/pkg/types/libvirt/validation/platform_test.go index 67377269f2f..6e6ab5c2b5a 100644 --- a/pkg/types/libvirt/validation/platform_test.go +++ b/pkg/types/libvirt/validation/platform_test.go @@ -12,7 +12,7 @@ import ( func validPlatform() *libvirt.Platform { return &libvirt.Platform{ URI: "qemu+tcp://192.168.122.1/system", - Network: libvirt.Network{ + Network: &libvirt.Network{ IfName: "tt0", }, } @@ -38,6 +38,15 @@ func TestValidatePlatform(t *testing.T) { }(), valid: false, }, + { + name: "missing network", + platform: func() *libvirt.Platform { + p := validPlatform() + p.Network = nil + return p + }(), + valid: false, + }, { name: "missing interface name", platform: func() *libvirt.Platform { diff --git a/pkg/types/none/defaults/platform.go b/pkg/types/none/defaults/platform.go new file mode 100644 index 00000000000..f8e636b78e9 --- /dev/null +++ b/pkg/types/none/defaults/platform.go @@ -0,0 +1,9 @@ +package defaults + +import ( + "github.com/openshift/installer/pkg/types/none" +) + +// SetPlatformDefaults sets the defaults for the platform. +func SetPlatformDefaults(p *none.Platform) { +} diff --git a/pkg/types/openstack/defaults/platform.go b/pkg/types/openstack/defaults/platform.go new file mode 100644 index 00000000000..b7817671302 --- /dev/null +++ b/pkg/types/openstack/defaults/platform.go @@ -0,0 +1,9 @@ +package defaults + +import ( + "github.com/openshift/installer/pkg/types/openstack" +) + +// SetPlatformDefaults sets the defaults for the platform. +func SetPlatformDefaults(p *openstack.Platform) { +} diff --git a/pkg/types/openstack/platform.go b/pkg/types/openstack/platform.go index ca474845121..02218b837c4 100644 --- a/pkg/types/openstack/platform.go +++ b/pkg/types/openstack/platform.go @@ -9,6 +9,7 @@ type Platform struct { // DefaultMachinePlatform is the default configuration used when // installing on OpenStack for machine pools which do not define their own // platform configuration. + // +optional DefaultMachinePlatform *MachinePool `json:"defaultMachinePlatform,omitempty"` // Cloud diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index 385a2d5dba8..cb6b9094af3 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -33,7 +33,11 @@ func ValidateInstallConfig(c *types.InstallConfig, openStackValidValuesFetcher o if err := validate.DomainName(c.BaseDomain); err != nil { allErrs = append(allErrs, field.Invalid(field.NewPath("baseDomain"), c.BaseDomain, err.Error())) } - allErrs = append(allErrs, validateNetworking(&c.Networking, field.NewPath("networking"))...) + if c.Networking != nil { + allErrs = append(allErrs, validateNetworking(c.Networking, field.NewPath("networking"))...) + } else { + allErrs = append(allErrs, field.Required(field.NewPath("networking"), "networking is required")) + } allErrs = append(allErrs, validateMachinePools(c.Machines, field.NewPath("machines"), c.Platform.Name())...) allErrs = append(allErrs, validatePlatform(&c.Platform, field.NewPath("platform"), openStackValidValuesFetcher)...) if err := validate.ImagePullSecret(c.PullSecret); err != nil { diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index 8742cfb4542..34e6927815c 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -22,10 +22,10 @@ func validInstallConfig() *types.InstallConfig { Name: "test-cluster", }, BaseDomain: "test-domain", - Networking: types.Networking{ + Networking: &types.Networking{ Type: "OpenshiftSDN", - MachineCIDR: *ipnet.MustParseCIDR("10.0.0.0/16"), - ServiceCIDR: *ipnet.MustParseCIDR("172.30.0.0/16"), + MachineCIDR: ipnet.MustParseCIDR("10.0.0.0/16"), + ServiceCIDR: ipnet.MustParseCIDR("172.30.0.0/16"), ClusterNetworks: []netopv1.ClusterNetwork{ { CIDR: "192.168.1.0/24", @@ -42,14 +42,28 @@ func validInstallConfig() *types.InstallConfig { }, }, Platform: types.Platform{ - AWS: &aws.Platform{ - Region: "us-east-1", - }, + AWS: validAWSPlatform(), }, PullSecret: `{"auths":{"example.com":{"auth":"authorization value"}}}`, } } +func validAWSPlatform() *aws.Platform { + return &aws.Platform{ + Region: "us-east-1", + } +} + +func validLibvirtPlatform() *libvirt.Platform { + return &libvirt.Platform{ + URI: "qemu+tcp://192.168.122.1/system", + Network: &libvirt.Network{ + IfName: "tt0", + }, + } + +} + func TestValidateInstallConfig(t *testing.T) { cases := []struct { name string @@ -87,6 +101,15 @@ func TestValidateInstallConfig(t *testing.T) { }(), expectedError: `^baseDomain: Invalid value: "\.bad-domain\.": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '\.', and must start and end with an alphanumeric character \(e\.g\. 'example\.com', regex used for validation is '\[a-z0-9]\(\[-a-z0-9]\*\[a-z0-9]\)\?\(\\\.\[a-z0-9]\(\[-a-z0-9]\*\[a-z0-9]\)\?\)\*'\)$`, }, + { + name: "missing networking", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Networking = nil + return c + }(), + expectedError: `^networking: Required value: networking is required$`, + }, { name: "invalid network type", installConfig: func() *types.InstallConfig { @@ -100,7 +123,7 @@ func TestValidateInstallConfig(t *testing.T) { name: "invalid service cidr", installConfig: func() *types.InstallConfig { c := validInstallConfig() - c.Networking.ServiceCIDR = ipnet.IPNet{} + c.Networking.ServiceCIDR = &ipnet.IPNet{} return c }(), expectedError: `^networking\.serviceCIDR: Invalid value: ipnet\.IPNet{IPNet:net\.IPNet{IP:net\.IP\(nil\), Mask:net\.IPMask\(nil\)}}: must use IPv4$`, @@ -194,10 +217,10 @@ func TestValidateInstallConfig(t *testing.T) { name: "multiple platforms", installConfig: func() *types.InstallConfig { c := validInstallConfig() - c.Platform.Libvirt = &libvirt.Platform{} + c.Platform.Libvirt = validLibvirtPlatform() return c }(), - expectedError: `^\[platform: Invalid value: types\.Platform{AWS:\(\*aws\.Platform\)\(0x[0-9a-f]*\), Libvirt:\(\*libvirt\.Platform\)\(0x[0-9a-f]*\), None:\(\*none\.Platform\)\(nil\), OpenStack:\(\*openstack\.Platform\)\(nil\)}: must only specify a single type of platform; cannot use both "aws" and "libvirt", platform\.libvirt\.uri: Invalid value: "": invalid URI "" \(no scheme\), platform\.libvirt\.network\.if: Required value]$`, + expectedError: `^platform: Invalid value: types\.Platform{AWS:\(\*aws\.Platform\)\(0x[0-9a-f]*\), Libvirt:\(\*libvirt\.Platform\)\(0x[0-9a-f]*\), None:\(\*none\.Platform\)\(nil\), OpenStack:\(\*openstack\.Platform\)\(nil\)}: must only specify a single type of platform; cannot use both "aws" and "libvirt"$`, }, { name: "invalid aws platform", @@ -215,12 +238,7 @@ func TestValidateInstallConfig(t *testing.T) { installConfig: func() *types.InstallConfig { c := validInstallConfig() c.Platform = types.Platform{ - Libvirt: &libvirt.Platform{ - URI: "qemu+tcp://192.168.122.1/system", - Network: libvirt.Network{ - IfName: "tt0", - }, - }, + Libvirt: validLibvirtPlatform(), } return c }(), @@ -231,11 +249,12 @@ func TestValidateInstallConfig(t *testing.T) { installConfig: func() *types.InstallConfig { c := validInstallConfig() c.Platform = types.Platform{ - Libvirt: &libvirt.Platform{}, + Libvirt: validLibvirtPlatform(), } + c.Platform.Libvirt.URI = "" return c }(), - expectedError: `^\[platform: Invalid value: types\.Platform{AWS:\(\*aws\.Platform\)\(nil\), Libvirt:\(\*libvirt\.Platform\)\(0x[0-9a-f]*\), None:\(\*none\.Platform\)\(nil\), OpenStack:\(\*openstack\.Platform\)\(nil\)}: must specify one of the platforms \(aws, none, openstack\), platform\.libvirt\.uri: Invalid value: "": invalid URI "" \(no scheme\), platform\.libvirt\.network\.if: Required value]$`, + expectedError: `^\[platform: Invalid value: types\.Platform{AWS:\(\*aws\.Platform\)\(nil\), Libvirt:\(\*libvirt\.Platform\)\(0x[0-9a-f]*\), None:\(\*none\.Platform\)\(nil\), OpenStack:\(\*openstack\.Platform\)\(nil\)}: must specify one of the platforms \(aws, none, openstack\), platform\.libvirt\.uri: Invalid value: "": invalid URI "" \(no scheme\)]$`, }, { name: "valid openstack platform",