fix: install biz when restart base #24
Conversation
WalkthroughThe changes in this pull request involve renaming several variables and method receiver names in the Changes
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 (5)
cmd/module-controller/main.go (2)
Line range hint
107-116: Consider making QPS and Burst rates configurableThe QPS (100) and Burst (200) rates are hardcoded in the code. Consider making these configurable through environment variables for better flexibility in different environments.
// Initialize controller manager kubeConfig := config.GetConfigOrDie() -// TODO: should support to set from parameter -kubeConfig.QPS = 100 -kubeConfig.Burst = 200 +kubeConfig.QPS = float32(utils.GetEnvAsInt("KUBE_CLIENT_QPS", 100)) +kubeConfig.Burst = utils.GetEnvAsInt("KUBE_CLIENT_BURST", 200)
132-143: Enhance error handling with more specific error messagesThe error messages could be more descriptive to help with debugging issues during controller setup.
moduleDeploymentController, err := module_deployment_controller.NewModuleDeploymentController(env) if err != nil { - zlogger.Error(err, "unable to set up module_deployment_controller") + zlogger.Error(err, "failed to create module deployment controller", + "env", env) return } err = moduleDeploymentController.SetupWithManager(ctx, k8sControllerManager) if err != nil { - zlogger.Error(err, "unable to setup module_deployment_controller") + zlogger.Error(err, "failed to setup module deployment controller with manager", + "env", env) return }controller/module_deployment_controller/module_deployment_controller.go (3)
Line range hint
274-314: Address the TODO comment in updateDeploymentReplicasThe TODO comment on line 277 indicates missing implementation details. This is a critical function for replica synchronization and should be fully implemented.
Would you like me to help implement the missing functionality or create a GitHub issue to track this task?
Consider adding retry mechanism for replica updates
The replica update operation is critical but doesn't have a retry mechanism in case of temporary failures.
if int32(sameClusterNodeCount) != *deployment.Spec.Replicas { + backoff := wait.Backoff{ + Steps: 5, + Duration: 100 * time.Millisecond, + Factor: 2.0, + } + err := retry.OnError(backoff, errors2.IsConflict, func() error { err := tracker.G().FuncTrack(deployment.Labels[vkModel.LabelKeyOfTraceID], vkModel.TrackSceneVPodDeploy, model.TrackEventVPodPeerDeploymentReplicaModify, deployment.Labels, func() (error, vkModel.ErrorCode) { return moduleDeploymentController.updateDeploymentReplicasOfKubernetes( ctx, sameClusterNodeCount, deployment) }) + return err + }) if err != nil { logger.Error(err, fmt.Sprintf("failed to update deployment replicas of %s", deployment.Name)) } }
Line range hint
351-380: Optimize node readiness check performanceThe node readiness check could be optimized by breaking the inner loop once we find the node is not ready.
for _, node := range nodeList.Items { if node.Status.Phase == corev1.NodeRunning { + nodeReady := false for _, cond := range node.Status.Conditions { if cond.Type == corev1.NodeReady { if cond.Status == corev1.ConditionTrue { readyNodeCnt++ - break + nodeReady = true } + break } } + if !nodeReady { + continue + } } }
Line range hint
381-390: Enhance error logging with deployment detailsAdd more context to error messages to aid in debugging deployment update issues.
err := moduleDeploymentController.client.Patch(ctx, &deployment, patch) if err != nil && !errors2.IsNotFound(err) { + logger := zaplogger.FromContext(ctx) + logger.Error(err, "Failed to update deployment replicas", + "deployment", deployment.Name, + "namespace", deployment.Namespace, + "current_replicas", *old.Spec.Replicas, + "desired_replicas", replicas) return err, model.CodeKubernetesOperationFailed }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
cmd/module-controller/main.go(3 hunks)controller/module_deployment_controller/module_deployment_controller.go(14 hunks)
🔇 Additional comments (1)
cmd/module-controller/main.go (1)
Line range hint 123-131: LGTM! Clear and well-structured configuration
The VNode controller configuration is properly structured with clear parameter names and appropriate variable naming.
2 participants
fix koupleless/koupleless#363
Summary by CodeRabbit
mgrtok8sControllerManager,rcctovNodeControllerConfig, andmdctomoduleDeploymentController.ModuleDeploymentControllerstruct.These changes enhance code readability while maintaining existing functionality.