TrafficSplit: Clarify root service can be any addressable name that resolves to an application accessible by the mesh #223
Conversation
johnsonshi
requested review from
bridgetkromhout,
grampelberg,
ilevine,
lachie83,
leecalcote,
mhausenblas,
michelleN,
nicholasjackson,
Pothulapati,
slack and
stefanprodan
as code owners
|
@michelleN I've opened this PR to address #215. |
johnsonshi
changed the title
johnsonshi
changed the title
|
@stefanprodan - I can see where you're coming from. This proposal came from a conversation that @johnsonshi and I had about TrafficSplit. I asked him to open a PR with the change so we could use it to discuss. This can probably be broken into two PRs (1) Clarifying that the root service does not need to be a Kubernetes service and (2) The proposed change of changing the service key to "fqdn". Thoughts on this are welcome. Thanks @johnsonshi for the PR. |
|
@michelleN this PR went in the opposite direction, we already released v1alpha4 https://github.com/servicemeshinterface/smi-sdk-go/tree/main/pkg/apis/split/v1alpha4 so such a change can't happen here but in v1alpha5. I also think we should't be renaming fields, in v1alpha5 we could add the |
|
Maybe a better option would be to point the |
|
Hi @michelleN and @stefanprodan, as per the last SMI community meeting, we decided to split up this PR into two. This PR is the first one, which is the uncontroversial PR that clarifies the language. We can open another PR (replacing https://docs.google.com/document/d/1NTBaJf6LhUBlF8_lfvBBt_MbyPvT-6CZNg6Ckpm_yCo/edit# |
stefanprodan
changed the title
|
@johnsonshi @michelleN have you considered my proposal?
|
|
That's a great idea @stefanprodan. I'll look up info on k8s ExternalName services and see what can be done. Thanks! |
|
@stefanprodan - I don't think we'd be breaking the SMI API by updating this language. I think the original intention was always to use the service key as an fqdn the way @johnsonshi has described. Please correct me if I'm wrong @grampelberg @nicholasjackson and others. I think we'd be making things a little more complicated for the end user by making them use ExternalName Service. |
|
Service was always designed to be a loose coupling in the original spec, this would allow for virtual services that only have meaning to the service mesh, external services such as those running on VMs, and other clusters. As far as I am aware there is no validation that attempts to tie service to a Kubernetes service in any of the current implementations. Not sure we needed to enforce this either as this does not really break the API as it was always a string. |
|
Reading back through this though, are we confident that this needs to be an FQDN? For example, in Consul you can define a service that is addressable just by its name from within the data center (cluster). E.G. If I were to implement the proposal to the letter I would have to add a fake TLD In the instance that the service mesh just supports simple service names and does not support multi-cluster or TLDs, does enforcing FQDN make sense. I agree with the sentiment in the PR to clean up the use but I honestly think that this just needs to be something like |
|
@nicholasjackson Those are really good points and I am for using the wording you suggested. |
johnsonshi
changed the title
|
Incorporated wording as suggested by @nicholasjackson into my PR. Thanks! |
|
Thanks @johnsonshi ... We've updated our contributing guidelines to reflect a new process. Contributions / updates to the spec will now be taken on the |
johnsonshi
dismissed
stefanprodan’s stale review
The base branch was changed.
There was a problem hiding this comment.
Changes look great @johnsonshi . Thank you.
Could you move these changes to apis/traffic-split/traffic-split.md?
TrafficSplit: Clarify root service can be any addressable name that resolves to an application accessible by the mesh. The TrafficSplit root service can either be a (1) Kubernetes service that resolves to the desired root service (such as `service-name` or `service-name.service-namespace`) (2) other endpoints/FQDNs that resolve to the desired root service (such as `foo.com`) (3) any other addressable name for an application accessible by the service mesh. Resolves #215. Signed-off-by: Johnson Shi <[email protected]>
|
@michelleN done. |
TrafficSplit: Clarify root service can be any addressable name that resolves to an application accessible by the mesh
This clarifies that the TrafficSplit root
can be referenced through an FQDN.
The TrafficSplit root FQDN can either be a
(1) Kubernetes service that resolves
to the FQDN (such as service.service-namespace)
(2) other FQDNs (such as foo.com)
Resolves #215.
Signed-off-by: Johnson Shi [email protected]