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: Use data from config file when available #55

Merged
merged 3 commits into from
Feb 5, 2024

Conversation

blanquicet
Copy link
Member

@blanquicet blanquicet commented Feb 2, 2024

This PR improves the management of node information in two situations:

Situation 1

Issue: When we set the default node using the config use-node command and we still want to run one command in a differente node (using --node or --id) without the need of changing the default node, the command will fail because we will have VMSS info + Node or VMSS info + ID and it will not know what to select.

Solution: Don't read the current config vmss info if node or id was specified. It is not needed anyway.

# Initial state:
$ ./kubectl-aks config import
# aks-nodepool1-17267820-vmss000002 is the node I need to use more frequently:
$ ./kubectl-aks config use-node aks-nodepool1-17267820-vmss000002

# However, if I need to run one single command in node aks-nodepool1-17267820-vmss000000

# Before this PR
$ ./kubectl-aks check-apiserver-connectivity --node aks-nodepool1-17267820-vmss000000
Error: specify either 'node' or VMMS instance information ('subscription', 'node-resource-group', 'vmss' and 'instance-id')

# After this PR
$ ./kubectl-aks check-apiserver-connectivity --node aks-nodepool1-17267820-vmss000000
Connectivity check: succeeded

Situation 2

kubectl-aks should use the node information from config file when available:

# Initial situation:
$ ./kubectl-aks config import
$ ./kubectl-aks config use-node aks-nodepool1-17267820-vmss000000

# Node not available in the confi file will use the API server:
$ ./kubectl-aks check-apiserver-connectivity --node jose
\Error: getting vm: retrieving Azure resource ID of node jose from API server: nodes "jose" not found

# Node available in the config file will not use the API server:
$ ./kubectl-aks check-apiserver-connectivity --node aks-nodepool1-17267820-vmss000002
|DEBUG[0000] Using VMSS information from config for node aks-nodepool1-17267820-vmss000002
Connectivity check: succeeded

# Without specifying any node, it will use aks-nodepool1-17267820-vmss000000 (current-node) from config:
$ ./kubectl-aks check-apiserver-connectivity
Connectivity check: succeeded

Issue:
If users import the vmss info and then try to use --node or --id, then
the command will fail because we will have vmss info + node or vmss info
+ resource-id and it will not know what to select.

Solution:
Don't read the current config vmss info if node or id was specified. It
is not needed anyway.

Initial state:

$ ./kubectl-aks config import

Let's say aks-nodepool1-17267820-vmss000002 is the node I need to use more frequently:

$ ./kubectl-aks config use-node aks-nodepool1-17267820-vmss000002

However, if I need to run one single command in node aks-nodepool1-17267820-vmss000000

Before this PR:

$ ./kubectl-aks check-apiserver-connectivity --node
aks-nodepool1-17267820-vmss000000
Error: specify either 'node' or VMMS instance information ('subscription', 'node-resource-group', 'vmss' and 'instance-id')

After this PR:

$ ./kubectl-aks check-apiserver-connectivity --node
aks-nodepool1-17267820-vmss000000
Connectivity check: succeeded

Signed-off-by: Jose Blanquicet <[email protected]>
`kubectl-aks` should use the node information from config file when
available.

Initial situation:

$ ./kubectl-aks config import
$ ./kubectl-aks config use-node aks-nodepool1-17267820-vmss000000

Node not available in the confi file will use the API server:

$ ./kubectl-aks check-apiserver-connectivity --node jose
\Error: getting vm: retrieving Azure resource ID of node jose from API server: nodes "jose" not found

Node available in the config file will not use the API server:

$ ./kubectl-aks check-apiserver-connectivity --node aks-nodepool1-17267820-vmss000002
|DEBUG[0000] Using VMSS information from config for node aks-nodepool1-17267820-vmss000002
Connectivity check: succeeded

Without specifying any node, it will use aks-nodepool1-17267820-vmss000000 (current-node) from config:

$ ./kubectl-aks check-apiserver-connectivity
Connectivity check: succeeded

Signed-off-by: Jose Blanquicet <[email protected]>
@blanquicet
Copy link
Member Author

These changes improve user experience in use cases like the one described in this AKS TSG: https://github.com/MicrosoftDocs/SupportArticles-docs-pr/pull/5675

Copy link
Member

@mqasimsarfraz mqasimsarfraz left a comment

Choose a reason for hiding this comment

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

LGTM, I tested it and it works fine!

I agree being able to use flags like --node with current config node is better!

cmd/utils/vmss.go Show resolved Hide resolved
@blanquicet blanquicet merged commit 76393e0 into main Feb 5, 2024
15 checks passed
@blanquicet blanquicet deleted the jose/use-available-data branch February 5, 2024 17:41
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

Successfully merging this pull request may close these issues.

2 participants