feat: add support for lws in RAGengine

[MartinForReal]

Reason for Change:

Add support for lws. Requirements

• [ ] added unit tests and e2e tests (if applicable).

Issue Fixed:

Fixes #873

Notes for Reviewers:

[cvvz]

Should watch for lws at here

[kaito-pr-agent[bot]]

Title

(Describe updated until commit https://github.com/kaito-project/kaito/commit/4c2a4eaa25353ee0b28265a2c02fbd241ef18bbd)

Add Support for LeaderWorkerSet in RAGengine

---

Description

• Added support for LeaderWorkerSet (LWS) in RAGengine

• Updated feature gates to include LWS

• Modified controllers to handle LWS based on feature flag

• Added new manifest generation for LWS

---

Changes walkthrough 📝

	Relevant files
Enhancement	8 files main.go Imported LWS API                                                                                  +2/-0      main.go Imported LWS API                                                                                  +2/-0      featuregates.go Added LWS feature gate                                                                      +1/-0      preset-rag.go Conditionally generate LWS manifest                                            +9/-3      ragengine_controller.go Modified to handle LWS based on feature flag                          +54/-32  leader_worker_set.go Added LWS manifest generation                                                        +104/-0  consts.go Added LWS feature flag constant                                                    +1/-0      resources.go Added support for LWS resource handling                                    +18/-2
Tests	1 files leader_worker_set_test.go Added tests for LWS manifest generation                                    +36/-0
Configuration changes	1 files Makefile Added LWS installation target                                                        +11/-0
Dependencies	2 files go.mod Updated dependencies for LWS                                                          +66/-61  go.sum Updated checksums for LWS dependencies                                      +82/-71

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[kaito-pr-agent[bot]]

PR Reviewer Guide 🔍

(Review updated until commit https://github.com/kaito-project/kaito/commit/4c2a4eaa25353ee0b28265a2c02fbd241ef18bbd)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The err variable is redeclared inside the if featuregates.FeatureGates[consts.FeatureFlagLWS] block, which shadows the outer err variable. This could lead to incorrect error handling. var err error if featuregates.FeatureGates[consts.FeatureFlagLWS] { Logging Error The err variable is logged before being returned, but it is not checked for nil. This could result in logging a nil error. klog.ErrorS(err, "resource", resource) return err Undefined Variable The controller variable is used in the OwnerReferences slice but is not defined anywhere in the provided code. Controller: &controller,

[kaito-pr-agent[bot]]

PR Code Suggestions ✨

Latest suggestions up to 4c2a4ea

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Initialize controller variable Define the controller variable before using it in the OwnerReferences. pkg/ragengine/manifests/leader_worker_set.go [40-46] +controller := true OwnerReferences: []v1.OwnerReference{ { APIVersion: kaitov1alpha1.GroupVersion.String(), Kind: "RAGEngine", UID: ragEngineObj.UID, Name: ragEngineObj.Name, Controller: &controller, }, }, Suggestion importance[1-10]: 8 __ Why: The controller variable is used without being defined, which would cause a compilation error. Initializing it ensures the code compiles and functions correctly.	Medium
General	Improve error logging consistency Ensure that the error handling for ensureService does not overwrite the original error and properly logs it. pkg/ragengine/controllers/ragengine_controller.go [125-131] if err := c.ensureService(ctx, ragEngineObj); err != nil { - if err := c.updateStatusConditionIfNotMatch(ctx, ragEngineObj, kaitov1alpha1.RAGEngineConditionTypeSucceeded, metav1.ConditionFalse, - "ragEngineFailed", err.Error()); err != nil { - klog.ErrorS(err, "failed to update ragEngine status", "ragEngine", klog.KObj(ragEngineObj)) - return reconcile.Result{}, err + if updateErr := c.updateStatusConditionIfNotMatch(ctx, ragEngineObj, kaitov1alpha1.RAGEngineConditionTypeSucceeded, metav1.ConditionFalse, + "ragEngineFailed", err.Error()); updateErr != nil { + klog.ErrorS(updateErr, "failed to update ragEngine status", "ragEngine", klog.KObj(ragEngineObj)) + return reconcile.Result{}, updateErr } + return reconcile.Result{}, err } Suggestion importance[1-10]: 7 __ Why: The suggestion ensures that the original error is preserved and logged, which is important for debugging and maintaining the integrity of error handling.	Medium
	Prevent logging expected NotFound errors Avoid logging errors when they are NotFound, as this is expected behavior and not an error condition. pkg/utils/resources/resources.go [54-56] err := kubeClient.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, resource, &client.GetOptions{}) -klog.ErrorS(err, "resource", resource, "namespace", namespace, "name", name) +if !apierrors.IsNotFound(err) { + klog.ErrorS(err, "resource", resource, "namespace", namespace, "name", name) +} return err Suggestion importance[1-10]: 7 __ Why: Logging NotFound errors as regular errors can clutter logs and mislead developers. This suggestion prevents unnecessary logging of expected conditions.	Medium
	Verify Helm chart URL Ensure the Helm chart URL is correct and accessible. Makefile [428-433] +LWS_VERSION=v0.6.1 +lws-install: ## Install LeaderWorkerSet + helm upgrade --install lws https://github.com/kubernetes-sigs/lws/releases/download/${LWS_VERSION}/lws-chart-${LWS_VERSION}.tgz \ + --namespace lws-system \ + --create-namespace \ + --wait --timeout 300s - Suggestion importance[1-10]: 2 __ Why: The suggestion asks to verify the Helm chart URL, which is a good practice but does not provide any specific change or improvement to the existing code. It is more of a verification step rather than a code modification.	Low

---

Previous suggestions

Suggestions up to commit 177d649

Category	Suggestion	Impact
Possible issue	Initialize controller variable Define the controller variable before using it in OwnerReferences. pkg/ragengine/manifests/leader_worker_set.go [45-50] +controller := true OwnerReferences: []v1.OwnerReference{ { APIVersion: kaitov1alpha1.GroupVersion.String(), Kind: "RAGEngine", UID: ragEngineObj.UID, Name: ragEngineObj.Name, Controller: &controller, }, }, Suggestion importance[1-10]: 8 __ Why: This suggestion addresses a critical issue by defining the controller variable before using it in OwnerReferences. It fixes a potential bug and ensures the code functions correctly.	Medium
General	Prevent unnecessary error logging Avoid logging errors when they are NotFound, as this is expected behavior. pkg/utils/resources/resources.go [54-56] err := kubeClient.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, resource, &client.GetOptions{}) -klog.ErrorS(err, "resource", resource, "namespace", namespace, "name", name) +if !apierrors.IsNotFound(err) { + klog.ErrorS(err, "resource", resource, "namespace", namespace, "name", name) +} return err Suggestion importance[1-10]: 7 __ Why: This suggestion is relevant and improves the code by preventing unnecessary error logging when the error is NotFound. It enhances the maintainability and readability of the code.	Medium
	Improve error handling consistency Ensure proper error handling and logging when updating status conditions. pkg/ragengine/controllers/ragengine_controller.go [123-130] if err := c.ensureService(ctx, ragEngineObj); err != nil { - if err := c.updateStatusConditionIfNotMatch(ctx, ragEngineObj, kaitov1alpha1.RAGEngineConditionTypeSucceeded, metav1.ConditionFalse, - "ragEngineFailed", err.Error()); err != nil { - klog.ErrorS(err, "failed to update ragEngine status", "ragEngine", klog.KObj(ragEngineObj)) - return reconcile.Result{}, err + if updateErr := c.updateStatusConditionIfNotMatch(ctx, ragEngineObj, kaitov1alpha1.RAGEngineConditionTypeSucceeded, metav1.ConditionFalse, + "ragEngineFailed", err.Error()); updateErr != nil { + klog.ErrorS(updateErr, "failed to update ragEngine status", "ragEngine", klog.KObj(ragEngineObj)) + return reconcile.Result{}, updateErr } + return reconcile.Result{}, err } Suggestion importance[1-10]: 2 __ Why: The suggestion is about improving error handling consistency, but it doesn't address any critical issue and only suggests renaming a variable for clarity, which is a minor improvement.	Low
	Verify Helm chart URL Ensure that the LWS_VERSION variable is correctly set and that the Helm chart URL is accurate. Makefile [428-433] +LWS_VERSION=v0.6.1 +lws-install: ## Install LeaderWorkerSet + helm upgrade --install lws https://github.com/kubernetes-sigs/lws/releases/download/${LWS_VERSION}/lws-chart-${LWS_VERSION}.tgz \ + --namespace lws-system \ + --create-namespace \ + --wait --timeout 300s - Suggestion importance[1-10]: 2 __ Why: The suggestion asks to verify the Helm chart URL, which is already correct based on the PR diff. This is a minimal check and offers no direct improvement to the code.	Low

Suggestions up to commit 177d649

Category	Suggestion	Impact
Possible issue	Initialize controller variable Define the controller variable before using it in OwnerReferences. pkg/ragengine/manifests/leader_worker_set.go [45-50] +controller := true OwnerReferences: []v1.OwnerReference{ { APIVersion: kaitov1alpha1.GroupVersion.String(), Kind: "RAGEngine", UID: ragEngineObj.UID, Name: ragEngineObj.Name, Controller: &controller, }, }, Suggestion importance[1-10]: 8 __ Why: The suggestion addresses a critical issue by defining the controller variable before using it in OwnerReferences. This prevents a runtime error due to an undefined variable and ensures the code functions correctly.	Medium
General	Prevent logging expected errors Avoid logging errors when they are not found, as this is expected behavior. pkg/utils/resources/resources.go [54-56] err := kubeClient.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, resource, &client.GetOptions{}) -klog.ErrorS(err, "resource", resource, "namespace", namespace, "name", name) +if !apierrors.IsNotFound(err) { + klog.ErrorS(err, "resource", resource, "namespace", namespace, "name", name) +} return err Suggestion importance[1-10]: 5 __ Why: The suggestion prevents logging errors when they are not found, which is expected behavior. This improves the log clarity by avoiding unnecessary log entries for common scenarios.	Low
	Improve error handling consistency Ensure proper error handling and logging when updating status conditions. pkg/ragengine/controllers/ragengine_controller.go [125-130] if err := c.ensureService(ctx, ragEngineObj); err != nil { - if err := c.updateStatusConditionIfNotMatch(ctx, ragEngineObj, kaitov1alpha1.RAGEngineConditionTypeSucceeded, metav1.ConditionFalse, - "ragEngineFailed", err.Error()); err != nil { - klog.ErrorS(err, "failed to update ragEngine status", "ragEngine", klog.KObj(ragEngineObj)) - return reconcile.Result{}, err + if updateErr := c.updateStatusConditionIfNotMatch(ctx, ragEngineObj, kaitov1alpha1.RAGEngineConditionTypeSucceeded, metav1.ConditionFalse, + "ragEngineFailed", err.Error()); updateErr != nil { + klog.ErrorS(updateErr, "failed to update ragEngine status", "ragEngine", klog.KObj(ragEngineObj)) + return reconcile.Result{}, updateErr } + return reconcile.Result{}, err } Suggestion importance[1-10]: 3 __ Why: The suggestion improves error handling by ensuring that the error returned from updateStatusConditionIfNotMatch is used consistently. However, it does not address a critical issue and offers a minor improvement in error handling.	Low
	Verify Helm chart URL Ensure that the LWS_VERSION variable is correctly set and that the Helm chart URL is accurate. Makefile [428-433] +LWS_VERSION=v0.6.1 +lws-install: ## Install LeaderWorkerSet + helm upgrade --install lws https://github.com/kubernetes-sigs/lws/releases/download/${LWS_VERSION}/lws-chart-${LWS_VERSION}.tgz \ + --namespace lws-system \ + --create-namespace \ + --wait --timeout 300s - Suggestion importance[1-10]: 2 __ Why: The suggestion asks to verify the Helm chart URL, which is already correct based on the PR diff. This is a minimal check and does not provide significant improvement.	Low

[ishaansehgal99]

From high level why is LWS needed for RAG?

[bangqipropel]

what's the idea to do this?