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

📖 Update glossary to distinguish bootstrap from provisioning #9484

Closed
wants to merge 2 commits into from

Conversation

johananl
Copy link
Member

What this PR does / why we need it:

Two concerns are commonly conflated in CAPI - running a bootstrapper (e.g. kubeadm) and performing OS customizations using cloud-init or Ignition. The two are closely-related yet distinct. Issues such as #5294 stem at least in part from this conflation.

This PR updates our glossary to clearly distinguish between the two concerns by introducing the term provisioner as well as clarifying other related terms.

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

None

/area documentation

Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
@k8s-ci-robot k8s-ci-robot added area/documentation Issues or PRs related to documentation cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 22, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign oscr for approval. For more information see the Kubernetes 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:

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

Copy link
Contributor

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't disagree with the definitions here, but Bootstrapper contradicts Bootstrap. I think that contradiction is intentional - as this PR changes the definitions used in CAPI rather than clarifies them.

I don't think updating the Glossary is the right way to approach making these changes. I think work should continue on #5294 which will hopefully result in a proposal which can be reviewed and decided on by the community.

@johananl
Copy link
Member Author

johananl commented Sep 22, 2023

Thanks for weighing in @killianmuldoon!

this PR changes the definitions used in CAPI rather than clarifies them.

Yes, my intention is certainly to propose a change to our terminology by introducing a new term - "provisioning" - and distinguishing it from "bootstrap". I think it's necessary because in the current state the term "bootstrap" could mean two different things, and I think this fact has caused confusion already and will keep causing confusion if we work on issues such as #5294 without addressing this ambiguity.

I don't think updating the Glossary is the right way to approach making these changes. I think work should continue on #5294 which will hopefully result in a proposal which can be reviewed and decided on by the community.

The two aren't mutually exclusive 🙂 The need to disambiguate "bootstrap" stems directly from #5294 - see @randomvariable's comment in the issue's description. As I started looking into the issue I quickly realized that people mean different things when they say "bootstrap", both in discussions and in the codebase. Example:

// CloudConfig make the bootstrap data to be of cloud-config format.
CloudConfig Format = "cloud-config"

Here "bootstrap" refers to cloud-init code. But we also have e.g. "kubeadm bootstrap provider", in which case "bootstrap" refers to what kubeadm does. As long as bootstrap could mean either "running kubeadm" or "running cloud-init", I think we'll keep encountering problems like the one #5294 aims to solve. Do you think otherwise?

That said, I read the current definition of Bootstrap and I think you're right regarding the contradiction. The following phrasing from the definition of "bootstrap" does indeed sound a lot like what I mean by Provisioning:

assembling data to provide when creating the server that backs the Machine, as well as runtime configuration of the software running on that server

That makes me think we need to update the Bootstrap term, too, to mean "executing kubeadm (or an alternative)", speaking strictly about the action of joining a server as a k8s cluster node. I'm not too attached to any specific terminology, all I want is for us to be able to distinguish between kubeadm and cloud-init/Ignition when discussing code changes. It's actually pretty hard to write a proposal for #5294 without updating the terminology.

Of course I'm counting on community involvement, including in this PR 🙂 If you think we shouldn't update the terminology as suggested above, could you propose how we solve the ambiguity in "bootstrap", assuming you agree that's a problem we want to address?

@killianmuldoon
Copy link
Contributor

I understand the concerns with the current definitions, but I don't think this is the right approach to changing them.

My idea of how this flow should work is that the proposal should include whatever changes to terminology are needed. Proposals will often include a Glossary section. Once the proposal is accepted we should update the repo glossary to reflect the new shared understanding of the terms.

IMO that's the best way to prevent over-and-back changes to these definitions. It also prevents drift between how these terms are defined and how they are currently expressed in the CAPI contract and the behaviour of CAPI components.

@fabriziopandini
Copy link
Member

I agree with @killianmuldoon, if we are willing to invest in this area, let's address all the efforts in a proposal for #5294 that should include updated/new glossary terms as well as the full context to fully understand them

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2023
@johananl
Copy link
Member Author

OK folks, makes sense to me. I'll include the glossary changes in the proposal for #5294 and close this for now. Thanks!

@johananl johananl closed this Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Issues or PRs related to documentation cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants