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

config refactoring: use runtime config #1409

Merged
merged 4 commits into from
Jan 26, 2022

Conversation

trawler
Copy link
Contributor

@trawler trawler commented Jan 3, 2022

Signed-off-by: Karen Almog [email protected]

Issue
Fixes #1202 #1261
What this PR Includes
This PR refactors the entire handling of k0s config. Among other changes it:

  1. Removes --config as a global flag to all commands
  2. Keeps --config flag only on the k0s controller and k0s reset commands (note on the reset command below).
  3. Adds a default value to k0s config: /etc/k0s/k0s.yaml
  4. Generates a runtime config whenever k0s controller goes up and keeps it under /run/k0s/k0s.yaml. This runtime config file is removed when the controller is stopped.
  5. The runtime config is a merged config of custom values and default config values (show merged config #1261)

Note on k0s reset:
Since the runtime config only exists while the controller is up, k0s reset (that can only be performed while the controller is down) needs to still retail the --config flag. A workaround to that would be to use the global location /etc/k0s/k0s.yaml as a source of configuration.

To Do:

cmd/controller/controller.go Outdated Show resolved Hide resolved
@trawler trawler force-pushed the cfg_bootstrap_init branch 4 times, most recently from 1eac183 to b2b0b51 Compare January 13, 2022 11:27
@trawler trawler force-pushed the cfg_bootstrap_init branch 3 times, most recently from a706a30 to 211db00 Compare January 13, 2022 12:22
Copy link
Member

@jnummelin jnummelin left a comment

Choose a reason for hiding this comment

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

Left some questions and general comments. The direction is good in general.

I'm slightly worried that dropping the -config option in many commands will break backwards compatibility e.g. with k0sctl. Maybe rebase with the #1450 to verify this.

cmd/api/api.go Show resolved Hide resolved
cmd/reset/reset.go Outdated Show resolved Hide resolved
cmd/restore/restore.go Outdated Show resolved Hide resolved
pkg/config/cli.go Outdated Show resolved Hide resolved
@kke
Copy link
Contributor

kke commented Jan 19, 2022

Left some questions and general comments. The direction is good in general.

I'm slightly worried that dropping the -config option in many commands will break backwards compatibility e.g. with k0sctl. Maybe rebase with the #1450 to verify this.

You could leave a nonfunctional hidden --config flag, maybe it could warn but ignore.

trawler pushed a commit to trawler/k0sctl that referenced this pull request Jan 20, 2022
This commit removes the config flag to comply with the changes in
k0sproject/k0s#1409

Signed-off-by: Karen Almog <[email protected]>
trawler pushed a commit to trawler/k0sctl that referenced this pull request Jan 20, 2022
This commit removes the config flag to comply with the changes in
k0sproject/k0s#1409

Signed-off-by: Karen Almog <[email protected]>
@trawler
Copy link
Contributor Author

trawler commented Jan 20, 2022

Left some questions and general comments. The direction is good in general.

I'm slightly worried that dropping the -config option in many commands will break backwards compatibility e.g. with k0sctl. Maybe rebase with the #1450 to verify this.

k0sctl requires a very minor change for token creation. after that, make check-k0sctl passes: https://github.com/k0sproject/k0sctl/compare/main...trawler:k0s_config_flag_1409?expand=1

trawler pushed a commit to trawler/k0sctl that referenced this pull request Jan 20, 2022
This commit removes the config flag to comply with the changes in
k0sproject/k0s#1409

Signed-off-by: Karen Almog <[email protected]>
@trawler
Copy link
Contributor Author

trawler commented Jan 21, 2022

Waiting for next k0sctl that includes k0sproject/k0sctl#303 to run the make check-k0sctl test again

kke added a commit to k0sproject/k0sctl that referenced this pull request Jan 21, 2022
* Remove --config flag from token command

This commit removes the config flag to comply with the changes in
k0sproject/k0s#1409

Signed-off-by: Karen Almog <[email protected]>

* Light touch-up

* Typo

Co-authored-by: Kimmo Lehto <[email protected]>
@trawler trawler force-pushed the cfg_bootstrap_init branch 2 times, most recently from aaa5028 to 9be3ace Compare January 21, 2022 12:17
Karen Almog added 4 commits January 21, 2022 12:50
This commit:
  - Removes the default value in cobra config flag
  - Adds a logic to set the default file under the config package

Signed-off-by: Karen Almog <[email protected]>
Copy link
Contributor

@kke kke left a comment

Choose a reason for hiding this comment

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

It looks ok to me but it's fairly big and touches everything so I think the best would be to get it in as soon as possible before a release that includes it so that there is time to find any problems.

@trawler trawler merged commit 32752bc into k0sproject:main Jan 26, 2022
@trawler trawler deleted the cfg_bootstrap_init branch February 8, 2022 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants