Add explanation for why ports are on Services instead of TrafficSplit

[grampelberg]

Closes #14

[grampelberg]

@stefanprodan how does this look? It feels like the port would be needed for the apex service as well. I'd prefer to not have it there, but I don't know there's a way around it.

A way around this entirely would be to just maintain the ports (eg. web:8080 -> web-next:8080) implicitly. I'm not a fan of that as it isn't how you'd expect to use a k8s service.

[nicholasjackson]

I think the port also needs to be defined as an optional property, Consul connect does not expose ports to the end user as they do not need them. The public port for the data plane is stored in the service catalog which Envoy reads from the Endpoint Discovery Service.

I have not had much chance to comment in general on Traffic Split as this is going to be a 1.6 feature for consul connect. However, the object model at present is that traffic can be split in two different ways.

The first is to an entirely different service name and the second is the same service name which has specific metadata.

e.g.

                service_a[version=1]
service_a 
                service_a[version=2]

                 service_a
service_a
                service_a_new

Will add a PR to suggest metadata selectors in the morning.

[grampelberg]

@adleong you're in the implementation here right now, what do you think?

[grampelberg]

@nicholasjackson I spent some more time thinking about it (and chatting things through with @olix0r) and ended up where you are.

@stefanprodan I've had a rollercoaster of a morning =) I tried to explain why ports don't make sense on TrafficSplit. wdyt?

[stefanprodan]

LGTM

Maybe change the PR title to reflect the content

[stefanprodan]

@grampelberg delegating the ports to services makes sense to me but what about timeouts, retries and CORS policies? If the TrafficSplit is a subset of VirtualServices maybe these settings should be delegated to some other object type like app profiles?

[grampelberg]

@stefanprodan great question! Check out #16 (https://github.com/deislabs/smi-spec/blob/be15236286090768794af46731dccd02eaef64e0/traffic-policy.md#httpservice). I'm not sure that's 100% of the way there yet, feels a little odd right now. That said, there definitely needs to be a collection of routes that can take the timeout/retry policies.

CORS is an odd one, that feels like an ingress concern primarily. Is that hung off VirtualServices?

ps. thank you for reminding me the commit was wrong now!

[stefanprodan]

@grampelberg here is a VirtualService example of how Flagger drives a canary deployment. The virtual service generated by Flagger has timeout/retry/cors policies, uri rewrite and prefix:

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: frontend
  namespace: test
  ownerReferences:
    - apiVersion: flagger.app/v1alpha3
      blockOwnerDeletion: true
      controller: true
      kind: Canary
      name: podinfo
      uid: 3a4a40dd-3875-11e9-8e1d-42010a9c0fd1
spec:
  gateways:
    - public-gateway.istio-system.svc.cluster.local
    - mesh
  hosts:
    - frontend.example.com
    - frontend
  http:
  - appendHeaders:
      x-envoy-max-retries: "10"
      x-envoy-retry-on: gateway-error,connect-failure,refused-stream
      x-envoy-upstream-rq-timeout-ms: "15000"
    corsPolicy:
      allowHeaders:
      - x-some-header
      allowMethods:
      - GET
      allowOrigin:
      - example.com
      maxAge: 24h
    match:
    - uri:
        prefix: /api
    rewrite:
      uri: /
    route:
    - destination:
        host: podinfo-primary
        port:
          number: 9898
      weight: 100
    - destination:
        host: podinfo-canary
        port:
          number: 9898
      weight: 0

[grampelberg]

@stefanprodan what's the relationship between gateways and virtual services? A lot of this I'd assume would go on the gateway and not the service itself.