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

Prompt if reboot is required when apply affect may need one. #6394

Open
Jackbennett opened this issue Oct 11, 2022 · 6 comments
Open

Prompt if reboot is required when apply affect may need one. #6394

Jackbennett opened this issue Oct 11, 2022 · 6 comments

Comments

@Jackbennett
Copy link

Feature Request

Updated machine registry config requires a reboot, but the cli does not warn you;

> talosctl apply -f controlplane.yaml
Applied configuration without a reboot

Kind of implies to me there's a check if a reboot might be required on apply.

Description

Within the manual;

.machine.registries (CRI containerd plugin will not pick up the registry authentication settings without a reboot)
https://www.talos.dev/v1.2/talos-guides/configuration/editing-machine-configuration/

So upon applying that change the CLI could know it won't take effect until reboot. I think not automatically rolling out reboots is the correct behaviour.

@smira
Copy link
Member

smira commented Oct 11, 2022

It's complicated, as images are pulled by Talos and CRI.

Talos applies any changes on the fly.

CRI applies some changes on the fly, and some not. CRI should be fixed eventually to support reloading everything on the fly, but it's not something we have control over.

Copy link

github-actions bot commented Jul 4, 2024

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jul 4, 2024
@Jackbennett
Copy link
Author

It's not stale but unresolved.
Making the cri dynamic works sure be cool but I think this issue is more about the manual knows a key needs a reboot so the cli could let me know ones needed. Or if not at apply time some reconcile earning, like you get if I use mismatched talosctl versions

@github-actions github-actions bot removed the Stale label Jul 5, 2024
@rothgar
Copy link
Member

rothgar commented Oct 2, 2024

Is this request asking if we can have the CLI print something like configuration applied: reboot needed [will not happen automatically] when the configuration change requires a reboot?

@Jackbennett
Copy link
Author

Yeh that's accurate, if there's a modified config that it's known requires a reboot to take effect.

Possibly it might be better worded like "Configuration applied, changes affected on reboot: [.machine.registries, etc...]" with a view to seeing that too with talosctl node status

If this is a whole thing then maybe close the issue. But seeing as the docs know this, maybe the cli types can react to it quite simply?

I've not really had this next example with Talos but before I've seen config changes made weeks/months ago that broke everything, but that only applied on reboot, so once a reboot finally happened, the thing to fix wasn't recent at all.

I think this is important as your config yaml evolves from patches to a git tracked file instead of regenerated from scratch each the time, new defaults can get missed. Maybe I'm always supposed to start patching machine config from newly generated configs.

@smira
Copy link
Member

smira commented Oct 15, 2024

The "defaults" don't change over time, as long as you properly generated "base layer" of the machine configuration:

talosctl gen config --talos-version=v1.8 --with-secrets=secrets.yaml

The --talos-version=v1.8 is the compatibility guarantee, it should be the version of Talos the cluster was created with, and it should never change.

This way even if you update talosctl, the config will stay same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants