Replace the inventory group kube-master with kube_control_plane #7256

oomichi · 2021-02-06T01:53:27Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This replaces kube-master with kube_control_plane because of 1:

  The Kubernetes project is moving away from wording that is
  considered offensive. A new working group WG Naming was created
  to track this work, and the word "master" was declared as offensive.
  A proposal was formalized for replacing the word "master" with
  "control plane". This means it should be removed from source code,
  documentation, and user-facing configuration from Kubernetes and
  its sub-projects.

NOTE: The reason why this changes it to kube_control_plane not kube-control-plane is for valid group names on ansible.

Which issue(s) this PR fixes:

Fixes # #7157

Does this PR introduce a user-facing change?:

The inventory group *kube-master* has been renamed to *kube_control_plane*.
Please update your inventory file by replacing *kube-master* if continuing to use the existing inventory file.

oomichi · 2021-02-06T01:58:40Z

WORK_NOTE: Basically this work is done with the script:

#!/bin/bash

FILES=$(grep kube-master * -R | awk -F: '{print $1}' | sort | uniq)
for file in $FILES
do
        sed -i s/"kube-master"/"kube-control-plane"/g $file
done

But we need to take care of the following things by hands:

Update a table in ./docs/ha-mode.md
Update contrib/inventory_builder/inventory.py for E501 error(line too long)
Update docs/recover-control-plane.md by adding \ in "kube_control_plane"
Revert change on contrib/terraform/openstack/README.md because that is on cloud-provider-openstack
Add the following lines into yml files under the root directory.

- name: Add kube-master nodes to kube_control_plane
  # This is for old inventory which contains kube-master instead of kube_control_plane
  hosts: kube-master
  gather_facts: false
  tasks:
    - name: add nodes to kube_control_plane group
      group_by:
        key: 'kube_control_plane'

champtar · 2021-02-06T02:11:06Z

Can we take this opportunity to replace - with _ in group names ?

oomichi · 2021-02-06T02:18:25Z

Can we take this opportunity to replace - with _ in group names ?

TBH I cannot find concrete naming rule document, but _ seems fine according to the sample from https://docs.ansible.com/ansible/2.7/user_guide/intro_inventory.html

a_group:
    testvar: a
    ansible_group_priority: 10
b_group:
    testvar: b

But this PR is already XL size.
Please don't try making this PR bigger and bigger ;-)

champtar · 2021-02-06T02:46:50Z

https://github.com/kubernetes-sigs/kubespray/blob/master/ansible.cfg#L7
At least use kube_control_plane and we can finish the cleanup in another PR, so we break user inventory only once

contrib/terraform/openstack/README.md

docs/ansible.md

docs/recover-control-plane.md

oomichi · 2021-02-24T22:38:03Z

packet_debian9-calico-upgrade job was failed as https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/1053891745

TASK [kubernetes/preinstall : Stop if either kube-master or kube-node group is empty] ***
task path: /builds/kargo-ci/kubernetes-sigs-kubespray/roles/kubernetes/preinstall/tasks/0020-verify-settings.yml:2
Wednesday 24 February 2021  21:39:42 +0000 (0:00:00.776)       0:02:53.961 **** 
failed: [instance-2] (item=kube-master) => {
    "ansible_loop_var": "item",
    "assertion": "groups.get('kube-master')",
    "changed": false,
    "evaluated_to": false,
    "item": "kube-master",
    "msg": "Assertion failed"
}
ok: [instance-2] => (item=kube-node) => {
    "ansible_loop_var": "item",
    "changed": false,
    "item": "kube-node",
    "msg": "All assertions passed"
}

That seems inventory-builder needs to output kube-master to pass this job.
After releasing a version, we can make inventory-builder ouput kube_control_plane instead.

champtar · 2021-03-11T15:01:01Z

cluster.yml

+  tasks:
+    - name: add nodes to kube_control_plane group
+      group_by:
+        key: 'kube_control_plane'


I'm wondering if in some cases we loose the k8s-cluster groups_vars.
Does a play with hosts: kube_control_plane get groups_vars from k8s-cluster ?

On the above case(kube-master exists instead of kube_control_plane), kube_control_plane has the same instances as kube-master and these instances are in k8s-cluster.
Can this be answer for your question?

I'm just not sure how defaults work anymore, server mix the defaults from all groups they are in, or when you ask for hosts: kube_control_plane, you will get the server vars, the vars for kube_control_plane and not the vars from k8s-cluster because kube_control_plane is not in k8s-cluster

OK, I am testing this PR locally to see the above question.

/hold

confirmed this PR works with old inventory which contain kube-master instead of kube_control_plane.

/hold cancel

Thanks for checking !

oomichi · 2021-03-22T16:47:14Z

Need to rebase
/hold

oomichi · 2021-03-23T19:10:14Z

/hold cancel

oomichi · 2021-03-23T19:22:36Z

/cc @EppO

champtar

Some file that need modification I think

contrib/terraform/upcloud/templates/inventory.tpl
.gitlab-ci.yml
.gitlab-ci/packet.yml

And you need to rebase on latest master and fix

roles/kubernetes/control-plane/templates/k8s-certs-renew.timer.j2

(I just did git grep kube-master)

champtar · 2021-03-23T19:45:52Z

cluster.yml

@@ -2,6 +2,15 @@
 - name: Check ansible version
  import_playbook: ansible_version.yml

+- name: Add kube-master nodes to kube_control_plane


I see that this task/play is copied pasted in multiple file, could you factor that in a file and use import_playbook ?
This will help to finish moving away from invalid group name

I want to keep here as it and will do cleanup with another PR because this already takes much time.

champtar · 2021-03-23T19:48:36Z

tests/cloud_playbooks/roles/packet-ci/templates/inventory.j2

@@ -4,6 +4,9 @@ instance-{{ loop.index }} ansible_ssh_host={{instance.stdout}}
 {% endfor %}

 {% if mode is defined and mode in ["separate", "separate-scale"] %}
+[kube_control_plane]
+instance-1
+
 [kube-master]
 instance-1


why do you keep [kube-master] here ? (multiple occurrences in this file)

Yes, we need to keep this for passing upgrading test which is from stable version.

Maybe put a #TODO: remove after 2.1X

oomichi · 2021-03-23T21:22:17Z

Some file that need modification I think
contrib/terraform/upcloud/templates/inventory.tpl
.gitlab-ci.yml
.gitlab-ci/packet.yml
And you need to rebase on latest master and fix
roles/kubernetes/control-plane/templates/k8s-certs-renew.timer.j2
(I just did git grep kube-master)

Thanks for your review, that is a nice point.
I will change here soon.

champtar · 2021-03-23T21:36:29Z

contrib/terraform/upcloud/templates/inventory.tpl

@@ -3,7 +3,7 @@
 ${connection_strings_master}
 ${connection_strings_worker}

-[kube-master]
+[kube_control_plane]


you forgot to change line 16

This replaces kube-master with kube_control_plane because of [1]: The Kubernetes project is moving away from wording that is considered offensive. A new working group WG Naming was created to track this work, and the word "master" was declared as offensive. A proposal was formalized for replacing the word "master" with "control plane". This means it should be removed from source code, documentation, and user-facing configuration from Kubernetes and its sub-projects. NOTE: The reason why this changes it to kube_control_plane not kube-control-plane is for valid group names on ansible. [1]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-cluster-lifecycle/kubeadm/2067-rename-master-label-taint/README.md#motivation

k8s-ci-robot · 2021-03-23T21:54:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: champtar, floryut, oomichi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [floryut]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

champtar · 2021-03-23T21:55:17Z

/lgtm

This replaces kube-master with kube_control_plane because of [1]: The Kubernetes project is moving away from wording that is considered offensive. A new working group WG Naming was created to track this work, and the word "master" was declared as offensive. A proposal was formalized for replacing the word "master" with "control plane". This means it should be removed from source code, documentation, and user-facing configuration from Kubernetes and its sub-projects. NOTE: The reason why this changes it to kube_control_plane not kube-control-plane is for valid group names on ansible. [1]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-cluster-lifecycle/kubeadm/2067-rename-master-label-taint/README.md#motivation

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 6, 2021

k8s-ci-robot requested review from alijahnas and bozzo February 6, 2021 01:53

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 6, 2021

oomichi force-pushed the replace-master-3 branch from 08975ea to d58153a Compare February 6, 2021 02:02

oomichi marked this pull request as draft February 6, 2021 02:32

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2021

oomichi force-pushed the replace-master-3 branch from d58153a to 1fdc51f Compare February 23, 2021 19:11

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2021

oomichi changed the title ~~WIP: Replace the inventory group kube-master with kube-control-plane~~ WIP: Replace the inventory group kube-master with kube_control_plane Feb 23, 2021

oomichi marked this pull request as ready for review February 23, 2021 20:41

oomichi changed the title ~~WIP: Replace the inventory group kube-master with kube_control_plane~~ Replace the inventory group kube-master with kube_control_plane Feb 23, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 23, 2021

oomichi changed the title ~~Replace the inventory group kube-master with kube_control_plane~~ WIP: Replace the inventory group kube-master with kube_control_plane Feb 23, 2021

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 23, 2021

oomichi force-pushed the replace-master-3 branch from 1fdc51f to a355a45 Compare February 24, 2021 19:57

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 24, 2021

oomichi commented Feb 24, 2021

View reviewed changes

contrib/terraform/openstack/README.md Outdated Show resolved Hide resolved

docs/ansible.md Outdated Show resolved Hide resolved

docs/ansible.md Outdated Show resolved Hide resolved

docs/recover-control-plane.md Outdated Show resolved Hide resolved

oomichi force-pushed the replace-master-3 branch from a355a45 to 399e8e9 Compare February 24, 2021 20:17

oomichi changed the title ~~WIP: Replace the inventory group kube-master with kube_control_plane~~ Replace the inventory group kube-master with kube_control_plane Feb 24, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 24, 2021

oomichi force-pushed the replace-master-3 branch from 399e8e9 to 376693e Compare February 24, 2021 22:39

champtar mentioned this pull request Mar 11, 2021

Auto renew control plane certificates #7358

Merged

champtar reviewed Mar 11, 2021

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 11, 2021

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 22, 2021

oomichi force-pushed the replace-master-3 branch from 2bd1429 to a19bc68 Compare March 22, 2021 18:01

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 22, 2021

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2021

k8s-ci-robot requested a review from EppO March 23, 2021 19:22

champtar suggested changes Mar 23, 2021

View reviewed changes

oomichi force-pushed the replace-master-3 branch from a19bc68 to dfee7bd Compare March 23, 2021 21:22

champtar reviewed Mar 23, 2021

View reviewed changes

oomichi force-pushed the replace-master-3 branch from dfee7bd to b152b17 Compare March 23, 2021 21:48

champtar approved these changes Mar 23, 2021

View reviewed changes

k8s-ci-robot assigned champtar Mar 23, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 23, 2021

k8s-ci-robot merged commit 486b223 into kubernetes-sigs:master Mar 24, 2021

oomichi mentioned this pull request Mar 24, 2021

Replace master with "control plane" #7157

Closed

floryut mentioned this pull request Apr 5, 2021

Fix CI template for etcd recover jobs (kube-master rename) #7441

Merged

floryut mentioned this pull request May 11, 2021

Release Proposal v2.16 #7602

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace the inventory group kube-master with kube_control_plane #7256

Replace the inventory group kube-master with kube_control_plane #7256

oomichi commented Feb 6, 2021 •

edited

Loading

oomichi commented Feb 6, 2021 •

edited

Loading

champtar commented Feb 6, 2021

oomichi commented Feb 6, 2021

champtar commented Feb 6, 2021

oomichi commented Feb 24, 2021

champtar Mar 11, 2021

oomichi Mar 11, 2021

champtar Mar 11, 2021

oomichi Mar 11, 2021

oomichi Mar 11, 2021

champtar Mar 11, 2021

oomichi commented Mar 22, 2021

oomichi commented Mar 23, 2021

oomichi commented Mar 23, 2021

champtar left a comment •

edited

Loading

champtar Mar 23, 2021

oomichi Mar 23, 2021

champtar Mar 23, 2021

champtar Mar 23, 2021

oomichi Mar 23, 2021

champtar Mar 23, 2021

oomichi commented Mar 23, 2021

champtar Mar 23, 2021

k8s-ci-robot commented Mar 23, 2021

champtar commented Mar 23, 2021

Replace the inventory group kube-master with kube_control_plane #7256

Replace the inventory group kube-master with kube_control_plane #7256

Conversation

oomichi commented Feb 6, 2021 • edited Loading

oomichi commented Feb 6, 2021 • edited Loading

champtar commented Feb 6, 2021

oomichi commented Feb 6, 2021

champtar commented Feb 6, 2021

oomichi commented Feb 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oomichi commented Mar 22, 2021

oomichi commented Mar 23, 2021

oomichi commented Mar 23, 2021

champtar left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oomichi commented Mar 23, 2021

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 23, 2021

champtar commented Mar 23, 2021

oomichi commented Feb 6, 2021 •

edited

Loading

oomichi commented Feb 6, 2021 •

edited

Loading

champtar left a comment •

edited

Loading