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

docs: add control plane conversion guide and 0.9 upgrade notes #3278

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

smira
Copy link
Member

@smira smira commented Mar 9, 2021

These docs are critical to get 0.9.0-beta released.

Signed-off-by: Andrey Smirnov smirnov.andrey@gmail.com

@smira smira added this to the v0.9 milestone Mar 9, 2021
@smira
Copy link
Member Author

smira commented Mar 9, 2021

/approve

Copy link
Contributor

@Ulexus Ulexus left a comment

Choose a reason for hiding this comment

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

Mostly nits, but some important user-facing verbiage.

talosctl -n <master node IP> get StaticPods.kubernetes.talos.dev
talosctl -n <master node IP> get Manifests.kubernetes.talos.dev

bootstrap manifests will only be applied for missing resources, existing resources will not be updated
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this mean? Missing from where? What bootstrap manifests? Resources existing where? Is this something I (the user) need to worry about?

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure if we can omit that from the output completely, but the idea is pretty simple. once conversion is complete, Talos nodes will try to create any missing Kubernetes resources found in the bootstrap manifests (which are in turn generated from Talos machine configuration).

So if someone modified some resource, it's fine as Talos won't overwrite them, but if someone deleted some manifest, Talos will attempt to re-create it.

Copy link
Member Author

Choose a reason for hiding this comment

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

it is explained more around lines 108-112, but probably script itself deserves better message

talosctl -n <master node IP> get Manifests.kubernetes.talos.dev

bootstrap manifests will only be applied for missing resources, existing resources will not be updated
confirm disabling pod-checkpointer to proceed with control plane update [yes/no]:
Copy link
Contributor

Choose a reason for hiding this comment

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

The user isn't likely to know anything about the pod checkpointer, so they cannot make this choice.

Instead, perhaps:

In order to upgrade components, we need to disable the Pod Checkpointer because the Pod Checkpointer's job is to replace components that are supposed to run if they stop running.
Once you disable the Pod Checkpointers, you should avoid rebooting your control plane nodes until the entire conversion is complete.
Do you wish to continue now?

Copy link
Member Author

Choose a reason for hiding this comment

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

it looks too verbose for me, but let me take this to another PR as this is script output and it should be fixed in the code first, and I'll update the docs in subsequent PR once I fix the code itself

Static pod definitions are generated from machine configuration and should match pod template as generated by Talos on bootstrap of self-hosted control plane unless there were some manual changes applied to the daemonset specs after bootstrap.
Talos patches machine configuration with container image versions scraped from the daemonset definition, fetches service account key from Kubernetes secrets.
Aggregator CA can't be recovered from the self-hosted control plane, so new CA gets generated.

Copy link
Contributor

Choose a reason for hiding this comment

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

An additional note should be added here to tell the user to say NO to this for the first run, so that they can verify the manifests, as indicated below.

Copy link
Member Author

Choose a reason for hiding this comment

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

technically they don't have to say no, as they can use another window to perform lookups.

Copy link
Contributor

Choose a reason for hiding this comment

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

My key point here is that we want to prevent people from blindly saying "yes" before they have inspected things. The pipelining is convenient, but if it requires branching to perform checks, we should make that apparent before they blindly say "yes".

website/content/docs/v0.9/Guides/upgrading-talos.md Outdated Show resolved Hide resolved
website/content/docs/v0.9/Guides/upgrading-talos.md Outdated Show resolved Hide resolved
website/content/docs/v0.9/Guides/upgrading-talos.md Outdated Show resolved Hide resolved
website/content/docs/v0.9/Guides/upgrading-talos.md Outdated Show resolved Hide resolved
website/content/docs/v0.9/Guides/upgrading-talos.md Outdated Show resolved Hide resolved
These docs are critical to get 0.9.0-beta released.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
@smira
Copy link
Member Author

smira commented Mar 10, 2021

/lgtm

@talos-bot talos-bot merged commit ae8bedb into siderolabs:master Mar 10, 2021
smira added a commit to smira/talos that referenced this pull request Mar 11, 2021
This includes Sean's comments from siderolabs#3278 and introduces a new flag which
is referenced in manual conversion process document.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
smira added a commit to smira/talos that referenced this pull request Mar 11, 2021
This includes Sean's comments from siderolabs#3278 and introduces a new flag which
is referenced in manual conversion process document.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
talos-bot pushed a commit that referenced this pull request Mar 12, 2021
This includes Sean's comments from #3278 and introduces a new flag which
is referenced in manual conversion process document.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
smira added a commit to smira/talos that referenced this pull request Mar 12, 2021
This includes Sean's comments from siderolabs#3278 and introduces a new flag which
is referenced in manual conversion process document.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
(cherry picked from commit 6f7df3d)
smira added a commit that referenced this pull request Mar 15, 2021
This includes Sean's comments from #3278 and introduces a new flag which
is referenced in manual conversion process document.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
(cherry picked from commit 6f7df3d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants