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

command(s?) in documentation don't work. #144

Open
mikedlr opened this issue Oct 22, 2024 · 12 comments
Open

command(s?) in documentation don't work. #144

mikedlr opened this issue Oct 22, 2024 · 12 comments

Comments

@mikedlr
Copy link

mikedlr commented Oct 22, 2024

In the main documentation there is a command

$ kubectl label node kind-control-plane node.kubernetes.io/exclude-from-external-load-balancers-
node/kind-control-plane unlabeled

This command does not work. I believe that it should be something like

kubectl label node --overwrite kind-2-control-plane node.kubernetes.io/exclude-from-external-load-balancers=true

instead - probably better if the prompt isn't included in the copied text. It might be good if the documents were automatically tested e.g. with https://github.com/JNRowe/shell-doctest

@BenTheElder
Copy link
Member

It might be good if the documents were automatically tested e.g. with https://github.com/JNRowe/shell-doctest

that might get a bit expensive since you'd need the requisites setup.

they're also probably not changing often.

I agree that we should have copyable commands.

@mikedlr
Copy link
Author

mikedlr commented Oct 23, 2024

I think that the answer to this bug is that the wrong highlighting code is in use in the documentation

```sh
prompt$ some command
some result
````
prompt$ some command
some result

should be

```console
prompt$ some command
some result
````
propmt$ some command
some result

https://stackoverflow.com/questions/20303826/how-to-highlight-bash-shell-commands-in-markdown

@zrisher
Copy link
Contributor

zrisher commented Nov 26, 2024

I had this same issue. The command is quite long, and the readme has a maximum width, so at first glance it looks like one long command instead of a command and a response. Personally I would just remove the response, so:

kubectl label node kind-control-plane node.kubernetes.io/exclude-from-external-load-balancers-

Then users can copy it without worrying about amending what they've copied.

@zrisher
Copy link
Contributor

zrisher commented Nov 26, 2024

I also personally felt the documentation around this command was unclear:

Control-plane nodes need to remove the special label node.kubernetes.io/exclude-from-external-load-balancers to be able to access the workloads running on those nodes using a LoadBalancer Service.

Does "these nodes" mean just the control-plane node? Or the nodes it controls? I.e. do I need to run this command if I don't plan on putting a load balancer in front of my control plane? Or is this something I need to do to use this tool at all?

@aojea
Copy link
Contributor

aojea commented Nov 26, 2024

@zrisher how familiar are you with kubernetes? just trying to understand if you are looking at this from a totally new person or with some background

@zrisher
Copy link
Contributor

zrisher commented Nov 26, 2024

First K8S project, a lot of hours in but also many new technologies to research besides just K8S. Definitely the first time I've tried to get a load balancer running on a local cluster.

@aojea
Copy link
Contributor

aojea commented Nov 26, 2024

yeah, I see, this project README assumes some previous knowledge, now I understand why you feel this friction

@zrisher
Copy link
Contributor

zrisher commented Nov 26, 2024

If this configuration step is needed in all cases, I think

We also need to remove the label node.kubernetes.io/exclude-from-external-load-balancers from the control plane node:

is more clear for both the experienced and iniates.

@BenTheElder
Copy link
Member

BenTheElder commented Nov 26, 2024

FWIW I personally think it's generally best practice that examples have a shell prompt ($) and command output and "you should run this command" snippets to only have exactly the command to copy/paste and I would support a PR to correct that.


Does "these nodes" mean just the control-plane node? Or the nodes it controls? I.e. do I need to run this command if I don't plan on putting a load balancer in front of my control plane? Or is this something I need to do to use this tool at all?

There can be more than one control-plane node, which is why it says "these nodes". The nodes with this label.
That label controls excluding nodes from load balancing.

Regarding control plane vs worker nodes ... https://kind.sigs.k8s.io/docs/user/configuration/#nodes

If this configuration step is needed in all cases, I think

It is not required to use the tool. You could only serve traffic to other nodes. It is dependent on your use case.


The official docs for this label are at https://kubernetes.io/docs/reference/labels-annotations-taints/ which we should reference. Specifically https://kubernetes.io/docs/reference/labels-annotations-taints/#node-kubernetes-io-exclude-from-external-load-balancers

The even more specific reference is: https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/create-cluster-kubeadm/#control-plane-node-isolation

@zrisher
Copy link
Contributor

zrisher commented Nov 27, 2024

Thanks @BenTheElder, this clears things up for me. I had read all of this except for that last link - the source of my confusion was that I assumed people would not be scheduling workloads onto their control-plane node. So in the case I envisioned, if the docs say we need to remove node.kubernetes.io/exclude-from-external-load-balancers from the control plane node, it must somehow affect other nodes managed by that control plane.

In fact, as I understand now, you only need to remove this label if you are trying to loadbalance a workload served by a pod that is scheduled onto a control plane node (which is a case I didn't account for).

Given that understanding, I would perhaps suggest:

If you are running workloads on control plane nodes (as is the default kind configuration), you will need to remove the special label node.kubernetes.io/exclude-from-external-load-balancers to access those workloads using a LoadBalancer.

@BenTheElder
Copy link
Member

In fact, as I understand now, you only need to remove this label if you are trying to loadbalance a workload served by a pod that is scheduled onto a control plane node (which is a case I didn't account for).

Yes, that's exactly it. We find users doing all sorts of things with the tools 😅


For single-node-clusters we currently automatically remove the scheduling taint that limits the node to only control plane workloads, we should consider automatically removing this label as well. I'll defer to @aojea on that one though, we can track a feature in kubernetes-sigs/kind if we think that makes sense.

The note would still be relevant in some form for people with other cluster shapes though.

Getting robust cross-platform loadbalancing w/ kind is still a bit new (for a long time we pointed to metallb which worked pretty well on Linux at least), and the first use case is the same as kind: developing Kubernetes itself. We clearly have more to do to flesh things out to be friendly for general purpose (which is definitely supported, but needs more work like this issue).

Thanks for understanding and working with us :-)

@aojea
Copy link
Contributor

aojea commented Nov 28, 2024

Given that understanding, I would perhaps suggest:

If you are running workloads on control plane nodes (as is the default kind configuration), you will need to remove the special label node.kubernetes.io/exclude-from-external-load-balancers to access those workloads using a LoadBalancer.

SGTM please go ahead and file a PR

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

No branches or pull requests

4 participants