Add status field to TransportServer resource

[soneillf5]

Proposed changes

This change adds the status field to the TransportServer resource.
This enables a user to get the status of the TransportServer i.e.

$ kubectl get ts
NAME      STATE   REASON           IP    PORTS   AGE
dns-tcp   Valid   AddedOrUpdated                 54m

Checklist

Before creating a PR, run through this checklist and mark each as complete.

• I have read the CONTRIBUTING doc
• I have added tests that prove my fix is effective or that my feature works
• I have checked that all unit tests pass after adding my changes
• I have updated necessary documentation
• I have rebased my branch onto master
• I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

[pleshakov]

Hi @soneillf5

Please see my feedback.

With TransportServer, it is a bit trickier than with VirtualServer: VirtualServer can have one or two ports. For example it can have port 80 or ports 80 and 443. However, with TransportServer, we only have one port, which corresponds to the port of the listener.

For TCP/UDP TransportServer , it only makes sense to report IP and port when:

• external-status-address is set in the ConfigMap (we assume that address exposes all ports)
• the type LoadBalancer service, configured for KIC and used via -external-service, configures a port and protocol that matches the port and protocol of the listener used in the TransportServer. So if the Services exposes port 53, we report the service public IPs and port 53 for a TransportServer that has a listener that uses port 53. Otherwise, if the port is not configured in the Service, it will not be accessible via the allocated cloud LoadBalancer.

For TLS Passthrough TransportServer, which uses port 443, it makes sense to report IP and port in the status when:

• external-status-address is set in the ConfigMap
• there is a type LoadBalancer services, configured for KIC and used via -external-service. The services we provide in the installation manifests, helm and operator, have ports 80 and 443 configured.
• IngressLink is configured https://github.com/nginxinc/kubernetes-ingress/blob/master/docs-web/integration-with-cis.md (IngressLink supports ports 80 and 443)

[soneillf5]

Hi @pleshakov, thank for the review. I've addressed most of the comments.

The outstanding issue is your comment about reporting the external IP/Port.
You state 'it only makes sense to report the IP/host' under certain conditions.
My understanding is that the output of k get ts depends on the CRD k8s.nginx.org_transportservers.yaml which we don't change at runtime? If that's the case, how do we change what we report?

Or do you mean that we should check for all of the conditions you mentioned?

[soneillf5]

The IP and Port fields are a lot more nuanced and involved, causing a lot of rework on this PR.

I've removed the two fields and logic required for them. If they are still required, we can add them in a new PR.

@pleshakov

[soneillf5]

@pleshakov updated and ready to merge

[pleshakov]

@soneillf5 lgtm

(but I can't find this though #1425 (comment) )