add skip_etcd_even_nodes_check var to allow even number of etcd nodes

[pedro-peter]

What type of PR is this?
/kind feature

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind flake

What this PR does / why we need it:
Currently the only way to add an even number of control-plane/etcd nodes to a cluster is to disable all assert checks by passing ignore_assert_errors=yes

It would be better just to skip the etcd even nodes assert check and leave all other asserts as they can catch useful problems.

Which issue(s) this PR fixes:

Fixes #12013

Special notes for your reviewer:
I've updated the doc to use the new skip_etcd_even_nodes_check flag. This isn't really a breaking change as ignore_assert_errors=yes will still work.

Does this PR introduce a user-facing change?:

skip_etcd_even_nodes_check flag can now be used to allow creating an uneven number of control-plane/etcd nodes

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pedro-peter
Once this PR has been reviewed and has the lgtm label, please assign cristicalin for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[k8s-ci-robot]

Hi @pedro-peter. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ant31]

In which situation would you want even number of etcd?

[pedro-peter]

In which situation would you want even number of etcd?

when replacing a control-plane/etcd node, you might want to add the new node before removing the old node:
https://github.com/kubernetes-sigs/kubespray/blob/master/docs/operations/nodes.md#adding-an-etcd-node

there's a short duration where you'll have an even number of etcd nodes.

[VannTen]

when replacing a control-plane/etcd node, you might want to add the new node before removing the old node

Actually, I think you need to remove the old before adding the new.
If I'm not mistaken, etcd quorum is number of host / 2 + 1 so if you add 1 nodes and have now 4, quorum is now 3. You still have to turn off at least one machine, so you didn't gain anything from adding a node, but you added a point of failure.

[ant31]

yes, it's remove then add

Etcd documentation on live reconfigurations:
https://etcd.io/docs/v3.5/op-guide/runtime-configuration/

To replace the machine, follow the instructions for removing the member from the cluster, and then add a new member in its place. If the cluster holds more than 50MB, it is recommended to migrate the failed member’s data directory if it is still accessible

and

All changes to the cluster must be done sequentially:
To update a single member peerURLs, issue an update operation
To replace a healthy single member, remove the old member then add a new member
To increase from 3 to 5 members, issue two add operations
To decrease from 5 to 3, issue two remove operations

Also I don't think kubespray etcd role is tested to do such operations.

Overall I don't think we should add variable to allow an even number of etcd. It's never a situation you want to be in and could mislead users new to kubernetes.

[pedro-peter]

I still think it's useful to allow creating an uneven number of etcd nodes (as long as it's disabled by default)

currently you can only do this by disabling all assert checks...

cluster operators can make the decision if they want to risk running a cluster for a duration with an uneven number of etcd nodes.

[morguldir]

When you remove a node you also end up with an even amount of nodes, and with 2 nodes the quorum is 2, so until you deploy the new node, you have zero failure tolerance

But I think the safer way to avoid this is to add two nodes, e.g. etcd4 and etcd5, then remove etcd3 and etcd5 at the same time. Using etcd learners would also help

[pedro-peter]

I think people are missing the point of this PR...

You can already add an uneven number of control-plane/etcd nodes in a kubespray cluster. You currently need to disable all assert checks (https://github.com/kubernetes-sigs/kubespray/blob/master/roles/kubernetes/preinstall/tasks/0040-verify-settings.yml) using var ignore_assert_errors=yes to do this (this is in the documentation).

If folks want to add an even number of control-plane/etcd nodes, it's better just to allow disabling the single asert check (https://github.com/kubernetes-sigs/kubespray/pull/12014/files#diff-f817513f21cf14346b8672affc950686b1c4e3b9521671e0bbb76aac63fa5f6bR67) and allow the remaining checks to run.

[morguldir]

I don't think anyone missed that, adding an even amount of etcd nodes is a large footgun, so i understand not wanting to add any form of support for it

this is in the documentation

It probably shouldn't be, but it's at least obvious that it's a dangerous operation

[pedro-peter]

But I think the safer way to avoid this is to add two nodes, e.g. etcd4 and etcd5, then remove etcd3 and etcd5 at the same time. Using etcd learners would also help

this is definitely a safer operation to replace a control-plane/etcd node should probably be added to the documentation:
https://github.com/kubernetes-sigs/kubespray/blob/master/docs/operations/nodes.md#1-add-the-new-node-running-clusteryml

there are cases though where a cluster operator might decide to take the risk of running an even number of etcd nodes (for a short duration). folks may not have the hardware available to spin up new control-plane nodes.