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

[FEATURE] Add --labels flags to add a labels to a node #578

Closed
developer-guy opened this issue Apr 27, 2021 · 28 comments · Fixed by #584
Closed

[FEATURE] Add --labels flags to add a labels to a node #578

developer-guy opened this issue Apr 27, 2021 · 28 comments · Fixed by #584
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@developer-guy
Copy link
Contributor

developer-guy commented Apr 27, 2021

Is your feature request related to a problem or a Pull Request

I would like to add labels to my nodes while adding them to a running cluster.

Scope of your request

  • add --labels flag to k3d node create command

Describe the solution you'd like

Add --labels flag to k3d node create (and possibly others) that is passed down to docker without any modification.

Describe alternatives you've considered

@developer-guy developer-guy added the enhancement New feature or request label Apr 27, 2021
@developer-guy
Copy link
Contributor Author

I'm a volunteer to do this btw 🙋🏻‍♂️😉

@iwilltry42
Copy link
Member

Hi @developer-guy , thanks for opening this feature request 👍

I'm a volunteer to do this btw

I'll be more than happy to accept your PR :)

@iwilltry42 iwilltry42 added this to the v4.5.0 milestone Apr 30, 2021
@ejose19
Copy link
Contributor

ejose19 commented Apr 30, 2021

Actually was wondering why neither --label nor labels in yaml file was applying labels to nodes and it seems #163 actually refers to docker labels, IMO it should have been called --docker-label as proposed by the PR submitter, and now that there will be --node-label as well it would avoid ambiguity

@iwilltry42
Copy link
Member

Hi @ejose19 👋
k3d is not touching the Kubernetes part so far in order to avoid dragging in Kubernetes/K3s dependencies.
We may re-think that in the future to do something like we do with CoreDNS configmap patching.
But this issue and the cluster create --label flags only concern docker.

that is passed down to docker without any modification.

@ejose19
Copy link
Contributor

ejose19 commented Apr 30, 2021

Hello @iwilltry42, thanks for clarifying

@developer-guy
Copy link
Contributor Author

@iwilltry42 I opened a pull request btw, thank you 😋🙋🏻‍♂️🎉

@ejose19
Copy link
Contributor

ejose19 commented Apr 30, 2021

@developer-guy I've checked your PR at it clearly says This PR is going to provide users to be able to add labels to the Kubernetes nodes., do you mean an equivalent to kubectl label node $node $label correct? because that's what I meant in the original comment.

Would it pick labels from config.yaml file or something extra needs to be added?

@developer-guy
Copy link
Contributor Author

developer-guy commented Apr 30, 2021

yeah, exactly, "k3s" has a flag called "--node-label" in order to add labels to the nodes while provisioning, so, with this PR we add this support to the k3d. You should specify the "--labels" flag while issuing the command "k3d node create".

@iwilltry42
Copy link
Member

Then I clearly misunderstood what you wrote in the issue text, sorry 🤔

@developer-guy
Copy link
Contributor Author

How can I explain it better? Would you like to give me a suggestion?

@ejose19
Copy link
Contributor

ejose19 commented Apr 30, 2021

I'm really in favor of using --node-label & --docker-label, it's explicit and given all the confusion it's better to play it safe.

Related: I was trying to test your PR but it seems something is broken with native k3d node create, new container just keep restarting, and docker logs outputs this:

[FATAL tini (7)] exec /bin/entrypoint.sh failed: No such file or directory
[FATAL tini (7)] exec /bin/entrypoint.sh failed: No such file or directory
[FATAL tini (7)] exec /bin/entrypoint.sh failed: No such file or directory
[FATAL tini (7)] exec /bin/entrypoint.sh failed: No such file or directory
[FATAL tini (7)] exec /bin/entrypoint.sh failed: No such file or directory
[FATAL tini (7)] exec /bin/entrypoint.sh failed: No such file or directory

This also happens on latest k3d binary, so the issue is not related with your PR.

EDIT: @iwilltry42 Upon inspecting /bin/entrypoint.sh in a working container, that file is related to the #579, but it seems the workaround doesn't inject the entrypoint when using k3d node create

@iwilltry42
Copy link
Member

@developer-guy

How can I explain it better? Would you like to give me a suggestion?

I misunderstood your following line to be referring to docker labels.

that is passed down to docker without any modification.

@ejose19

I'm really in favor of using --node-label & --docker-label, it's explicit and given all the confusion it's better to play it safe.

Agree

Regarding your error: did you set K3D_FIX_CGROUPV2=1 in your environment?
That's only supported for cluster create at the moment.
Follow-up PR for that to land in the next patch release as this is still testing (and waiting for k3s confirmation).

@ejose19
Copy link
Contributor

ejose19 commented Apr 30, 2021

@iwilltry42 yes, good that you also submitted iwilltry42/k3s:dev-20210427.2, was able to test with that one.

@developer-guy I was able to test your PR and it works correctly. It would be great if the flag was also available at k3d cluster create using the same schema as actual --label "foo=bar@server[0]"

I see the label flags as this:

k3d cluster create --docker-label "foo=abc@server[0]" --node-label "bar=xyz@server[0]"
k3d node create --docker-label "foo=abc" --node-label "bar=xyz"

That should be good for all cases and consistently named. Will test your PR once you push new changes.

@developer-guy
Copy link
Contributor Author

so, I need to separate the label flag for the container and node like the following:
--docker-label "foo=abc" --node-label "bar=xyz"

And I should also add this support for both cluster and node commands, am I right?

@ejose19
Copy link
Contributor

ejose19 commented Apr 30, 2021

That's how I see it, @iwilltry42 what do you think?

@developer-guy
Copy link
Contributor Author

developer-guy commented May 1, 2021

@ejose19 are you aware of the followings flags that are being already available for the command k3d create cluster:

--k3s-agent-arg k3s agent    Additional args passed to the k3s agent command on agent nodes (new flag per arg)
--k3s-server-arg k3s server  Additional args passed to the k3s server command on server nodes (new flag per arg)

We can use these flags to specify node labels while we are provisioning the cluster, but what I'm trying to do with this PR is that there is support for this in the k3d node command, so, this PR resolves that.

@ejose19
Copy link
Contributor

ejose19 commented May 1, 2021

@developer-guy Yes I'm aware, but the issue with these flags is that they apply to all servers or agents, which defeat the point of using labels in first place (I can already differentiate servers from agents using either role some of the labels | annotations generated by k3s).

The logic is already implemented by #378 so I'm sure a lot of it can be taken from there, but if you only want to submit for k3d node np, I will try to get a follow-up PR to get k3d cluster equivalents (just make sure to rename flag to --node-label instead of --labels)

@developer-guy
Copy link
Contributor Author

developer-guy commented May 1, 2021

The logic is already implemented by #378 so I'm sure a lot of it can be taken from there, but if you only want to submit for k3d node np, I will try to get a follow-up PR to get k3d cluster equivalents (just make sure to rename flag to --node-label instead of --labels)

@ejose19 Yeah sure, thanks for the PR that you mention, I might take some of the codes from there, yes, I only want to add this support for k3d node command, I'll fix the flag name 👍

@Dentrax
Copy link

Dentrax commented May 2, 2021

I just updated the flag name from labels to node-label in here. @ejose19 FYI. :)

@developer-guy
Copy link
Contributor Author

@ejose19, is there anything you want us to do?

@ejose19
Copy link
Contributor

ejose19 commented May 3, 2021

Hello @developer-guy, I'm not a maintainer so only @iwilltry42 is allowed to merge your PR, I will do a follow-up PR after yours is merged to implement the equivalent for k3d cluster

@developer-guy
Copy link
Contributor Author

@iwilltry42 wdyt? Anything that you want us to do?

@iwilltry42
Copy link
Member

Hey 👋
Thanks for your input folks 👍
I just got back to this, sorry for the delay.
We might want to think about extending the flag with --k3s-node-label (and in that case possibly think of a short-hand for it) to separate k3s-functionality-specifc flags from the others 🤔

@developer-guy
Copy link
Contributor Author

Hey 👋
Thanks for your input folks 👍
I just got back to this, sorry for the delay.
We might want to think about extending the flag with --k3s-node-label (and in that case possibly think of a short-hand for it) to separate k3s-functionality-specifc flags from the others 🤔

No worries 🙋🏻‍♂️, of course, @iwilltry42, we can do that, @Dentrax would you like to change the flag?

@georgelza
Copy link

so was directed here by a search... I want to add labels to my k3d nodes, so that deployments can be coded with a selector (similar to what I need to do in AWS EKS, spread across AZ's)
Can anyone advise if this is possible with a multi node K3D cluster and how ?

@iwilltry42
Copy link
Member

Hi @georgelza , like so: k3d cluster create --k3s-node-label FOO=BAR@agent:0 to add the label to the agent with index 0.
More info on nodefilters (the stuff starting with @: https://k3d.io/v5.4.1/design/concepts/#nodefilters)

@georgelza
Copy link

... I'd prefer to know how we can add this as a label to the config file, adding a label to each set of nodes.

G

@iwilltry42
Copy link
Member

... I'd prefer to know how we can add this as a label to the config file, adding a label to each set of nodes.

G

You may want to checkout the docs: https://k3d.io/v5.4.1/usage/configfile/ the config key is options.k3s.nodeLabels

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
5 participants