operator/ingress: Add load balancer type

[Miciah]

Add a field to ingress controllers' endpoint publishing strategy to allow specifying parameters for load balancers. Define a parameter to allow choosing between an internal or an external load balancer.

This PR resolves NE-186.

• operator/v1/types_ingress.go (LoadBalancerType): New type.
  (InternalLoadBalancer, ExternalLoadBalancer): New values.
  (LoadBalancerStrategy): New type for load balancer parameters.
  (EndpointPublishingStrategy): Add loadBalancer field.
• operator/v1/zz_generated.deepcopy.go:
• operator/v1/zz_generated.swagger_doc_generated.go: Regenerate.

[ironcladlou]

I'm trying to think of any other possible future types which might be in conflict with this. For example, is there some other "type" of load balancer that must also be internal? If so, would that indicate type is too broad a term? Compared to say, "scope" or "visibility"?

[Miciah]

We went with a string type instead of a Boolean value because we don't know what possible future types there may be. GCP uses the annotation cloud.google.com/load-balancer-type=Internal, AWS uses service.beta.kubernetes.io/aws-load-balancer-internal=0.0.0.0/0, and the other clouds use variations on somethingsomething-internal-somethingsomething=true.

[ironcladlou]

I was probably unclear. I wasn't suggesting a boolean type for Type, I was wondering if Type should be Scope or Visibility or something to that effect.

[sttts]

For compatibiliy with https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190325-unions.md, you need structs for each discriminator value:

Type LoadBalancerType `json:"type"`
External *External `json:"external,omitempty"`
Internal *Internal `json:"internal,omitempty"`

I don't think discriminator only (or partial sub-structs) will work.

[ironcladlou]

Is Internal/External a type of LoadBalancerStrategy as opposed to (for example) a property of or a policy applied to a LoadBalancerStrategy? Seems to me more property-like (hence Scope or Visibility as alternatives to Type).

[ironcladlou]

I don't know that we actually need a union type here. If not, my only proposed change is:

type LoadBalancerStrategy struct {
	// scope indicates the visibility scope of the load balancer.  Possible values are
	// "External" and "Internal".  The default is "External".
	Scope LoadBalancerScope `json:"scope"`
}

The PR currently proposes "Type" instead of "Scope", which 1. by the existing naming conventions seems to imply a union type (which this PR does not propose), and 2. by extension of the union type implication, implies a taxonomy of kinds of load balancers rather than one of many properties of load balancers (I currently think the latter is more accurate). I hope I described that in a way that makes sense...

[Miciah]

Latest push makes the following changes:

• Change "type" to "scope" in the types and fields that this PR adds.
• Define empty HostNetworkStrategy and PrivateStrategy struct types.
• Using these new types, add hostNetwork and private union fields to EndpointPublishingStrategy, so that each defined value for the type discriminator of EndpointPublishingStrategy has a corresponding union field.

[ironcladlou]

@deads2k @sttts @damemi PTAL to make sure we're getting the union right?

[Miciah]

/hold

[Miciah]

/hold cancel

[Miciah]

Rebased.

[Miciah]

Latest push marks union fields with // +nullable.

[ironcladlou]

/lgtm

[deads2k]

An empty config should work. I think this needs to be tagged optional

[deads2k]

Just the comment about the optionalness of a field. lgtm otherwise.

[Miciah]

Latest push marks scope as optional.

[deads2k]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, ironcladlou, Miciah

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 [deads2k]

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