From 65751b2f6d65b9dd8a9663b41b0a2dbf1825bf86 Mon Sep 17 00:00:00 2001 From: KfreeZ Date: Tue, 3 Sep 2024 15:47:31 +0800 Subject: [PATCH 1/7] add UT for reconcile filters Signed-off-by: KfreeZ --- .../controller/gmconnector_controller.go | 151 ++++----- .../controller/gmconnector_controller_test.go | 317 ++++++++++++++---- 2 files changed, 332 insertions(+), 136 deletions(-) diff --git a/microservices-connector/internal/controller/gmconnector_controller.go b/microservices-connector/internal/controller/gmconnector_controller.go index 72fe2977..ee3df3fb 100644 --- a/microservices-connector/internal/controller/gmconnector_controller.go +++ b/microservices-connector/internal/controller/gmconnector_controller.go @@ -797,100 +797,97 @@ func isMetadataChanged(oldObject, newObject *metav1.ObjectMeta) bool { return false } -// SetupWithManager sets up the controller with the Manager. -func (r *GMConnectorReconciler) SetupWithManager(mgr ctrl.Manager) error { - // Predicate to ignore updates to status subresource - ignoreStatusUpdatePredicate := predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - // Cast objects to your GMConnector struct - oldObject, ok1 := e.ObjectOld.(*mcv1alpha3.GMConnector) - newObject, ok2 := e.ObjectNew.(*mcv1alpha3.GMConnector) - if !ok1 || !ok2 { - // Not the correct type, allow the event through - return true - } +func isGMCSpecOrMetadataChanged(e event.UpdateEvent) bool { + oldObject, ok1 := e.ObjectOld.(*mcv1alpha3.GMConnector) + newObject, ok2 := e.ObjectNew.(*mcv1alpha3.GMConnector) + if !ok1 || !ok2 { + // Not the correct type, allow the event through + return true + } - specChanged := !reflect.DeepEqual(oldObject.Spec, newObject.Spec) - metadataChanged := isMetadataChanged(&(oldObject.ObjectMeta), &(newObject.ObjectMeta)) + specChanged := !reflect.DeepEqual(oldObject.Spec, newObject.Spec) + metadataChanged := isMetadataChanged(&(oldObject.ObjectMeta), &(newObject.ObjectMeta)) - fmt.Printf("\n| spec changed %t | meta changed: %t |\n", specChanged, metadataChanged) + fmt.Printf("\n| spec changed %t | meta changed: %t |\n", specChanged, metadataChanged) - // Compare the old and new spec, ignore metadata, status changes - // metadata change: name, namespace, such change should create a new GMC - // status change: depoyment status - return specChanged || metadataChanged - }, - // Other funcs like CreateFunc, DeleteFunc, GenericFunc can be left as default - // if you only want to customize the UpdateFunc behavior. + // Compare the old and new spec, ignore metadata, status changes + // metadata change: name, namespace, such change should create a new GMC + // status change: deployment status + return specChanged || metadataChanged +} + +func isDeploymentStatusChanged(e event.UpdateEvent) bool { + oldDeployment, ok1 := e.ObjectOld.(*appsv1.Deployment) + newDeployment, ok2 := e.ObjectNew.(*appsv1.Deployment) + if !ok1 || !ok2 { + // Not the correct type, allow the event through + return true } - // Predicate to only trigger on status changes for Deployment - deploymentStatusChangePredicate := predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - oldDeployment, ok1 := e.ObjectOld.(*appsv1.Deployment) - newDeployment, ok2 := e.ObjectNew.(*appsv1.Deployment) - if !ok1 || !ok2 { - // Not the correct type, allow the event through - fmt.Printf("| status missing |\n") - return true + if len(newDeployment.OwnerReferences) == 0 { + // fmt.Printf("| %s:%s: no owner reference |\n", newDeployment.Namespace, newDeployment.Name) + return false + } else { + for _, owner := range newDeployment.OwnerReferences { + if owner.Kind == "GMConnector" { + // fmt.Printf("| %s:%s: owner is GMConnector |\n", newDeployment.Namespace, newDeployment.Name) + break } + } + } - if len(newDeployment.OwnerReferences) == 0 { - // fmt.Printf("| %s:%s: no owner reference |\n", newDeployment.Namespace, newDeployment.Name) - return false - } else { - for _, owner := range newDeployment.OwnerReferences { - if owner.Kind == "GMConnector" { - // fmt.Printf("| %s:%s: owner is GMConnector |\n", newDeployment.Namespace, newDeployment.Name) - break - } + oldStatus := corev1.ConditionUnknown + newStatus := corev1.ConditionUnknown + if !reflect.DeepEqual(oldDeployment.Status, newDeployment.Status) { + if newDeployment.Status.Conditions != nil { + for _, condition := range newDeployment.Status.Conditions { + if condition.Type == appsv1.DeploymentAvailable { + newStatus = condition.Status } } - - oldStatus := corev1.ConditionUnknown - newStatus := corev1.ConditionUnknown - if !reflect.DeepEqual(oldDeployment.Status, newDeployment.Status) { - if newDeployment.Status.Conditions != nil { - for _, condition := range newDeployment.Status.Conditions { - if condition.Type == appsv1.DeploymentAvailable { - newStatus = condition.Status - } - } - } - if oldDeployment.Status.Conditions != nil { - for _, condition := range oldDeployment.Status.Conditions { - if condition.Type == appsv1.DeploymentAvailable { - oldStatus = condition.Status - } - } - } - // Only trigger if the status has changed from true to false|unknown or vice versa - if (oldStatus == corev1.ConditionTrue && oldStatus != newStatus) || - (newStatus == corev1.ConditionTrue && oldStatus != newStatus) { - { - fmt.Printf("| %s:%s: status changed from : %v to %v|\n", - newDeployment.Namespace, newDeployment.Name, - oldStatus, newStatus) - return true - } + } + if oldDeployment.Status.Conditions != nil { + for _, condition := range oldDeployment.Status.Conditions { + if condition.Type == appsv1.DeploymentAvailable { + oldStatus = condition.Status } } - return false - }, - //ignore create and delete events, otherwise it will trigger the nested reconcile which is meaningless - CreateFunc: func(e event.CreateEvent) bool { - return false - }, DeleteFunc: func(e event.DeleteEvent) bool { - return false - }, + } + // Only trigger if the status has changed from true to false|unknown or vice versa + if (oldStatus == corev1.ConditionTrue && oldStatus != newStatus) || + (newStatus == corev1.ConditionTrue && oldStatus != newStatus) { + { + fmt.Printf("| %s:%s: status changed from : %v to %v|\n", + newDeployment.Namespace, newDeployment.Name, + oldStatus, newStatus) + return true + } + } + } + return false + +} + +// SetupWithManager sets up the controller with the Manager. +func (r *GMConnectorReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Predicate to ignore updates to status subresource + gmcfilter := predicate.Funcs{ + UpdateFunc: isGMCSpecOrMetadataChanged, + // Other funcs like CreateFunc, DeleteFunc, GenericFunc can be left as default + // if you only want to customize the UpdateFunc behavior. + } + + // Predicate to only trigger on status changes for Deployment + deploymentFilter := predicate.Funcs{ + UpdateFunc: isDeploymentStatusChanged, } return ctrl.NewControllerManagedBy(mgr). - For(&mcv1alpha3.GMConnector{}, builder.WithPredicates(ignoreStatusUpdatePredicate)). + For(&mcv1alpha3.GMConnector{}, builder.WithPredicates(gmcfilter)). Watches( &appsv1.Deployment{}, &handler.EnqueueRequestForObject{}, - builder.WithPredicates(deploymentStatusChangePredicate), + builder.WithPredicates(deploymentFilter), ). Complete(r) } diff --git a/microservices-connector/internal/controller/gmconnector_controller_test.go b/microservices-connector/internal/controller/gmconnector_controller_test.go index 8b600cfb..f290ddb0 100644 --- a/microservices-connector/internal/controller/gmconnector_controller_test.go +++ b/microservices-connector/internal/controller/gmconnector_controller_test.go @@ -14,6 +14,8 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/reconcile" appsv1 "k8s.io/api/apps/v1" @@ -37,7 +39,25 @@ var _ = Describe("GMConnector Controller", func() { BeforeEach(func() { By("creating the custom resource for the Kind GMConnector") - err := k8sClient.Get(ctx, typeNamespacedName, gmconnector) + + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + Scheme: k8sClient.Scheme(), + // Client: k8sClient, + }) + Expect(err).NotTo(HaveOccurred()) + + // Create a new GMConnectorReconciler + reconciler := &GMConnectorReconciler{ + Client: k8sClient, + // Log: ctrl.Log.WithName("controllers").WithName("GMConnector"), + Scheme: mgr.GetScheme(), + } + + // Call the SetupWithManager function + err = reconciler.SetupWithManager(mgr) + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Get(ctx, typeNamespacedName, gmconnector) if err != nil && errors.IsNotFound(err) { resource := &mcv1alpha3.GMConnector{ TypeMeta: metav1.TypeMeta{ @@ -596,6 +616,223 @@ var _ = Describe("GMConnector Controller", func() { }) }) +var _ = Describe("Predicate Functions", func() { + var ( + oldGMConnector *mcv1alpha3.GMConnector + newGMConnector *mcv1alpha3.GMConnector + oldDeployment *appsv1.Deployment + newDeployment *appsv1.Deployment + updateEvent event.UpdateEvent + ) + + BeforeEach(func() { + oldGMConnector = &mcv1alpha3.GMConnector{ + Spec: mcv1alpha3.GMConnectorSpec{ + RouterConfig: mcv1alpha3.RouterConfig{ + Name: "router", + ServiceName: "router-service", + Config: map[string]string{ + "endpoint": "/", + }, + }, + Nodes: map[string]mcv1alpha3.Router{ + "root": { + RouterType: "Sequence", + Steps: []mcv1alpha3.Step{ + { + StepName: Embedding, + Data: "$response", + Executor: mcv1alpha3.Executor{ + InternalService: mcv1alpha3.GMCTarget{ + NameSpace: "default", + ServiceName: "embedding-service", + Config: map[string]string{ + "endpoint": "/v1/embeddings", + "TEI_EMBEDDING_ENDPOINT": "tei-embedding-service", + }, + }, + }, + }, + { + StepName: TeiEmbedding, + Executor: mcv1alpha3.Executor{ + InternalService: mcv1alpha3.GMCTarget{ + NameSpace: "default", + ServiceName: "tei-embedding-service", + Config: map[string]string{ + "endpoint": "/v1/tei-embeddings", + "MODEL_ID": "somemodel", + }, + IsDownstreamService: true, + }, + }, + }, + { + StepName: VectorDB, + Executor: mcv1alpha3.Executor{ + InternalService: mcv1alpha3.GMCTarget{ + NameSpace: "default", + ServiceName: "vector-service", + Config: map[string]string{ + "endpoint": "/v1/vec", + }, + IsDownstreamService: true, + }, + }, + }, + { + StepName: Retriever, + Executor: mcv1alpha3.Executor{ + InternalService: mcv1alpha3.GMCTarget{ + NameSpace: "default", + ServiceName: "retriever-service", + Config: map[string]string{ + "endpoint": "/v1/retrv", + "REDIS_URL": "vector-service", + "TEI_EMBEDDING_ENDPOINT": "tei-embedding-service", + }, + }, + }, + }, + { + StepName: Reranking, + Executor: mcv1alpha3.Executor{ + InternalService: mcv1alpha3.GMCTarget{ + NameSpace: "default", + ServiceName: "rerank-service", + Config: map[string]string{ + "endpoint": "/v1/reranking", + "TEI_RERANKING_ENDPOINT": "tei-reranking-svc", + }, + }, + }, + }, + { + StepName: TeiReranking, + Executor: mcv1alpha3.Executor{ + InternalService: mcv1alpha3.GMCTarget{ + NameSpace: "default", + ServiceName: "tei-reranking-svc", + Config: map[string]string{ + "endpoint": "/rernk", + }, + IsDownstreamService: true, + }, + }, + }, + { + StepName: Tgi, + Executor: mcv1alpha3.Executor{ + InternalService: mcv1alpha3.GMCTarget{ + NameSpace: "default", + ServiceName: "tgi-service-name", + Config: map[string]string{ + "endpoint": "/generate", + }, + IsDownstreamService: true, + }, + }, + }, + { + StepName: Llm, + Executor: mcv1alpha3.Executor{ + InternalService: mcv1alpha3.GMCTarget{ + NameSpace: "default", + ServiceName: "llm-service", + Config: map[string]string{ + "endpoint": "/v1/llm", + "TGI_LLM_ENDPOINT": "tgi-service-name", + }, + }, + }, + }, + }, + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-resource2", + Namespace: "default", + UID: "1f9a258c-b7d2-4bb3-9fac-ddf1b4369d25", + }, + } + newGMConnector = oldGMConnector.DeepCopy() + oldDeployment = &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "embedding-service-deployment", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "gmc.opea.io/v1alpha3", + Kind: "GMConnector", + Name: "test-resource2", + UID: "1f9a258c-b7d2-4bb3-9fac-ddf1b4369d25", + }, + }, + }, + Status: appsv1.DeploymentStatus{ + AvailableReplicas: 1, + Conditions: []appsv1.DeploymentCondition{ + { + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }, + }, + ReadyReplicas: 1, + Replicas: 1, + UpdatedReplicas: 1, + }, + } + newDeployment = oldDeployment.DeepCopy() + }) + + Describe("isGMConnectorSpecOrMetadataChanged", func() { + It("should return true if the spec has changed", func() { + newGMConnector.Spec.RouterConfig.Name = "newRouter" + updateEvent = event.UpdateEvent{ + ObjectOld: oldGMConnector, + ObjectNew: newGMConnector, + } + Expect(isGMCSpecOrMetadataChanged(updateEvent)).To(BeTrue()) + }) + + It("should return true if the metadata has changed", func() { + newGMConnector.ObjectMeta.Name = "new-name" + updateEvent = event.UpdateEvent{ + ObjectOld: oldGMConnector, + ObjectNew: newGMConnector, + } + Expect(isGMCSpecOrMetadataChanged(updateEvent)).To(BeTrue()) + }) + It("should return false if neither spec nor metadata has changed", func() { + updateEvent = event.UpdateEvent{ + ObjectOld: oldGMConnector, + ObjectNew: newGMConnector, + } + Expect(isGMCSpecOrMetadataChanged(updateEvent)).To(BeFalse()) + }) + }) + + Describe("isDeploymentStatusChanged", func() { + It("should return true if the status has changed", func() { + newDeployment.Status.Conditions[0].Status = corev1.ConditionFalse + updateEvent = event.UpdateEvent{ + ObjectOld: oldDeployment, + ObjectNew: newDeployment, + } + Expect(isDeploymentStatusChanged(updateEvent)).To(BeTrue()) + }) + + It("should return false if the status has not changed", func() { + updateEvent = event.UpdateEvent{ + ObjectOld: oldDeployment, + ObjectNew: newDeployment, + } + Expect(isDeploymentStatusChanged(updateEvent)).To(BeFalse()) + }) + }) +}) + func TestGetServiceURL(t *testing.T) { service := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -756,65 +993,27 @@ func TestIsMetadataChanged(t *testing.T) { } } -// func TestHandleStatusUpdate(t *testing.T) { -// // Create a fake GMConnector object -// graph := &mcv1alpha3.GMConnector{ -// ObjectMeta: metav1.ObjectMeta{ -// Namespace: "default", -// Name: "test-graph", -// }, -// } - -// // Create a fake Deployment object -// deployment := &appsv1.Deployment{ -// ObjectMeta: metav1.ObjectMeta{ -// Namespace: "default", -// Name: "test-deployment", -// OwnerReferences: []metav1.OwnerReference{ -// { -// Kind: "GMConnector", -// Name: "test-graph", -// }, -// }, -// }, +// func TestSetupWithManager(t *testing.T) { +// // Set up a fake client and manager +// fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() +// mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ +// Scheme: scheme.Scheme, +// Client: fakeClient, +// }) +// assert.NoError(t, err) + +// // Create a new GMConnectorReconciler +// reconciler := &GMConnectorReconciler{ +// Client: mgr.GetClient(), +// Log: ctrl.Log.WithName("controllers").WithName("GMConnector"), +// Scheme: mgr.GetScheme(), // } -// // Create a fake GMConnectorReconciler -// r := &GMConnectorReconciler{ -// Client: fake.NewFakeClientWithScheme(scheme.Scheme, graph, deployment), -// } - -// // Create a fake context -// ctx := context.TODO() - -// // Create a fake reconcile request -// req := reconcile.Request{ -// NamespacedName: types.NamespacedName{ -// Namespace: "default", -// Name: "test-deployment", -// }, -// } - -// // Call the handleStatusUpdate function -// result, err := r.handleStatusUpdate(ctx, deployment) - -// // Check the result and error -// if err != nil { -// t.Errorf("handleStatusUpdate returned an error: %v", err) -// } - -// if result != (reconcile.Result{}) { -// t.Errorf("handleStatusUpdate returned an unexpected result: %v", result) -// } +// // Call the SetupWithManager function +// err = reconciler.SetupWithManager(mgr) +// assert.NoError(t, err) -// // Check if the GMConnector object's status has been updated -// err = r.Get(ctx, types.NamespacedName{Namespace: "default", Name: "test-graph"}, graph) -// if err != nil { -// t.Errorf("Failed to get GMConnector object: %v", err) -// } - -// expectedStatus := "0/0/1" -// if graph.Status.Status != expectedStatus { -// t.Errorf("GMConnector object's status is not updated correctly. Expected: %s, Got: %s", expectedStatus, graph.Status.Status) -// } +// // Additional assertions can be added here to verify the setup +// // For example, you can check if the controller is added to the manager +// assert.NotNil(t, mgr.GetController("GMConnector")) // } From 99c826e2b978f7dd0f1e5cd9aa60da7ff787dc3f Mon Sep 17 00:00:00 2001 From: KfreeZ Date: Tue, 3 Sep 2024 15:51:17 +0800 Subject: [PATCH 2/7] remove useless code Signed-off-by: KfreeZ --- .../controller/gmconnector_controller_test.go | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/microservices-connector/internal/controller/gmconnector_controller_test.go b/microservices-connector/internal/controller/gmconnector_controller_test.go index f290ddb0..3bf1f1a9 100644 --- a/microservices-connector/internal/controller/gmconnector_controller_test.go +++ b/microservices-connector/internal/controller/gmconnector_controller_test.go @@ -992,28 +992,3 @@ func TestIsMetadataChanged(t *testing.T) { t.Errorf("Expected metadata changes to not be detected, but got true") } } - -// func TestSetupWithManager(t *testing.T) { -// // Set up a fake client and manager -// fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() -// mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ -// Scheme: scheme.Scheme, -// Client: fakeClient, -// }) -// assert.NoError(t, err) - -// // Create a new GMConnectorReconciler -// reconciler := &GMConnectorReconciler{ -// Client: mgr.GetClient(), -// Log: ctrl.Log.WithName("controllers").WithName("GMConnector"), -// Scheme: mgr.GetScheme(), -// } - -// // Call the SetupWithManager function -// err = reconciler.SetupWithManager(mgr) -// assert.NoError(t, err) - -// // Additional assertions can be added here to verify the setup -// // For example, you can check if the controller is added to the manager -// assert.NotNil(t, mgr.GetController("GMConnector")) -// } From f83619195bd3687aa959f69c39299bcd9f7d7fc9 Mon Sep 17 00:00:00 2001 From: KfreeZ Date: Tue, 3 Sep 2024 16:14:19 +0800 Subject: [PATCH 3/7] update tests Signed-off-by: KfreeZ --- .../controller/gmconnector_controller_test.go | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/microservices-connector/internal/controller/gmconnector_controller_test.go b/microservices-connector/internal/controller/gmconnector_controller_test.go index 3bf1f1a9..f5e51f86 100644 --- a/microservices-connector/internal/controller/gmconnector_controller_test.go +++ b/microservices-connector/internal/controller/gmconnector_controller_test.go @@ -39,25 +39,7 @@ var _ = Describe("GMConnector Controller", func() { BeforeEach(func() { By("creating the custom resource for the Kind GMConnector") - - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ - Scheme: k8sClient.Scheme(), - // Client: k8sClient, - }) - Expect(err).NotTo(HaveOccurred()) - - // Create a new GMConnectorReconciler - reconciler := &GMConnectorReconciler{ - Client: k8sClient, - // Log: ctrl.Log.WithName("controllers").WithName("GMConnector"), - Scheme: mgr.GetScheme(), - } - - // Call the SetupWithManager function - err = reconciler.SetupWithManager(mgr) - Expect(err).NotTo(HaveOccurred()) - - err = k8sClient.Get(ctx, typeNamespacedName, gmconnector) + err := k8sClient.Get(ctx, typeNamespacedName, gmconnector) if err != nil && errors.IsNotFound(err) { resource := &mcv1alpha3.GMConnector{ TypeMeta: metav1.TypeMeta{ @@ -626,6 +608,24 @@ var _ = Describe("Predicate Functions", func() { ) BeforeEach(func() { + + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + Scheme: k8sClient.Scheme(), + // Client: k8sClient, + }) + Expect(err).NotTo(HaveOccurred()) + + // Create a new GMConnectorReconciler + reconciler := &GMConnectorReconciler{ + Client: k8sClient, + // Log: ctrl.Log.WithName("controllers").WithName("GMConnector"), + Scheme: mgr.GetScheme(), + } + + // Call the SetupWithManager function + err = reconciler.SetupWithManager(mgr) + Expect(err).NotTo(HaveOccurred()) + oldGMConnector = &mcv1alpha3.GMConnector{ Spec: mcv1alpha3.GMConnectorSpec{ RouterConfig: mcv1alpha3.RouterConfig{ @@ -752,7 +752,7 @@ var _ = Describe("Predicate Functions", func() { }, ObjectMeta: metav1.ObjectMeta{ Name: "test-resource2", - Namespace: "default", + Namespace: "default2", UID: "1f9a258c-b7d2-4bb3-9fac-ddf1b4369d25", }, } @@ -760,7 +760,7 @@ var _ = Describe("Predicate Functions", func() { oldDeployment = &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "embedding-service-deployment", - Namespace: "default", + Namespace: "default2", OwnerReferences: []metav1.OwnerReference{ { APIVersion: "gmc.opea.io/v1alpha3", From bef38c4071b740c72d7e7394af4d6ac44b0d4abb Mon Sep 17 00:00:00 2001 From: KfreeZ Date: Tue, 3 Sep 2024 16:32:18 +0800 Subject: [PATCH 4/7] debug CI/CD failed Signed-off-by: KfreeZ --- .../controller/gmconnector_controller_test.go | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/microservices-connector/internal/controller/gmconnector_controller_test.go b/microservices-connector/internal/controller/gmconnector_controller_test.go index f5e51f86..60cf7b9e 100644 --- a/microservices-connector/internal/controller/gmconnector_controller_test.go +++ b/microservices-connector/internal/controller/gmconnector_controller_test.go @@ -14,7 +14,6 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -609,22 +608,22 @@ var _ = Describe("Predicate Functions", func() { BeforeEach(func() { - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ - Scheme: k8sClient.Scheme(), - // Client: k8sClient, - }) - Expect(err).NotTo(HaveOccurred()) - - // Create a new GMConnectorReconciler - reconciler := &GMConnectorReconciler{ - Client: k8sClient, - // Log: ctrl.Log.WithName("controllers").WithName("GMConnector"), - Scheme: mgr.GetScheme(), - } - - // Call the SetupWithManager function - err = reconciler.SetupWithManager(mgr) - Expect(err).NotTo(HaveOccurred()) + // mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + // Scheme: k8sClient.Scheme(), + // // Client: k8sClient, + // }) + // Expect(err).NotTo(HaveOccurred()) + + // // Create a new GMConnectorReconciler + // reconciler := &GMConnectorReconciler{ + // Client: k8sClient, + // // Log: ctrl.Log.WithName("controllers").WithName("GMConnector"), + // Scheme: mgr.GetScheme(), + // } + + // // Call the SetupWithManager function + // err = reconciler.SetupWithManager(mgr) + // Expect(err).NotTo(HaveOccurred()) oldGMConnector = &mcv1alpha3.GMConnector{ Spec: mcv1alpha3.GMConnectorSpec{ From 0f1e843bf38189ac4133fc84a77908bb550f5499 Mon Sep 17 00:00:00 2001 From: KfreeZ Date: Wed, 4 Sep 2024 09:24:29 +0800 Subject: [PATCH 5/7] skip use pod_count in e2e's check_gmc_status, the pod_count is not acurrate sometimes when terninating pod is not cleared but counted Signed-off-by: KfreeZ --- .github/workflows/scripts/e2e/gmc_xeon_test.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/scripts/e2e/gmc_xeon_test.sh b/.github/workflows/scripts/e2e/gmc_xeon_test.sh index 39f1716e..080279df 100755 --- a/.github/workflows/scripts/e2e/gmc_xeon_test.sh +++ b/.github/workflows/scripts/e2e/gmc_xeon_test.sh @@ -137,7 +137,7 @@ function validate_audioqa() { pods_count=$(kubectl get pods -n $AUDIOQA_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace - check_gmc_status $AUDIOQA_NAMESPACE 'audioqa' $((pods_count-1)) 0 7 + check_gmc_status $AUDIOQA_NAMESPACE 'audioqa' 7 0 7 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -190,7 +190,7 @@ function validate_chatqna() { pods_count=$(kubectl get pods -n $CHATQNA_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace - check_gmc_status $CHATQNA_NAMESPACE 'chatqa' $((pods_count-1)) 0 9 + check_gmc_status $CHATQNA_NAMESPACE 'chatqa' 9 0 9 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -260,7 +260,7 @@ function validate_chatqna_with_dataprep() { pods_count=$(kubectl get pods -n $CHATQNA_DATAPREP_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace - check_gmc_status $CHATQNA_DATAPREP_NAMESPACE 'chatqa' $((pods_count-1)) 0 10 + check_gmc_status $CHATQNA_DATAPREP_NAMESPACE 'chatqa' 10 0 10 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -353,7 +353,7 @@ function validate_chatqna_in_switch() { pods_count=$(kubectl get pods -n $CHATQNA_SWITCH_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace - check_gmc_status $CHATQNA_SWITCH_NAMESPACE 'switch' $((pods_count-1)) 0 15 + check_gmc_status $CHATQNA_SWITCH_NAMESPACE 'switch' 15 0 15 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -444,7 +444,7 @@ function validate_modify_config() { fi pods_count=$(kubectl get pods -n $MODIFY_STEP_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) - check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' $pods_count 0 3 + check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' 3 0 3 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -461,7 +461,7 @@ function validate_modify_config() { exit 1 fi - check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' $pods_count 0 3 + check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' 3 0 3 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -495,7 +495,7 @@ function validate_remove_step() { fi pods_count=$(kubectl get pods -n $DELETE_STEP_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) - check_gmc_status $DELETE_STEP_NAMESPACE 'codegen' $pods_count 0 3 + check_gmc_status $DELETE_STEP_NAMESPACE 'codegen' 3 0 3 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 From 72f2321ab262fd55b06247f9daf2c9454a721b9d Mon Sep 17 00:00:00 2001 From: KfreeZ Date: Wed, 4 Sep 2024 15:53:39 +0800 Subject: [PATCH 6/7] better than hardcode Signed-off-by: KfreeZ --- .../workflows/scripts/e2e/gmc_xeon_test.sh | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/scripts/e2e/gmc_xeon_test.sh b/.github/workflows/scripts/e2e/gmc_xeon_test.sh index 080279df..2dddf92d 100755 --- a/.github/workflows/scripts/e2e/gmc_xeon_test.sh +++ b/.github/workflows/scripts/e2e/gmc_xeon_test.sh @@ -134,10 +134,10 @@ function validate_audioqa() { exit 1 fi - pods_count=$(kubectl get pods -n $AUDIOQA_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) + pods_count=$(kubectl get pods -n $AUDIOQA_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace - check_gmc_status $AUDIOQA_NAMESPACE 'audioqa' 7 0 7 + check_gmc_status $AUDIOQA_NAMESPACE 'audioqa' $((pods_count-1)) 0 7 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -187,10 +187,10 @@ function validate_chatqna() { exit 1 fi - pods_count=$(kubectl get pods -n $CHATQNA_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) + pods_count=$(kubectl get pods -n $CHATQNA_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace - check_gmc_status $CHATQNA_NAMESPACE 'chatqa' 9 0 9 + check_gmc_status $CHATQNA_NAMESPACE 'chatqa' $((pods_count-1)) 0 9 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -257,10 +257,10 @@ function validate_chatqna_with_dataprep() { exit 1 fi - pods_count=$(kubectl get pods -n $CHATQNA_DATAPREP_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) + pods_count=$(kubectl get pods -n $CHATQNA_DATAPREP_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace - check_gmc_status $CHATQNA_DATAPREP_NAMESPACE 'chatqa' 10 0 10 + check_gmc_status $CHATQNA_DATAPREP_NAMESPACE 'chatqa' $((pods_count-1)) 0 10 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -350,10 +350,10 @@ function validate_chatqna_in_switch() { exit 1 fi - pods_count=$(kubectl get pods -n $CHATQNA_SWITCH_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) + pods_count=$(kubectl get pods -n $CHATQNA_SWITCH_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace - check_gmc_status $CHATQNA_SWITCH_NAMESPACE 'switch' 15 0 15 + check_gmc_status $CHATQNA_SWITCH_NAMESPACE 'switch' $((pods_count-1)) 0 15 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -443,8 +443,8 @@ function validate_modify_config() { exit 1 fi - pods_count=$(kubectl get pods -n $MODIFY_STEP_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) - check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' 3 0 3 + pods_count=$(kubectl get pods -n $MODIFY_STEP_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) + check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' $((pods_count-1)) 0 3 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -461,7 +461,7 @@ function validate_modify_config() { exit 1 fi - check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' 3 0 3 + check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' $((pods_count-1)) 0 3 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 @@ -494,8 +494,8 @@ function validate_remove_step() { exit 1 fi - pods_count=$(kubectl get pods -n $DELETE_STEP_NAMESPACE -o jsonpath='{.items[*].metadata.name}' | wc -w) - check_gmc_status $DELETE_STEP_NAMESPACE 'codegen' 3 0 3 + pods_count=$(kubectl get pods -n $DELETE_STEP_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) + check_gmc_status $DELETE_STEP_NAMESPACE 'codegen' $((pods_count-1)) 0 3 if [ $? -ne 0 ]; then echo "GMC status is not as expected" exit 1 From 82ead75d21eba9e65245e65c32118ae54229213c Mon Sep 17 00:00:00 2001 From: KfreeZ Date: Wed, 4 Sep 2024 16:11:08 +0800 Subject: [PATCH 7/7] count the pods right Signed-off-by: KfreeZ --- .github/workflows/scripts/e2e/gmc_xeon_test.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/scripts/e2e/gmc_xeon_test.sh b/.github/workflows/scripts/e2e/gmc_xeon_test.sh index 2dddf92d..cb7f4494 100755 --- a/.github/workflows/scripts/e2e/gmc_xeon_test.sh +++ b/.github/workflows/scripts/e2e/gmc_xeon_test.sh @@ -134,7 +134,7 @@ function validate_audioqa() { exit 1 fi - pods_count=$(kubectl get pods -n $AUDIOQA_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) + pods_count=$(kubectl get pods -n $AUDIOQA_NAMESPACE --no-headers | grep -v "Terminating" | wc -l) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace check_gmc_status $AUDIOQA_NAMESPACE 'audioqa' $((pods_count-1)) 0 7 @@ -187,7 +187,7 @@ function validate_chatqna() { exit 1 fi - pods_count=$(kubectl get pods -n $CHATQNA_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) + pods_count=$(kubectl get pods -n $CHATQNA_NAMESPACE --no-headers | grep -v "Terminating" | wc -l) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace check_gmc_status $CHATQNA_NAMESPACE 'chatqa' $((pods_count-1)) 0 9 @@ -257,7 +257,7 @@ function validate_chatqna_with_dataprep() { exit 1 fi - pods_count=$(kubectl get pods -n $CHATQNA_DATAPREP_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) + pods_count=$(kubectl get pods -n $CHATQNA_DATAPREP_NAMESPACE --no-headers | grep -v "Terminating" | wc -l) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace check_gmc_status $CHATQNA_DATAPREP_NAMESPACE 'chatqa' $((pods_count-1)) 0 10 @@ -350,7 +350,7 @@ function validate_chatqna_in_switch() { exit 1 fi - pods_count=$(kubectl get pods -n $CHATQNA_SWITCH_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) + pods_count=$(kubectl get pods -n $CHATQNA_SWITCH_NAMESPACE --no-headers | grep -v "Terminating" | wc -l) # expected_ready_pods, expected_external_pods, expected_total_pods # pods_count-1 is to exclude the client pod in this namespace check_gmc_status $CHATQNA_SWITCH_NAMESPACE 'switch' $((pods_count-1)) 0 15 @@ -443,7 +443,7 @@ function validate_modify_config() { exit 1 fi - pods_count=$(kubectl get pods -n $MODIFY_STEP_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) + pods_count=$(kubectl get pods -n $MODIFY_STEP_NAMESPACE --no-headers | grep -v "Terminating" | wc -l) check_gmc_status $MODIFY_STEP_NAMESPACE 'codegen' $((pods_count-1)) 0 3 if [ $? -ne 0 ]; then echo "GMC status is not as expected" @@ -494,7 +494,7 @@ function validate_remove_step() { exit 1 fi - pods_count=$(kubectl get pods -n $DELETE_STEP_NAMESPACE --no-headers | grep -v "Terminating" | wc -w) + pods_count=$(kubectl get pods -n $DELETE_STEP_NAMESPACE --no-headers | grep -v "Terminating" | wc -l) check_gmc_status $DELETE_STEP_NAMESPACE 'codegen' $((pods_count-1)) 0 3 if [ $? -ne 0 ]; then echo "GMC status is not as expected"