📖 Etcd on data disk CAEP

[CecileRobertMichon]

What this PR does / why we need it:
This PR brings the CAEP for implementing disk setup and mounts in CABPK.

Comments on the Google doc are addressed.

Moving here for getting feedback from a broader audience.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Relates to #2994

[CecileRobertMichon]

/hold for rebase and community consensus

/assign @detiber
/assign @vincepri
/assign @fabriziopandini
/assign @bagnaram

[k8s-ci-robot]

@CecileRobertMichon: GitHub didn't allow me to assign the following users: bagnaram.

Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/hold for rebase and community consensus

/assign @detiber
/assign @vincepri
/assign @fabriziopandini
/assign @bagnaram

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/test-infra repository.

[bagnaram]

/assign

[CecileRobertMichon]

/kind proposal

[vincepri]

1 week timeout starting today?

[michaelgugino]

I don't prefer this proposal. As mentioned on the gdoc, I think continuing building an abstraction on an abstraction is the wrong direction. Cloud init already supports doing this, we should enable users to BYO cloud init vs consuming an ever-incomplete template system.

This goes back to the issue of the problem being a higher-level problem. One can't have an additional disk in the bootstrap config without having the corresponding machine infrastructure in place. So, there still needs to be yet-another layer on top to coordinate these pieces. Is there going to be some higher-level tuneable such as 'give-etcd-separate-disks' that is going to stamp out all the options for all the pieces down below? Doesn't seem feasible. At some point, we have to give users the power to make their own decisions.

[vincepri]

@michaelgugino You put in words something that myself (and probably others) have been feeling for a while. The bootstrap abstraction needs a complete rework, although without time and people-hours dedicated to it we won't be able to deliver such a rework (which will probably also require one or more API revisions to fully become concrete).

Yesterday, @CecileRobertMichon and I were talking about this proposal, and how the bootstrap provider is becoming more and more infrastructure aware and crossing boundaries.

While there are better, longer term solutions, I'm fully behind having these changes go in v1alpha3 because it's an iterative improvement (better than what we have today), and it opens new cases and features. In addition, we have the team at Microsoft behind this proposal, which is also going to help with implementation, support, and testing.

[CecileRobertMichon]

This goes back to the issue of the problem being a higher-level problem. One can't have an additional disk in the bootstrap config without having the corresponding machine infrastructure in place. So, there still needs to be yet-another layer on top to coordinate these pieces. Is there going to be some higher-level tuneable such as 'give-etcd-separate-disks' that is going to stamp out all the options for all the pieces down below? Doesn't seem feasible. At some point, we have to give users the power to make their own decisions.

@michaelgugino that is indeed a problem I've been thinking about a lot with this proposal, and I also put it in the proposal as the last "risk and mitigation". Right now, the solution I'm proposing is actually give the users the choice ( I personally see it as "putting the burden on the user" though) by exposing a generic disk setup and making the changes required to put etcd on an external disk part of the user template (ie. completely user configurable). There will not be "a higher-level tuneable such as 'give-etcd-separate-disks'". See my capz prototype for an example of this works in practice.

I agree that "KubeadmConfigSpec" is taking on too much infrastructure configuration right now but as Vince said, reworking CABPK needs to be its own proposal and would likely involve breaking changes. I'm trying to unblock CAPZ and CAPA users by providing a generic, reusable implementation that brings incremental value and can be done in v1alpha3.

[fabriziopandini]

+1 for me because this enables new use cases right now in v0.3.x.
I also agree that long term, we should find a way for moving infrastructure specific bits away from KubeadmConfig

[michaelgugino]

I don't have strong objection to this particular feature if we've identified that long term this provider is probably not the ideal implementation.

Perhaps, then, this CAEP should be more generic. Something like "Continue adding cloud-init functionality to CAPBK" so people can add features as they need them instead of having to file a new CAEP every time we want to extend the missing pieces. Perhaps some framework for expected implementation conformance.

[CecileRobertMichon]

@michaelgugino let's continue the conversation about making cloud init more accessible to the user and infrastructure bootstrap less tied to kubeadm bootstrapper in #3064

[vincepri]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, vincepri

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 [vincepri]

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

[detiber]

/lgtm

[vincepri]

/retest

[vincepri]

/milestone v0.3.x

[vincepri]

@CecileRobertMichon might want to try to rebase?

[CecileRobertMichon]

@vincepri I just did earlier, I'm pretty sure the failures are due to https://status.quay.io/

[vincepri]

uh oh! You're right, apologies

[CecileRobertMichon]

/retest

[vincepri]

/hold cancel