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.

Sign up for GitHub

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

Jump to bottom

Remove config flag as a global Flag + setting config default location to /etc/k0s/k0s.yaml #1389

Merged
merged 3 commits into from
Dec 28, 2021
Merged

Remove config flag as a global Flag + setting config default location to /etc/k0s/k0s.yaml #1389

merged 3 commits into from
Dec 28, 2021

Conversation

trawler
Copy link
Contributor

@trawler trawler commented Dec 23, 2021 •
edited
Loading

Fixes #1369 #1222

This PR is a sub-section of the work entailed in the original config-refactoring PR (#1247), since a rebase was too complex.

It includes:

  • removal of the config flag from unnecessary commands
  • setting the default config location to /etc/k0s/k0s.yaml
  • update of the docs/cli documentation
  • unifying the configFromStdin and ConfigFromFile for clusterConfig to ConfigFromReader

@trawler trawler requested a review from a team as a code owner December 23, 2021 12:10
@trawler trawler requested review from s0j and mviitane December 23, 2021 12:10
@trawler trawler changed the title Config refactoring Remove config flag as a global Flag Dec 23, 2021
@trawler trawler force-pushed the config_refactoring branch 3 times, most recently from 3566049 to b50bf8c Compare December 23, 2021 12:36
kke
kke previously requested changes Dec 23, 2021
View reviewed changes
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.

This includes all the kubectl docs in the cli/docs

With #1376 all the docs/cli stuff could be dropped.

kke
kke reviewed Dec 23, 2021
View reviewed changes
cmd/token/token.go Outdated Show resolved Hide resolved
@trawler trawler force-pushed the config_refactoring branch 2 times, most recently from 0cd3419 to 6ad3d88 Compare December 23, 2021 13:41
@trawler
Copy link
Contributor Author

trawler commented Dec 23, 2021 •
edited
Loading

Doesn't seem to include #1358

I listed the wrong issue. description fixed.

@trawler trawler force-pushed the config_refactoring branch 2 times, most recently from 6a180c2 to 269ab5f Compare December 23, 2021 14:35
@trawler trawler requested a review from kke December 23, 2021 14:36
@trawler trawler changed the title Remove config flag as a global Flag Remove config flag as a global Flag + setting config default location to /etc/k0s/k0s.yaml Dec 23, 2021
@trawler trawler force-pushed the config_refactoring branch 2 times, most recently from 0c3da92 to 5d5e0ae Compare December 24, 2021 11:16
@trawler trawler requested a review from ncopa December 24, 2021 11:34
@trawler trawler force-pushed the config_refactoring branch 2 times, most recently from 53817bc to 0b4b16a Compare December 24, 2021 12:04
@ncopa
Copy link
Collaborator

ncopa commented Dec 27, 2021

Would love to have a comment in the commit message explaining why we remove --config as global flag and why we use /etc/k0s/k0s.yaml as default config location, so future devs don't need to guess.

ncopa
ncopa requested changes Dec 27, 2021
View reviewed changes
Copy link
Collaborator

@ncopa ncopa left a comment

Choose a reason for hiding this comment

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

We need to make sure the config file is closed after it is read (unless we read from stdin)

pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
Karen Almog added 3 commits December 27, 2021 23:46
remove --config as a global flag
63c545f 
this commit removes config as a global flag so that it will not
show up in commands that don't use it (for example, worker).
it also sets the default config path to /etc/k0s/k0s.yaml since it is
already the de-facto location for k0sctl and also appears in our
documenttation.
Signed-off-by: Karen Almog <[email protected]>
config refactoring: make tests green again
901cddf 
Signed-off-by: Karen Almog <[email protected]>
config refactoring: defer close config file
5e2aa11 
Signed-off-by: Karen Almog <[email protected]>
@trawler trawler requested a review from ncopa December 27, 2021 23:48
@trawler trawler merged commit 6a651c0 into k0sproject:main Dec 28, 2021
@trawler trawler deleted the config_refactoring branch December 28, 2021 13:17
@trawler trawler linked an issue Dec 29, 2021 that may be closed by this pull request
k0s kubeconfig ignores config value for the external address #1202
Closed
@trawler trawler mentioned this pull request Dec 29, 2021
Closed
@trawler trawler linked an issue Dec 29, 2021 that may be closed by this pull request
Do not accept -c or --config option for the worker subcommand #1222
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncopa ncopa ncopa approved these changes

@s0j s0j Awaiting requested review from s0j s0j is a code owner automatically assigned from k0sproject/k0s-maintainers

@mviitane mviitane Awaiting requested review from mviitane mviitane is a code owner automatically assigned from k0sproject/k0s-maintainers

@jnummelin jnummelin Awaiting requested review from jnummelin

@mikhail-sakhnov mikhail-sakhnov Awaiting requested review from mikhail-sakhnov

@kke kke Awaiting requested review from kke

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@trawler @ncopa @kke