diff --git a/data/data/openstack/bootstrap/main.tf b/data/data/openstack/bootstrap/main.tf index 2778f7562e..9ae43f145b 100644 --- a/data/data/openstack/bootstrap/main.tf +++ b/data/data/openstack/bootstrap/main.tf @@ -22,6 +22,8 @@ resource "openstack_networking_port_v2" "bootstrap_port" { allowed_address_pairs { ip_address = var.node_dns_ip } + + depends_on = [var.master_port_ids] } data "openstack_compute_flavor_v2" "bootstrap_flavor" { diff --git a/data/data/openstack/bootstrap/variables.tf b/data/data/openstack/bootstrap/variables.tf index 709262af36..f3a42d5c86 100644 --- a/data/data/openstack/bootstrap/variables.tf +++ b/data/data/openstack/bootstrap/variables.tf @@ -62,3 +62,7 @@ variable "nodes_subnet_id" { variable "cluster_domain" { type = string } + +variable "master_port_ids" { + type = list(string) +} diff --git a/data/data/openstack/main.tf b/data/data/openstack/main.tf index 2a1d3237dc..e9402fe8d4 100644 --- a/data/data/openstack/main.tf +++ b/data/data/openstack/main.tf @@ -38,6 +38,7 @@ module "bootstrap" { private_network_id = module.topology.private_network_id master_sg_id = module.topology.master_sg_id bootstrap_shim_ignition = var.openstack_bootstrap_shim_ignition + master_port_ids = module.topology.master_port_ids } module "masters" { diff --git a/data/data/openstack/topology/private-network.tf b/data/data/openstack/topology/private-network.tf index a726a3e1fb..dcdfe83c3e 100644 --- a/data/data/openstack/topology/private-network.tf +++ b/data/data/openstack/topology/private-network.tf @@ -62,6 +62,8 @@ resource "openstack_networking_port_v2" "masters" { allowed_address_pairs { ip_address = var.ingress_ip } + + depends_on = [openstack_networking_port_v2.api_port, openstack_networking_port_v2.ingress_port] } resource "openstack_networking_port_v2" "api_port" { @@ -73,8 +75,7 @@ resource "openstack_networking_port_v2" "api_port" { tags = ["openshiftClusterID=${var.cluster_id}"] fixed_ip { - subnet_id = openstack_networking_subnet_v2.nodes.id - # FIXME(mandre) we could let the installer automatically pick up the address + subnet_id = openstack_networking_subnet_v2.nodes.id ip_address = var.api_int_ip } } @@ -88,8 +89,7 @@ resource "openstack_networking_port_v2" "ingress_port" { tags = ["openshiftClusterID=${var.cluster_id}"] fixed_ip { - subnet_id = openstack_networking_subnet_v2.nodes.id - # FIXME(mandre) we could let the installer automatically pick up the address + subnet_id = openstack_networking_subnet_v2.nodes.id ip_address = var.ingress_ip } } diff --git a/docs/user/openstack/customization.md b/docs/user/openstack/customization.md index e5eb094fa9..0b47f0b13a 100644 --- a/docs/user/openstack/customization.md +++ b/docs/user/openstack/customization.md @@ -23,7 +23,9 @@ Beyond the [platform-agnostic `install-config.yaml` properties](../customization * `octaviaSupport` (optional string): Whether OpenStack supports Octavia (`1` for true or `0` for false) * `region` (deprecated string): The OpenStack region where the cluster will be created. Currently this value is not used by the installer. * `trunkSupport` (optional string): Whether OpenStack ports can be trunked (`1` for true or `0` for false) -* `clusterOSImage` (optional string): Either a URL with `http(s)` or `file` scheme to override the default OS image for cluster nodes or an existing Glance image name. +* `clusterOSimage` (optional string): Either a URL with `http(s)` or `file` scheme to override the default OS image for cluster nodes or an existing Glance image name. +* `apiVIP` (optional string): An IP addresss on the machineNetwork that will be assigned to the API VIP. Be aware that the `10` and `11` of the machineNetwork will be taken by neutron dhcp by default, and wont be available. +* `ingressVIP` (optional string): An IP address on the machineNetwork that will be assigned to the ingress VIP. Be aware that the `10` and `11` of the machineNetwork will be taken by neutron dhcp by default, and wont be available. ## Machine pools diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 3d20d99a42..4ff977f803 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -363,27 +363,19 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { if err != nil { return err } - apiVIP, err := openstackdefaults.APIVIP(installConfig.Config.Networking) - if err != nil { - return err - } dnsVIP, err := openstackdefaults.DNSVIP(installConfig.Config.Networking) if err != nil { return err } - ingressVIP, err := openstackdefaults.IngressVIP(installConfig.Config.Networking) - if err != nil { - return err - } data, err = openstacktfvars.TFVars( masters[0].Spec.ProviderSpec.Value.Object.(*openstackprovider.OpenstackProviderSpec), installConfig.Config.Platform.OpenStack.Cloud, installConfig.Config.Platform.OpenStack.ExternalNetwork, installConfig.Config.Platform.OpenStack.ExternalDNS, installConfig.Config.Platform.OpenStack.LbFloatingIP, - apiVIP.String(), + installConfig.Config.Platform.OpenStack.APIVIP, dnsVIP.String(), - ingressVIP.String(), + installConfig.Config.Platform.OpenStack.IngressVIP, installConfig.Config.Platform.OpenStack.TrunkSupport, installConfig.Config.Platform.OpenStack.OctaviaSupport, string(*rhcosImage), diff --git a/pkg/asset/ignition/machine/node.go b/pkg/asset/ignition/machine/node.go index 0b4be65349..8f500bc7e9 100644 --- a/pkg/asset/ignition/machine/node.go +++ b/pkg/asset/ignition/machine/node.go @@ -11,7 +11,6 @@ import ( "github.com/openshift/installer/pkg/types" baremetaltypes "github.com/openshift/installer/pkg/types/baremetal" openstacktypes "github.com/openshift/installer/pkg/types/openstack" - openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults" ovirttypes "github.com/openshift/installer/pkg/types/ovirt" vspheretypes "github.com/openshift/installer/pkg/types/vsphere" ) @@ -29,10 +28,7 @@ func pointerIgnitionConfig(installConfig *types.InstallConfig, rootCA []byte, ro // way to configure DNS before Ignition runs. ignitionHost = net.JoinHostPort(installConfig.BareMetal.APIVIP, "22623") case openstacktypes.Name: - apiVIP, err := openstackdefaults.APIVIP(installConfig.Networking) - if err == nil { - ignitionHost = net.JoinHostPort(apiVIP.String(), "22623") - } + ignitionHost = net.JoinHostPort(installConfig.OpenStack.APIVIP, "22623") case ovirttypes.Name: ignitionHost = net.JoinHostPort(installConfig.Ovirt.APIVIP, "22623") case vspheretypes.Name: diff --git a/pkg/asset/manifests/infrastructure.go b/pkg/asset/manifests/infrastructure.go index 887cc5ffdb..cef81a46a8 100644 --- a/pkg/asset/manifests/infrastructure.go +++ b/pkg/asset/manifests/infrastructure.go @@ -124,22 +124,14 @@ func (i *Infrastructure) Generate(dependencies asset.Parents) error { config.Status.PlatformStatus.Type = configv1.NonePlatformType case openstack.Name: config.Status.PlatformStatus.Type = configv1.OpenStackPlatformType - apiVIP, err := openstackdefaults.APIVIP(installConfig.Config.Networking) - if err != nil { - return err - } dnsVIP, err := openstackdefaults.DNSVIP(installConfig.Config.Networking) if err != nil { return err } - ingressVIP, err := openstackdefaults.IngressVIP(installConfig.Config.Networking) - if err != nil { - return err - } config.Status.PlatformStatus.OpenStack = &configv1.OpenStackPlatformStatus{ - APIServerInternalIP: apiVIP.String(), + APIServerInternalIP: installConfig.Config.OpenStack.APIVIP, NodeDNSIP: dnsVIP.String(), - IngressIP: ingressVIP.String(), + IngressIP: installConfig.Config.OpenStack.IngressVIP, } case vsphere.Name: config.Status.PlatformStatus.Type = configv1.VSpherePlatformType diff --git a/pkg/asset/tls/mcscertkey.go b/pkg/asset/tls/mcscertkey.go index c9895f6c3d..2f01c150ed 100644 --- a/pkg/asset/tls/mcscertkey.go +++ b/pkg/asset/tls/mcscertkey.go @@ -9,7 +9,6 @@ import ( "github.com/openshift/installer/pkg/asset/installconfig" baremetaltypes "github.com/openshift/installer/pkg/types/baremetal" openstacktypes "github.com/openshift/installer/pkg/types/openstack" - openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults" ovirttypes "github.com/openshift/installer/pkg/types/ovirt" vspheretypes "github.com/openshift/installer/pkg/types/vsphere" ) @@ -50,12 +49,8 @@ func (a *MCSCertKey) Generate(dependencies asset.Parents) error { cfg.IPAddresses = []net.IP{net.ParseIP(installConfig.Config.BareMetal.APIVIP)} cfg.DNSNames = []string{hostname, installConfig.Config.BareMetal.APIVIP} case openstacktypes.Name: - apiVIP, err := openstackdefaults.APIVIP(installConfig.Config.Networking) - if err != nil { - return err - } - cfg.IPAddresses = []net.IP{apiVIP} - cfg.DNSNames = []string{hostname, apiVIP.String()} + cfg.IPAddresses = []net.IP{net.ParseIP(installConfig.Config.OpenStack.APIVIP)} + cfg.DNSNames = []string{hostname, installConfig.Config.OpenStack.APIVIP} case ovirttypes.Name: cfg.IPAddresses = []net.IP{net.ParseIP(installConfig.Config.Ovirt.APIVIP)} cfg.DNSNames = []string{hostname, installConfig.Config.Ovirt.APIVIP} diff --git a/pkg/types/defaults/installconfig.go b/pkg/types/defaults/installconfig.go index ba9ed26c96..629bf0ae48 100644 --- a/pkg/types/defaults/installconfig.go +++ b/pkg/types/defaults/installconfig.go @@ -76,7 +76,7 @@ func SetInstallConfigDefaults(c *types.InstallConfig) { case c.Platform.Libvirt != nil: libvirtdefaults.SetPlatformDefaults(c.Platform.Libvirt) case c.Platform.OpenStack != nil: - openstackdefaults.SetPlatformDefaults(c.Platform.OpenStack) + openstackdefaults.SetPlatformDefaults(c.Platform.OpenStack, c.Networking) case c.Platform.VSphere != nil: vspheredefaults.SetPlatformDefaults(c.Platform.VSphere, c) case c.Platform.BareMetal != nil: diff --git a/pkg/types/defaults/installconfig_test.go b/pkg/types/defaults/installconfig_test.go index 90345fc538..7afc96f5ec 100644 --- a/pkg/types/defaults/installconfig_test.go +++ b/pkg/types/defaults/installconfig_test.go @@ -68,7 +68,7 @@ func defaultLibvirtInstallConfig() *types.InstallConfig { func defaultOpenStackInstallConfig() *types.InstallConfig { c := defaultInstallConfig() c.Platform.OpenStack = &openstack.Platform{} - openstackdefaults.SetPlatformDefaults(c.Platform.OpenStack) + openstackdefaults.SetPlatformDefaults(c.Platform.OpenStack, c.Networking) return c } diff --git a/pkg/types/openstack/defaults/platform.go b/pkg/types/openstack/defaults/platform.go index 9852933060..4ea513e7f1 100644 --- a/pkg/types/openstack/defaults/platform.go +++ b/pkg/types/openstack/defaults/platform.go @@ -16,21 +16,31 @@ const ( ) // SetPlatformDefaults sets the defaults for the platform. -func SetPlatformDefaults(p *openstack.Platform) { +func SetPlatformDefaults(p *openstack.Platform, n *types.Networking) { if p.Cloud == "" { p.Cloud = os.Getenv("OS_CLOUD") if p.Cloud == "" { p.Cloud = DefaultCloudName } } -} + // APIVIP returns the internal virtual IP address (VIP) put in front + // of the Kubernetes API server for use by components inside the + // cluster. The DNS static pods running on the nodes resolve the + // api-int record to APIVIP. + if p.APIVIP == "" { + vip, _ := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 5) + p.APIVIP = vip.String() + } -// APIVIP returns the internal virtual IP address (VIP) put in front -// of the Kubernetes API server for use by components inside the -// cluster. The DNS static pods running on the nodes resolve the -// api-int record to APIVIP. -func APIVIP(networking *types.Networking) (net.IP, error) { - return cidr.Host(&networking.MachineNetwork[0].CIDR.IPNet, 5) + // IngressVIP returns the internal virtual IP address (VIP) put in + // front of the OpenShift router pods. This provides the internal + // accessibility to the internal pods running on the worker nodes, + // e.g. `console`. The DNS static pods running on the nodes resolve + // the wildcard apps record to IngressVIP. + if p.IngressVIP == "" { + vip, _ := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 7) + p.IngressVIP = vip.String() + } } // DNSVIP returns the internal virtual IP address (VIP) put in front @@ -40,12 +50,3 @@ func APIVIP(networking *types.Networking) (net.IP, error) { func DNSVIP(networking *types.Networking) (net.IP, error) { return cidr.Host(&networking.MachineNetwork[0].CIDR.IPNet, 6) } - -// IngressVIP returns the internal virtual IP address (VIP) put in -// front of the OpenShift router pods. This provides the internal -// accessibility to the internal pods running on the worker nodes, -// e.g. `console`. The DNS static pods running on the nodes resolve -// the wildcard apps record to IngressVIP. -func IngressVIP(networking *types.Networking) (net.IP, error) { - return cidr.Host(&networking.MachineNetwork[0].CIDR.IPNet, 7) -} diff --git a/pkg/types/openstack/platform.go b/pkg/types/openstack/platform.go index 183cb92f7b..298559d068 100644 --- a/pkg/types/openstack/platform.go +++ b/pkg/types/openstack/platform.go @@ -43,4 +43,14 @@ type Platform struct { // the default OS image for cluster nodes, or an existing Glance image name. // +optional ClusterOSImage string `json:"clusterOSImage,omitempty"` + + // APIVIP is the static IP on the nodes subnet that the api port for openshift will be assigned + // Default: will be set to the 5 on the first entry in the machineNetwork CIDR + // +optional + APIVIP string `json:"apiVIP,omitempty"` + + // IngressVIP is the static IP on the nodes subnet that the apps port for openshift will be assigned + // Default: will be set to the 7 on the first entry in the machineNewtwork CIDR + // +optional + IngressVIP string `json:"ingressVIP,omitempty"` } diff --git a/pkg/types/openstack/validation/platform.go b/pkg/types/openstack/validation/platform.go index 7b6ad4edd6..1eb5761a3e 100644 --- a/pkg/types/openstack/validation/platform.go +++ b/pkg/types/openstack/validation/platform.go @@ -2,6 +2,7 @@ package validation import ( "errors" + "net" "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/util/validation/field" @@ -61,10 +62,20 @@ func ValidatePlatform(p *openstack.Platform, n *types.Networking, fldPath *field for _, ip := range p.ExternalDNS { if err := validate.IP(ip); err != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("ExternalDNS"), p.ExternalDNS, err.Error())) + allErrs = append(allErrs, field.Invalid(fldPath.Child("externalDNS"), p.ExternalDNS, err.Error())) } } + err = validateVIP(p.APIVIP, n) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("apiVIP"), p.APIVIP, err.Error())) + } + + err = validateVIP(p.IngressVIP, n) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("ingressVIP"), p.IngressVIP, err.Error())) + } + return allErrs } @@ -76,3 +87,16 @@ func isValidValue(s string, validValues []string) bool { } return false } + +func validateVIP(vip string, n *types.Networking) error { + if vip != "" { + if err := validate.IP(vip); err != nil { + return err + } + + if !n.MachineNetwork[0].CIDR.Contains(net.ParseIP(vip)) { + return errors.New("IP is not in the machineNetwork") + } + } + return nil +} diff --git a/pkg/types/openstack/validation/platform_test.go b/pkg/types/openstack/validation/platform_test.go index 55971cbef4..ceea79670b 100644 --- a/pkg/types/openstack/validation/platform_test.go +++ b/pkg/types/openstack/validation/platform_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/openstack" "github.com/openshift/installer/pkg/types/openstack/validation/mock" @@ -21,10 +22,20 @@ func validPlatform() *openstack.Platform { } } +func validNetworking() *types.Networking { + return &types.Networking{ + NetworkType: "OpenShiftSDN", + MachineNetwork: []types.MachineNetworkEntry{{ + CIDR: *ipnet.MustParseCIDR("10.0.0.0/16"), + }}, + } +} + func TestValidatePlatform(t *testing.T) { cases := []struct { name string platform *openstack.Platform + networking *types.Networking noClouds bool noNetworks bool noFlavors bool @@ -33,9 +44,10 @@ func TestValidatePlatform(t *testing.T) { valid bool }{ { - name: "minimal", - platform: validPlatform(), - valid: true, + name: "minimal", + platform: validPlatform(), + networking: validNetworking(), + valid: true, }, { name: "missing cloud", @@ -44,7 +56,8 @@ func TestValidatePlatform(t *testing.T) { p.Cloud = "" return p }(), - valid: false, + networking: validNetworking(), + valid: false, }, { name: "missing external network", @@ -53,7 +66,8 @@ func TestValidatePlatform(t *testing.T) { p.ExternalNetwork = "" return p }(), - valid: false, + networking: validNetworking(), + valid: false, }, { name: "valid default machine pool", @@ -62,7 +76,8 @@ func TestValidatePlatform(t *testing.T) { p.DefaultMachinePlatform = &openstack.MachinePool{} return p }(), - valid: true, + networking: validNetworking(), + valid: true, }, { name: "non IP external dns", @@ -73,7 +88,8 @@ func TestValidatePlatform(t *testing.T) { } return p }(), - valid: false, + networking: validNetworking(), + valid: false, }, { name: "valid external dns", @@ -84,38 +100,104 @@ func TestValidatePlatform(t *testing.T) { } return p }(), - valid: true, + networking: validNetworking(), + valid: true, }, { - name: "clouds fetch failure", - platform: validPlatform(), - noClouds: true, - valid: false, + name: "clouds fetch failure", + platform: validPlatform(), + networking: validNetworking(), + noClouds: true, + valid: false, }, { name: "networks fetch failure", platform: validPlatform(), + networking: validNetworking(), noNetworks: true, valid: false, }, { - name: "flavors fetch failure", - platform: validPlatform(), - noFlavors: true, - valid: false, + name: "flavors fetch failure", + platform: validPlatform(), + networking: validNetworking(), + noFlavors: true, + valid: false, }, { - name: "network extensions fetch failure", - platform: validPlatform(), - noNetExts: true, - valid: true, + name: "network extensions fetch failure", + platform: validPlatform(), + networking: validNetworking(), + noNetExts: true, + valid: true, }, { name: "service catalog fetch failure", platform: validPlatform(), + networking: validNetworking(), noServiceCatalog: true, valid: true, }, + { + name: "valid custom API vip", + platform: func() *openstack.Platform { + p := validPlatform() + p.APIVIP = "10.0.0.9" + return p + }(), + networking: validNetworking(), + valid: true, + }, + { + name: "incorrect network custom API vip", + platform: func() *openstack.Platform { + p := validPlatform() + p.APIVIP = "11.1.0.5" + return p + }(), + networking: validNetworking(), + valid: false, + }, + { + name: "valid custom ingress vip", + platform: func() *openstack.Platform { + p := validPlatform() + p.IngressVIP = "10.0.0.9" + return p + }(), + networking: validNetworking(), + valid: true, + }, + { + name: "incorrect network custom ingress vip", + platform: func() *openstack.Platform { + p := validPlatform() + p.IngressVIP = "11.1.0.5" + return p + }(), + networking: validNetworking(), + valid: false, + }, + { + name: "invalid network custom ingress vip", + platform: func() *openstack.Platform { + p := validPlatform() + p.IngressVIP = "banana" + return p + }(), + networking: validNetworking(), + valid: false, + }, + { + name: "invalid network custom API vip", + platform: func() *openstack.Platform { + p := validPlatform() + p.APIVIP = "banana" + return p + }(), + networking: validNetworking(), + valid: false, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -170,7 +252,7 @@ func TestValidatePlatform(t *testing.T) { testConfig := types.InstallConfig{} testConfig.ObjectMeta.Name = "test" - err := ValidatePlatform(tc.platform, nil, field.NewPath("test-path"), fetcher, &testConfig).ToAggregate() + err := ValidatePlatform(tc.platform, tc.networking, field.NewPath("test-path"), fetcher, &testConfig).ToAggregate() if tc.valid { assert.NoError(t, err) } else {