Why does GCE ingress defer promoting static IP

[bowei]

From @tonglil on March 8, 2017 17:40

Something I have thought about for a while but haven't been able to reason about: https://github.com/kubernetes/ingress/blob/master/controllers/gce/loadbalancers/loadbalancers.go#L603-L608.

This seems like both :80 and :443 needs to be set before an IP can be promoted to static. Is there a reason why? I've git blamed and the past doesn't show a good reason for it either: kubernetes-retired/contrib@a2f82d5#diff-0ca32fb01d30b7ffd4f37d77adcce2e5R483.

I think the ip should either: always be promoted to static, or by a separate annotation to do so when the ingress creates an IP.

It's a bad UX currently when the user has to run another gcloud command/go into the console to promote the address when they only want HTTPS (via kubernetes.io/ingress.allow-http: "false").

Thoughts?

Copied from original issue: kubernetes/ingress-nginx#405

[bowei]

From @tonglil on May 8, 2017 23:4

I would like to propose a change (and I can implement it) to fix what seems to be like undesired/confusing static IP behavior:

allowHTTP	TLS enabled (via secret or pre-shared cert)	result	ports
default/true	none	ephemeral	port 80
false	none	ephemeral	no ports
default/true	yes	static	port 80, 443
false	yes	ephemeral	port 443

Ideally case 4 should always be static (if HTTPS is on, it should be static)?

The logic prior to pre-shared cert exhibits the behavior in the table above:

if l.runtimeInfo.AllowHTTP && l.runtimeInfo.TLS != nil {

The current logic also:

if l.runtimeInfo.AllowHTTP && (l.runtimeInfo.TLS != nil || l.runtimeInfo.TLSName != "") {

Proposed: make static IP independent of allowHTTP.

allowHTTP	TLS enabled (via secret or pre-shared cert)	result	ports
default/true	none	ephemeral	port 80
false	none	ephemeral	no ports
default/true	yes	static	port 80, 443
false	yes	static	port 443

if l.runtimeInfo.TLS != nil || l.runtimeInfo.TLSName != "" {

Please let me know if I am missing some implication this may cause.

[bowei]

From @nicksardo on May 8, 2017 23:28

Hey @tonglil, thanks for following up on this. With all the ingress bugs, this has been low priority.

I'm sure you know that it's necessary to have a static ip for the third case to work at all. But the fourth case doesn't mandate a static ip, right? Why does HTTPS mandate a static ip? I believe the original ingress author chose this logic because static IPs are a limited quantity; therefore, should be created at a last resort.

If I were running a production service via ingress, I would manually create a static IP and set it via annotation to relevant ingresses. I wouldn't want to risk any controller deleting that IP and waiting for the TTL to expire on a new address.

Thoughts?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

[tonglil]

If I were running a production service via ingress, I would manually create a static IP and set it via annotation to relevant ingresses. I wouldn't want to risk any controller deleting that IP and waiting for the TTL to expire on a new address.

I agree with this.

I still think regardless of if allowHTTP is true or false, if TLS is enabled, the IP should be promoted to "static" so that no controller can delete the IP.

However, the IP is not promoted if allowHTTP is false, yet promoted if it is true.

[tonglil]

/remove-lifecycle rotten

[floyd-may]

This is very insightful commentary from @bowei, IMHO:

If I were running a production service via ingress, I would manually create a static IP and set it via annotation to relevant ingresses. I wouldn't want to risk any controller deleting that IP and waiting for the TTL to expire on a new address.

For my part, some clear messaging in the events to that end would be helpful. I think I've found a corner case where no IP is getting created and the Ingress resource is stuck forever in limbo. However, the above quote makes sense, and I'm going to change my strategy to provision the IP before creating a new Ingress resource.

[nicksardo]

A static IP isn't always allocated because of quota concerns. I don't think this should change regardless of whether the ingress is HTTPS or not.
/close

@floyd-may these comments were copied by that account to this repo. Note that he isn't the original author of most of these comments.