Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation for catalog sync w/ K8S #4710

Merged
merged 8 commits into from
Sep 26, 2018
Merged

Documentation for catalog sync w/ K8S #4710

merged 8 commits into from
Sep 26, 2018

Conversation

mitchellh
Copy link
Contributor

This adds the documentation for the upcoming catalog sync functionality.

(Sorry, short PR description because that's about it!)

@mitchellh mitchellh requested review from anubhavmishra, adilyse and a team September 25, 2018 22:55

The service sync is done using an external long-running process in the
[consul-k8s project](https://github.com/hashicorp/consul-k8s). This process
can run both in or out of a Kubernetes cluster. However, running this within
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"This process can run both in or out of a Kubernetes cluster."

Should there be one instance of the process running at a time, or does it require two versions (one internal to kubernetes and one external)? It's not entirely clear from the wording.

If only one is needed, changing both -> either might be more clear.


**Why sync Kubernetes services to Consul?** Kubernetes services synced to the
Consul catalog enable Kubernetes services to be accessed by non-Kubernetes
nodes that are part of the Consul cluster or by other distinct Kubernetes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...or by other distinct Kubernetes clusters."

Checking my understanding: these other Kubernetes clusters still need to be part of the Consul cluster for this to work, right?

If so, this might be more clear:
"Kubernetes services synced to the Consul catalog enable Kubernetes services to be accessed by other Consul-registered services, whether they are non-Kubernetes nodes or part of a different Kubernetes cluster."

a static port on all nodes in the cluster, this limits the number of service
instances to be equal to the nodes running the target pods.

The service instances registered will be registered to the K8S node name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an extra registered in this sentence? Reading it, it makes more sense as "The service instances will be registered to the K8s node name..."


By default, all valid services (as explained above) are synced. This default
can be changed as configuration to the sync process. Syncing can also be
explicitly enabled or disabled using an annotation:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to explicitly enable sync for an unsupported kubernetes service type, i.e. ClusterIP?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very curious about this, and what the behavior would be. (For ClusterIP, registering endpoints as service instances in Consul?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since ClusterIP is a virtual IP and choosing it makes the kubernetes service only reachable from within the cluster. This means that nodes/services outside Kubernetes cluster won't be able to reach that service directly anyway, they will have to either use an Ingress controller or other service types like NodePort or LoadBalancer to access the service. It won't make much sense to sync that service to Consul since it is not discoverable externally unless someone is explicitly not trying to use kube-dns internally for communication inside the cluster and wanting to replace kube-dns with Consul DNS, which is unlikely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at launch, but its something we can and should consider for future support as we get more use cases, i.e. that'd work totally fine if the kube-proxy is locally available and running even if its external to the K8S cluster, technically. We just have to learn more about the use case.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we'd actually like to see is pulling in the endpoints of the ClusterIP instead of the ClusterIP itself. (Our pod IPs are on a flat network and so routable directly.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a totally reasonable ask, and we already have the endpoint syncing functionality in there for NodePort. We could add this without huge issue... @roboll I'll try to remember but if you can report this as a feature request on the consul-k8s repo (going OSS in a few hours) that'd be super helpful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mitchellh Using endpoints under the service (type ClusterIP) should work in most clusters with routable pod ips (except clusters running overlay).


### Service Tags

A service registered in Consul from K8S will always have the tag "k8s" added
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...in Consul from K8S..."

Most of the rest of the documentation spells out Kubernetes instead of abbreviating. Should that be the case here as well?


### Service Meta

A service registered in Consul from K8S will set the `external-source` key to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...in Consul from K8S..."

Same question about abbreviation.

"kubernetes". This can be used by API consumers, the UI, CLI, etc. to filter
service instances that are set in k8s. The Consul UI (in Consul 1.2.3 and later)
will read this value to show a Kubernetes icon next to all externally
registered services.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...show a Kubernetes icon next to all externally registered services."

Being very picky, this is technically only true for "externally registered Kubernetes services" though that should hopefully be understood based on the context.

Copy link
Contributor

@adilyse adilyse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it's well structured and very detailed. There were a couple of things I didn't understand, so I asked for some additional information. Also, feel free to take or leave the style commentary as needed :).

Apologies for the single comments vs. linked review comments, I hadn't used this feature in github before.

### Service Name

When a Consul service is synced to Kubernetes, the name of the Kubernetes
service will match exactly within Kubernetes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"... the name of the Kubernetes service will match exactly within Kubernetes."

Shouldn't it match the Consul service name? That is, "the name of the Kubernetes service will exactly match the Consul service name"?

service will match exactly within Kubernetes.

A service name prefix can be specified to the sync program. If specified, this
prefix will be attached to Consul services within Kubernetes. This defaults to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternate wording of the first part of the Service Name section:

"When a Consul service is synced to Kubernetes, the name of the resulting Kubernetes service will exactly match the Consul service name. To change this default exact match behavior, it is possible to specify a prefix to be added to Consul service names within Kubernetes by using the -k8s-service-prefix flag. This can also be specified in the Helm configuration."

@mitchellh
Copy link
Contributor Author

Great feedback thank you! All your feedback was spot on and I've incorporated your suggestions.

@anubhavmishra
Copy link
Member

anubhavmishra commented Sep 26, 2018

I am happy with the docs, I am sure we will hear feedback from folks and we can create an FAQ section to address that in the future.

@mitchellh mitchellh merged commit f680026 into master Sep 26, 2018
@mitchellh mitchellh deleted the f-sync-website branch September 26, 2018 14:43
mitchellh added a commit that referenced this pull request Sep 26, 2018
* website: docs for catalog sync

* website: document consul-k8s

* website: document helm options

* Add new helm fields

* website: fix the description of the sync page

* website: address feedback

* website: clarify coredns requirement

* website: clarify that the server cluster can run anywhere

(NOTE: This was squashed to make cherry-picking to stable-website easier)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants