Add MachineHealthChecks for Kubernetes VMs #398
Conversation
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
WalkthroughThis pull request includes updates to the Kubernetes application configuration files. The Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/apps/kubernetes/templates/cluster.yaml (1)
213-232: LGTM with suggestions for improvementThe addition of the MachineHealthCheck resource is a good practice for maintaining the health of your Kubernetes cluster. The structure and integration with existing resources look correct. However, I have a few suggestions to enhance its flexibility and clarity:
- Consider making the timeouts configurable. Currently,
nodeStartupTimeoutand the unhealthy condition timeouts are hardcoded. You could make these values customizable through Helm values:nodeStartupTimeout: {{ $group.healthCheck.nodeStartupTimeout | default "10m" }}
Verify if creating a MachineHealthCheck for each nodeGroup is intentional. If not, consider moving it outside the nodeGroups loop and adjusting the selector accordingly.
Add comments to explain the purpose of the MachineHealthCheck and how it works. This will help future maintainers understand the resource's role in the cluster's health management.
Would you like me to provide an example of how to implement these suggestions?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/apps/kubernetes/Chart.yaml (1 hunks)
- packages/apps/kubernetes/templates/cluster.yaml (1 hunks)
- packages/apps/versions_map (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/apps/kubernetes/Chart.yaml
🧰 Additional context used
🔇 Additional comments (4)
packages/apps/versions_map (2)
Line range hint
38-85: Consider the implications of updating multiple packages to HEAD.Several packages (mysql, nats, rabbitmq, redis, tcp-balancer, tenant, virtual-machine, and vpn) have been updated to HEAD. While this brings in the latest features and fixes, it also introduces potential instability.
To ensure system stability, please run comprehensive tests. Consider the following script to check for recent commits in these repositories:
#!/bin/bash # Description: Check for recent commits in updated repositories repos=("mysql" "nats" "rabbitmq" "redis" "tcp-balancer" "tenant" "virtual-machine" "vpn") for repo in "${repos[@]}"; do echo "Checking $repo repository:" gh repo clone "$repo/$repo" "/tmp/$repo" 2>/dev/null cd "/tmp/$repo" git fetch --all echo "Latest commit:" git log -1 --pretty=format:"%h - %s (%cr)" origin/main echo -e "\n" doneThis script will help verify that the HEAD versions are stable and suitable for your use case.
Consider implementing a strategy for managing package versions:
- Use specific version tags for production environments to ensure stability.
- Regularly update and test HEAD versions in a staging environment before promoting to production.
- Document the process for updating and testing package versions to maintain consistency across the team.
36-37: LGTM! Verify the commit hash for version 0.11.1.The addition of new kubernetes versions (0.11.1 and 0.12.0) is consistent with the PR objective. The use of a specific commit hash for 0.11.1 and marking 0.12.0 as HEAD follows good versioning practices.
Please run the following script to verify the commit hash for version 0.11.1:
packages/apps/kubernetes/templates/cluster.yaml (2)
Line range hint
1-232: Overall assessment: LGTM with minor suggestionsThe addition of the MachineHealthCheck resource to this Helm chart is a positive enhancement for maintaining the health of the Kubernetes cluster. The new resource is well-integrated with the existing MachineDeployment and aligns with the Cluster API specifications.
Key points:
- The MachineHealthCheck resource is correctly structured and placed within the nodeGroups loop.
- It uses appropriate selectors to match the corresponding MachineDeployment.
- The unhealthy conditions are defined to check for Ready status.
Suggestions for improvement:
- Make timeout values configurable through Helm values for greater flexibility.
- Add comments to explain the purpose and functionality of the MachineHealthCheck resource.
- Verify the intentionality of creating a MachineHealthCheck for each nodeGroup.
Overall, this change enhances the robustness of the cluster by automating health checks on nodes. With the suggested improvements, it will be more flexible and easier to maintain in the future.
213-232: Verify linkage between MachineHealthCheck and MachineDeploymentThe MachineHealthCheck resource seems to be correctly aligned with the MachineDeployment resource, as they share the same cluster name and deployment name in their selectors. However, to ensure proper functionality, please verify that:
- The labels in the MachineDeployment's template match the selector in the MachineHealthCheck.
- The MachineHealthCheck is created in the same namespace as the MachineDeployment.
To confirm this, you can run the following script:
This script will help ensure that the MachineHealthCheck is properly linked to the MachineDeployment.
| unhealthyConditions: | ||
| - type: Ready | ||
| status: Unknown | ||
| timeout: 30s |
There was a problem hiding this comment.
it would be good to be able to configure these timeouts via values.yaml
There was a problem hiding this comment.
Thank you for your review, I think for now it makes no sense since we're running on controlled environment.
There was a problem hiding this comment.
The impact of this setting being too low is really severe, my md went into a tailspin and only recovered when I reset this value much higher
Add `MachineHealthCheck` resource to continiusly checking Machine state. If Machine is not ready it will be recreated in 60 seconds after unavailabilty. (30 sec kubelet to stop posing the status + 30 sec MachineHealthCheck timeout) Fixes #365 Signed-off-by: Andrei Kvapil <kvapss@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a `MachineHealthCheck` resource to monitor the health of machine deployments in Kubernetes. - **Version Updates** - Updated the Kubernetes chart version from `0.11.1` to `0.12.0`. - Various packages' versions have been updated to reflect the latest revisions, ensuring accuracy in versioning. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Add `MachineHealthCheck` resource to continiusly checking Machine state. If Machine is not ready it will be recreated in 60 seconds after unavailabilty. (30 sec kubelet to stop posing the status + 30 sec MachineHealthCheck timeout) Fixes cozystack#365 Signed-off-by: Andrei Kvapil <kvapss@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a `MachineHealthCheck` resource to monitor the health of machine deployments in Kubernetes. - **Version Updates** - Updated the Kubernetes chart version from `0.11.1` to `0.12.0`. - Various packages' versions have been updated to reflect the latest revisions, ensuring accuracy in versioning. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
3 participants
Add
MachineHealthCheckresource to continiusly checking Machine state.If Machine is not ready it will be recreated in 60 seconds after unavailabilty. (30 sec kubelet to stop posing the status + 30 sec MachineHealthCheck timeout)
Fixes #365
Signed-off-by: Andrei Kvapil kvapss@gmail.com
Summary by CodeRabbit
New Features
MachineHealthCheckresource to monitor the health of machine deployments in Kubernetes.Version Updates
0.11.1to0.12.0.