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

Allow node-role labeling #580

Closed
paulbsch opened this issue Feb 25, 2019 · 3 comments

Comments

@paulbsch
Copy link
Contributor

commented Feb 25, 2019

Why do you want this feature?
In our use case, we utilize node-role.kuberneties.io/{role-name} labels. It is also useful for identifying nodes in the cluster when using tools like kubectl. Example:

$ kubectl get nodes
NAME                                           STATUS    ROLES             AGE       VERSION
ip-192-168-66-184.us-west-2.compute.internal   Ready     jarvice-compute   37m       v1.11.5
ip-192-168-71-24.us-west-2.compute.internal    Ready     jarvice-compute   37m       v1.11.5
ip-192-168-75-4.us-west-2.compute.internal     Ready     jarvice-system    37m       v1.11.5

Currently, eksctl doesn't allow any label matching the kubernetes.io domain. This makes perfect sense with respect to labels that should be immutable ("kubernetes.io/arch", kubernetes.io/hostname", etc.) However, node role labels don't seem to fall into that category.

What feature/behavior/change do you want?
Please alllow node role labeling. i.e. node-role.kubernetes.io/{role-name} e.g. node-role.kubernetes.io/compute

@errordeveloper

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

Do we currently return unknown 'kubernetes.io' or 'k8s.io' labels were specified kind of error?

It should be fairly easy to add node-role.kubernetes.io, would you like to try adding it yourself?

I believe it just needs to be added to the list of domain prefixes here:

for _, domain := range []string{"kubelet.kubernetes.io", "node.kubernetes.io"} {
if ns == domain || strings.HasSuffix(ns, "."+domain) {
allowedKubeletNamespace = true
}
}

@paulbsch

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2019

It does return that error, yes.

Adding that domain prefix solves the issue. I will submit a pull request.

@errordeveloper

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

Closed via #582.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.