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

Fix check for initialized nodes #599

Merged
merged 4 commits into from
Aug 13, 2024
Merged

Fix check for initialized nodes #599

merged 4 commits into from
Aug 13, 2024

Conversation

neoaggelos
Copy link
Contributor

@neoaggelos neoaggelos commented Aug 10, 2024

Summary

Followup for #564

Changes

  • Fix issue where cmdutil.GetNodeStatus() did not properly recognize 503 errors
  • Make this part of (k8sd.Client).NodeStatus()
  • Adjust CLI to use client.NodeStatus()
  • Removed cmdutil.GetNodeStatus() (it is not a command utility, it is functionality we want in the client)

Addresses

$ k8s status
root@t2:~# k8s status
Error: Failed to retrieve the node status.

The error was: failed to GET /k8sd/node: Database is not yet initialized

With the new changes, we are back to the previous error message we had, that shows how to bootstrap a new cluster:

root@t1:~# k8s status
Error: The node is not part of a Kubernetes cluster. You can bootstrap a new cluster with:

  sudo k8s bootstrap

@neoaggelos neoaggelos requested a review from a team as a code owner August 10, 2024 10:30
@eaudetcobello
Copy link
Contributor

I notice the isBootstrapped variables have been renamed to initialize. Can you speak a bit on the difference between the two ideas?

@neoaggelos
Copy link
Contributor Author

I notice the isBootstrapped variables have been renamed to initialize. Can you speak a bit on the difference between the two ideas?

The first node will bootstrap a new cluster. Other nodes join the cluster. isInitialized is used to mean that the node is part of the cluster.

Copy link
Contributor

@HomayoonAlimohammadi HomayoonAlimohammadi left a comment

Choose a reason for hiding this comment

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

Thanks a lot Angelos! LGTM

@neoaggelos neoaggelos merged commit 6b92ed7 into main Aug 13, 2024
19 checks passed
@neoaggelos neoaggelos deleted the fix/node-status branch August 13, 2024 07:52
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.

3 participants