kubernetes-sigs / kubespray

Deploy a Production Ready Kubernetes Cluster
Apache License 2.0
15.86k stars 6.41k forks source link

kubespray should stop using the wording "master" #11353

Open neolit123 opened 2 months ago

neolit123 commented 2 months ago

What would you like to be added

a few years back the Kubernetes projected created a working group called "WG naming" backed by CNCF members which established that words like "master" are offensive in Kubernetes. the whole documentation at k8s.io. was revamped, a number areas of kubernetes/kubernetes were changed, kubeadm and other SIG Cluster Lifecycle subprojects changed too. there was a kubeadm master -> control plane taint migration if you recall.

as much as i like kubepray, it seems that often it ends up on a bit of an island, where it seems isolated from what's going on in the top level SIG and the K8s ecosystem. i was recently pinged on this issue https://github.com/kubernetes-sigs/kubespray/issues/11350 where i saw kubespray logs such as "Upgrade first master...".

https://github.com/search?q=repo%3Akubernetes-sigs%2Fkubespray+master&type=code

kubespray should stop using the wording "master" and you can execute on this action in a few steps:

please let me know if you have any questions.

/priority important-longterm

Why is this needed

compliance with the rest of the Kubernetes project

neolit123 commented 2 months ago

@cristicalin @floryut @liupeng0518 @mzaian @oomichi @yankay @ant31

taken from https://github.com/kubernetes-sigs/kubespray/blob/master/OWNERS_ALIASES

PTAL

tico88612 commented 2 months ago

/help /good-first-issue

k8s-ci-robot commented 2 months ago

@tico88612: This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to [this](https://github.com/kubernetes-sigs/kubespray/issues/11353): >/help >/good-first-issue Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
floryut commented 2 months ago

Indeed at least this occurrence was forgotten https://github.com/kubernetes-sigs/kubespray/blob/1ebd860c13d95e7f19dd12f1fd9fa316cb0f9740/roles/kubernetes/control-plane/tasks/kubeadm-upgrade.yml#L12

tico88612 commented 2 months ago

Some documents are still written master, and we must check them thoroughly.

https://kubespray.io/#/docs/CNI/kube-router?id=verifying-kube-router-install

edgarzzc commented 2 months ago

how should we help with this? Shall we check the codes contain the master and fix it?

yankay commented 2 months ago

Thanks @neolit123

The kubespray needs to fix it . :-)

nicolas-goudry commented 2 months ago

I’d like to work on this if nobody started it already.

However, I have a few questions after having reviewed the current state of the repository regarding usages of the master term:

  1. at several places, the term is used as an ansible tag:
  2. should compatibility with old style group names be kept? (ie. kube-master group was renamed to kube_control_plane)
  3. what about the kube_master_* ansible variables?
  4. does the « master » term renaming should also be applied to etcd (eg. is_etcd_master)? I guess not, but I’m still wondering.
  5. should the « worker » term also be taken into account? If so, should it be renamed to just « node »? Or something else?

For (1), should we keep the tag as-is? Add a new control-plane tag and keep the master tag around? For how long though? I think that straightly replacing the master tag with control-plane would be a breaking change for people relying on it.

For (2), most of the comments mentioning the backward compatibility bit are from around 3 years ago, so I guess it would be safe to completely remove them (and effectively breaking backward compatibility, but since it’s been years, it shouldn’t bother anyone).

For (3), I think this is the most dangerous part about this tree-wide renaming since it touches directly to how Kubespray is configured. My thoughts are to add new control_plane_* variables which take the same default value as their kube_master_* siblings and keep supporting both for a bit. Compatibility could be achieved by a specific task, but ATM it seems this task may not be trivial to implement when thinking about the multiple possible scenarios and rules to define and respect in order to choose which one should be used (eg. kube_master_memory_reserved and control_plane_memory_reserved are both set and different, which one should be used and why?).

IMO this issue should be addressed by several PRs:

bogd commented 2 months ago

Since I seem to have triggered this (with the kubespray issue ), I would like to help. As @nicolas-goudry pointed out, this is not a trivial task, since it touches many kubespray internals - at a quick search, there are 1486 references to "master" in 331 files across the project.

And after performing the search, I actually came here to say something similar - while some of the search results are not relevant (references to master branches in git repos) and some fixes are simple (like the task name mentioned above), others are not (the variable names, handler names, etc. have a high risk of breaking stuff).

As for the documentation, I believe that needs to be done at the same time as the changes in the code. It will already be confusing if we have two sets of variables doing the same thing (kube_master_* vs kube_control_plane_*) - if the documentation is not also up to date, it will be painful....

[ Edited to add a quick note regarding item 4: the usage of etcd_master seems... inconsistent in the code. Unless I'm misunderstanding something:

- name: Gen_certs | Gather etcd member/admin and kube_control_plane client certs from first etcd node
  register: etcd_master_certs
...
  delegate_to: "{{ groups['etcd'][0] }}"

]

bogd commented 1 month ago

I started on the "low hanging fruit" (descriptions, comments, task names, etc) here . I am not touching any "sensitive" stuff (like variable names) yet.

A couple more things I noticed while going through the files:

speedythesnail commented 1 month ago

I think if you are offended at a common word used in tech, your time can be better spent elsewhere.

bogd commented 1 month ago

I think if you are offended at a common word used in tech, your time can be better spent elsewhere.

Since the last reply here was mine, I will assume that you are talking to me. :)

I am in no way offended by it. In fact, I do believe it will not go away for a long time (not even in this particular project) - for various reasons, some of which I have already explained above. Also, if you are looking for my personal opinion, I do believe that context matters a lot, and many current attempts to move away from a particular word are often.... (how should I say it diplomatically?) misguided.

However, this is not about my personal opinions. The decision has been taken a long time ago, K8s as a whole is moving (or has already moved) away from this terminology, and it does make sense that the other SIGs should follow (if for no other reason, at least to keep the terminology consistent across the project).

As for me, I got into this because I took this as an opportunity to better understand the kubespray internals, and wanted to contribute to a project I have been using for a while. If the team believes that my time "can be better spent elsewhere", they can tell me to stop and (hopefully) point me in another direction :)

speedythesnail commented 1 month ago

@bogd I appreciate your further explanation :). Given your point in your third paragraph, I would concur. I am frustrated by the original changes in terminology in different projects because it caused non-uniform terminology just so the leadership could feel good about themselves.

Context does matter a lot as you stated, and all too often I see people disguising their discrimination against other people by masking it as "caring for them", especially in tech.

There is a lot of controversy in several FOSS projects as of late by activists and I would hate to see another project thrown into turmoil :(.

VannTen commented 1 month ago

I think if you are offended at a common word used in tech, your time can be better spent elsewhere.

This has been discussed at length, in the kubernetes project as well as elsewhere.

Please refrain from discussing the why any further, this is about the how.

ant31 commented 1 month ago

@bogd Thank you for taking the time to looking at it. can you please make a "WIP PR" from your fork so it's easier to follow progress and comment along the way.

bogd commented 1 month ago

@ant31 - Thank you for the suggestion! I was looking for a way to get input from the team while working on this, and this seems like the best way to proceed.

Opened WIP PR #11394 . Looking forward to guidance on the more... "troublesome" steps of the process (see previous comments for some examples).

bogd commented 1 month ago

/assign

fortisitpl commented 1 week ago

There is no reason to change branch name, word master in context of repository is not offensive repository is not a person so calling it master is not wrong, This change will bring a lot of problems not only to the code but also to users, please test first upgrading working production cluster after this change.

yankay commented 1 week ago

There is no reason to change branch name, word master in context of repository is not offensive repository is not a person so calling it master is not wrong, This change will bring a lot of problems not only to the code but also to users, please test first upgrading working production cluster after this change.

Agree with you