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

Use current k9s NS if new context has no default NS #2197

Merged
merged 2 commits into from
Nov 12, 2023
+26 −2
Merged

Use current k9s NS if new context has no default NS #2197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
090b959
Fix resetting active namespace when switching ctx
mikutas Aug 14, 2023
cca576e
Respect existing behavior
mikutas Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions internal/client/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,19 @@ func (c *Config) CurrentContextName() (string, error) {
return cfg.CurrentContext, nil
}

func (c *Config) CurrentContextNamespace() (string, error) {
name, err := c.CurrentContextName()
if err != nil {
return "", err
}
context, err := c.GetContext(name)
if err != nil {
return "", err
}

return context.Namespace, nil
}

// GetContext fetch a given context or error if it does not exists.
func (c *Config) GetContext(n string) (*clientcmdapi.Context, error) {
cfg, err := c.RawConfig()
Expand Down Expand Up @@ -283,6 +296,13 @@ func (c *Config) CurrentUserName() (string, error) {
func (c *Config) CurrentNamespaceName() (string, error) {
ns, _, err := c.clientConfig().Namespace()

if ns == "default" {
ns, err = c.CurrentContextNamespace()
if ns == "" && err == nil {
return "", errors.New("No namespace specified in context")
}
}

return ns, err
}

Expand Down
6 changes: 4 additions & 2 deletions internal/client/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestConfigCurrentNamespace(t *testing.T) {
}{
"default": {
flags: &genericclioptions.ConfigFlags{KubeConfig: &kubeConfig},
namespace: "default",
namespace: "",
},
"withContext": {
flags: &genericclioptions.ConfigFlags{KubeConfig: &kubeConfig, Context: &bleeCTX},
Expand All @@ -128,7 +128,9 @@ func TestConfigCurrentNamespace(t *testing.T) {
t.Run(k, func(t *testing.T) {
cfg := client.NewConfig(u.flags)
ns, err := cfg.CurrentNamespaceName()
assert.Nil(t, err)
if ns != "" {
assert.Nil(t, err)
}
assert.Equal(t, u.namespace, ns)
})
}
Expand Down
1 change: 1 addition & 0 deletions internal/dao/testdata/config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ contexts:
name: fred
- context:
cluster: blee
namespace: zorg
user: blee
name: blee
- context:
Expand Down
1 change: 1 addition & 0 deletions internal/view/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ func (a *App) switchContext(name string) error {
ns, err := a.Conn().Config().CurrentNamespaceName()
Copy link
Owner

Choose a reason for hiding this comment

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

I think the current behavior is correct i.e when switching context we want to use the default namespace associated with that context vs using the namespace in the k9s config. For example if ctx1 specifies default namespace ns1 but the k9s config (if it exists!) specifies ns2 then k9s should use ns1 as the default namespace. This proposal would always override the namespace with the k9s config namespace or default if not set.
That said, a case could be made if the new context does not specify a namespace where we could override with the last visited namespace from the k9s config if set....

Does this make sense?

Copy link
Contributor Author

@mikutas mikutas Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you, I think I got the intent of existing behavior. 💡
I missed the usecase that we specify ns expressly in kubeconfig.
On the other hand, I'm not in the habit of specifying ns in kubeconfig, so the fallback suggested will be very convenient for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, long time no see @derailed .
Is there something I can do to go ahead?

if err != nil {
log.Warn().Msg("No namespace specified in context. Using K9s config")
ns = a.Config.ActiveNamespace()
}
a.initFactory(ns)

Expand Down