OpenStack: Custom API and Ingress vip addresses #3366

iamemilio · 2020-03-27T18:36:46Z

Customers may want to plumb the networking that enables external access to the cluster in a number of ways. To make their job easier, this feature allows them to select fixed IP addresses that they can reach the API and apps ingress at in their OpenShift cluster. This allows them to use/reuse pre-existing routing and external access schemes more easily. This adds an additional optional set of values to the openstack platform section of the install config as follows:

platform:
  openstack:
    cloud: ...
    apiVIP: "10.0.0.35"
    ingressVIP: "10.0.0.19"

Note that the default values have not changed. APIVIP still defaults to the 5 on the machineNetwork, and IngressVIP still defaults to the 7.

iamemilio · 2020-03-27T19:08:28Z

/cc @Fedosin

iamemilio · 2020-03-27T19:26:06Z

/label platform/openstack

jstuever · 2020-03-30T23:34:15Z

/cc @adduarte
/uncc @jstuever

iamemilio · 2020-03-31T14:00:03Z

/retest

Fedosin · 2020-03-31T17:18:19Z

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


APIVIP is the static IP...

Fedosin · 2020-03-31T17:18:46Z

pkg/types/openstack/platform.go

+	// +optional
+	APIVIP string `json:"apiVIP,omitempty"`
+
+	// ingressVIP is the static IP on the nodes subnet that the apps port for openshift will be assigned


IngressVIP is the static IP...

Fedosin · 2020-03-31T17:22:18Z

pkg/types/openstack/validation/platform.go

+
+	if p.IngressVIP != "" {
+		if err := validate.IP(p.IngressVIP); err != nil {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("IngressVIP"), p.IngressVIP, err.Error()))


fldPath.Child("ingressVIP")

Fedosin · 2020-03-31T17:22:51Z

pkg/types/openstack/validation/platform.go

+		}
+
+		if !n.MachineNetwork[0].CIDR.Contains(net.ParseIP(p.IngressVIP)) {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("IngressVIP"), p.IngressVIP, "IP is not in the nodesNetwork"))


fldPath.Child("ingressVIP")

iamemilio · 2020-03-31T17:52:34Z

/hold race condition for ports

iamemilio · 2020-04-01T20:05:59Z

/hold cancel

iamemilio · 2020-04-01T20:29:40Z

/retest

Fedosin · 2020-04-03T17:59:24Z

/lgtm

abhinavdahiya · 2020-04-08T06:27:42Z

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 machinesNetwork CIDR


machinesNetwork -> machineNetwork

also it seems like we pick the 5th host in the first network range of the list.

abhinavdahiya · 2020-04-08T06:28:35Z

pkg/types/openstack/validation/platform.go

+			allErrs = append(allErrs, field.Invalid(fldPath.Child("apiVIP"), p.APIVIP, err.Error()))
+		}
+		if !n.MachineNetwork[0].CIDR.Contains(net.ParseIP(p.APIVIP)) {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("apiVIP"), p.APIVIP, "IP is not in the machineNetwork"))


machineNetwork[0]

abhinavdahiya · 2020-04-08T06:29:39Z

pkg/types/openstack/validation/platform.go

+		if err := validate.IP(p.APIVIP); err != nil {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("apiVIP"), p.APIVIP, err.Error()))
+		}
+		if !n.MachineNetwork[0].CIDR.Contains(net.ParseIP(p.APIVIP)) {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("apiVIP"), p.APIVIP, "IP is not in the machineNetwork"))


the 2 step check is same for API and ingress VIP, so maybe use one function for both..

abhinavdahiya · 2020-04-08T06:30:12Z

pkg/types/openstack/validation/platform_test.go

+		MachineNetwork: []types.MachineNetworkEntry{
+			{
+				CIDR: *ipnet.MustParseCIDR("10.0.0.0/16"),
+			},
+		},


Suggested change

MachineNetwork: []types.MachineNetworkEntry{

{

CIDR: *ipnet.MustParseCIDR("10.0.0.0/16"),

},

},

MachineNetwork: []types.MachineNetworkEntry{{

CIDR: *ipnet.MustParseCIDR("10.0.0.0/16"),

}},

abhinavdahiya · 2020-04-08T06:30:46Z

pkg/types/openstack/validation/platform_test.go

+			name: "invalid network custom ingress vip",
+			platform: func() *openstack.Platform {
+				p := validPlatform()
+				p.IngressVIP = "banana"


abhinavdahiya · 2020-04-08T06:32:33Z

few nits, but i'll leave them upto openstack-approvers

/approve

/hold just to give people some time to fix the nits, please free to remove.

openshift-ci-robot · 2020-04-08T06:33:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [abhinavdahiya]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

co-authored-by: Fedosin mfedosin@redhat.com

Fedosin · 2020-04-08T13:37:56Z

/lgtm

Fedosin · 2020-04-08T14:44:21Z

/retest

iamemilio · 2020-04-08T15:33:32Z

/hold cancel

openshift-bot · 2020-04-08T15:59:02Z

/retest