From 05151d70692b48f51a98ea534f2031000482f36d Mon Sep 17 00:00:00 2001 From: razsamuel Date: Thu, 16 May 2024 12:13:49 +0300 Subject: [PATCH 1/7] Added env var of the feature, and implement a mechanism to reduce requests using hash --- pkg/config/config.go | 1 + pkg/config/models.go | 1 + pkg/k8s/controller.go | 55 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 625cff3..204b0bc 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -38,6 +38,7 @@ func Init() { NewString(&ApplicationConfig.PortClientSecret, "port-client-secret", "", "Port client secret. Required.") NewBool(&ApplicationConfig.CreateDefaultResources, "create-default-resources", true, "Create default resources on installation. Optional.") NewBool(&ApplicationConfig.OverwriteConfigurationOnRestart, "overwrite-configuration-on-restart", false, "Overwrite the configuration in port on restarting the exporter. Optional.") + NewBool(&ApplicationConfig.UpdateEntityOnlyOnDiff, "update-entity-only-on-diff", false, "Optimization to reduce requests to port. Optional.") // Deprecated NewBool(&ApplicationConfig.DeleteDependents, "delete-dependents", false, "Delete dependents. Optional.") diff --git a/pkg/config/models.go b/pkg/config/models.go index 8020d39..fe3e99d 100644 --- a/pkg/config/models.go +++ b/pkg/config/models.go @@ -26,4 +26,5 @@ type ApplicationConfiguration struct { Resources []port.Resource DeleteDependents bool `json:"deleteDependents,omitempty"` CreateMissingRelatedEntities bool `json:"createMissingRelatedEntities,omitempty"` + UpdateEntityOnlyOnDiff bool `json:"updateEntityOnlyOnDiff,omitempty"` } diff --git a/pkg/k8s/controller.go b/pkg/k8s/controller.go index dde3813..613ceaf 100644 --- a/pkg/k8s/controller.go +++ b/pkg/k8s/controller.go @@ -8,12 +8,15 @@ import ( "github.com/port-labs/port-k8s-exporter/pkg/port" "github.com/port-labs/port-k8s-exporter/pkg/port/cli" "github.com/port-labs/port-k8s-exporter/pkg/port/mapping" + "hash/fnv" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/informers" + "strconv" "time" + "encoding/json" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/cache" @@ -68,7 +71,42 @@ func NewController(resource port.AggregatedResource, portClient *cli.PortClient, item.ActionType = UpdateAction item.Key, err = cache.MetaNamespaceKeyFunc(new) if err == nil { - controller.workqueue.Add(item) + + if config.ApplicationConfig.UpdateEntityOnlyOnDiff == false { + controller.workqueue.Add(item) + } else { + for _, kindConfig := range controller.resource.KindConfigs { + oldEntities, err := controller.getObjectEntities(old, kindConfig.Selector, kindConfig.Port.Entity.Mappings) + if err != nil { + klog.Errorf("Error getting old entities: %v", err) + controller.workqueue.Add(item) + break + } + newEntities, err := controller.getObjectEntities(new, kindConfig.Selector, kindConfig.Port.Entity.Mappings) + if err != nil { + klog.Errorf("Error getting new entities: %v", err) + controller.workqueue.Add(item) + break + } + oldEntitiesHash, err := hashAllEntities(oldEntities) + if err != nil { + klog.Errorf("Error hashing old entities: %v", err) + controller.workqueue.Add(item) + break + } + newEntitiesHash, err := hashAllEntities(newEntities) + if err != nil { + klog.Errorf("Error hashing new entities: %v", err) + controller.workqueue.Add(item) + break + } + + if oldEntitiesHash != newEntitiesHash { + controller.workqueue.Add(item) + break + } + } + } } }, DeleteFunc: func(obj interface{}) { @@ -335,3 +373,18 @@ func (c *Controller) GetEntitiesSet() (map[string]interface{}, error) { return k8sEntitiesSet, nil } + +func hashAllEntities(entities []port.Entity) (string, error) { + h := fnv.New64a() + for _, entity := range entities { + entityBytes, err := json.Marshal(entity) + if err != nil { + return "", err + } + _, err = h.Write(entityBytes) + if err != nil { + return "", err + } + } + return strconv.FormatUint(h.Sum64(), 10), nil +} From d29109fb64ae9adca4d94827dbdf6a70b6d55d89 Mon Sep 17 00:00:00 2001 From: razsamuel Date: Thu, 16 May 2024 16:09:46 +0300 Subject: [PATCH 2/7] Add tests --- pkg/k8s/controller.go | 69 +++++++++++++------------ pkg/k8s/controller_test.go | 103 +++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 33 deletions(-) diff --git a/pkg/k8s/controller.go b/pkg/k8s/controller.go index 7d8c28b..3863cbf 100644 --- a/pkg/k8s/controller.go +++ b/pkg/k8s/controller.go @@ -73,40 +73,8 @@ func NewController(resource port.AggregatedResource, portClient *cli.PortClient, item.Key, err = cache.MetaNamespaceKeyFunc(new) if err == nil { - if config.ApplicationConfig.UpdateEntityOnlyOnDiff == false { + if controller.shouldSendUpdateEvent(old, new, config.ApplicationConfig.UpdateEntityOnlyOnDiff) { controller.workqueue.Add(item) - } else { - for _, kindConfig := range controller.resource.KindConfigs { - oldEntities, err := controller.getObjectEntities(old, kindConfig.Selector, kindConfig.Port.Entity.Mappings, kindConfig.Port.ItemsToParse) - if err != nil { - klog.Errorf("Error getting old entities: %v", err) - controller.workqueue.Add(item) - break - } - newEntities, err := controller.getObjectEntities(new, kindConfig.Selector, kindConfig.Port.Entity.Mappings, kindConfig.Port.ItemsToParse) - if err != nil { - klog.Errorf("Error getting new entities: %v", err) - controller.workqueue.Add(item) - break - } - oldEntitiesHash, err := hashAllEntities(oldEntities) - if err != nil { - klog.Errorf("Error hashing old entities: %v", err) - controller.workqueue.Add(item) - break - } - newEntitiesHash, err := hashAllEntities(newEntities) - if err != nil { - klog.Errorf("Error hashing new entities: %v", err) - controller.workqueue.Add(item) - break - } - - if oldEntitiesHash != newEntitiesHash { - controller.workqueue.Add(item) - break - } - } } } }, @@ -437,3 +405,38 @@ func hashAllEntities(entities []port.Entity) (string, error) { } return strconv.FormatUint(h.Sum64(), 10), nil } + +func (c *Controller) shouldSendUpdateEvent(old interface{}, new interface{}, updateEntityOnlyOnDiff bool) bool { + + if updateEntityOnlyOnDiff == false { + return true + } else { + for _, kindConfig := range c.resource.KindConfigs { + oldEntities, err := c.getObjectEntities(old, kindConfig.Selector, kindConfig.Port.Entity.Mappings, kindConfig.Port.ItemsToParse) + if err != nil { + klog.Errorf("Error getting old entities: %v", err) + return true + } + newEntities, err := c.getObjectEntities(new, kindConfig.Selector, kindConfig.Port.Entity.Mappings, kindConfig.Port.ItemsToParse) + if err != nil { + klog.Errorf("Error getting new entities: %v", err) + return true + } + oldEntitiesHash, err := hashAllEntities(oldEntities) + if err != nil { + klog.Errorf("Error hashing old entities: %v", err) + return true + } + newEntitiesHash, err := hashAllEntities(newEntities) + if err != nil { + klog.Errorf("Error hashing new entities: %v", err) + return true + } + + if oldEntitiesHash != newEntitiesHash { + return true + } + } + } + return false +} diff --git a/pkg/k8s/controller_test.go b/pkg/k8s/controller_test.go index 4440789..98e9d63 100644 --- a/pkg/k8s/controller_test.go +++ b/pkg/k8s/controller_test.go @@ -3,6 +3,9 @@ package k8s import ( "context" "fmt" + "github.com/stretchr/testify/assert" + "k8s.io/client-go/kubernetes/fake" + "log" "reflect" "strings" "testing" @@ -102,6 +105,33 @@ func newDeployment() *appsv1.Deployment { } } +func newDeploymentWithCustomLabels(labels map[string]string) *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "port-k8s-exporter", + Namespace: "port-k8s-exporter", + }, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: labels, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: labels, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "port-k8s-exporter", + Image: "port-k8s-exporter:latest", + }, + }, + }, + }, + }, + } +} + func newUnstructured(obj interface{}) *unstructured.Unstructured { res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) if err != nil { @@ -317,3 +347,76 @@ func TestGetEntitiesSet(t *testing.T) { f := newFixture(t, "", "", "", resource, objects) f.runControllerGetEntitiesSet(expectedEntitiesSet, false) } + +func TestUpdateDeploymentWithoutDiff(t *testing.T) { + clientset := fake.NewSimpleClientset() + d := newDeployment() + a, err := clientset.AppsV1().Deployments("port-k8s-exporter").Update(context.Background(), d, metav1.UpdateOptions{}) + if err != nil { + return + } + log.Fatal(a) + +} + +func TestUpdateHandler(t *testing.T) { + oldDeployment := []runtime.Object{newUnstructured(newDeployment())}[0] + newDep := []runtime.Object{newUnstructured(newDeploymentWithCustomLabels(map[string]string{"app": "port-k8s-exporter", "new": "label"}))}[0] + fullMapping := newResource("", []port.EntityMapping{ + { + Identifier: ".metadata.name", + Blueprint: "\"k8s-export-test-bp\"", + Icon: "\"Microservice\"", + Team: "\"Test\"", + Properties: map[string]string{ + "text": "\"pod\"", + "num": "1", + "bool": "true", + "obj": ".spec.selector", + "arr": ".spec.template.spec.containers", + }, + Relations: map[string]string{ + "k8s-relation": "\"e_AgPMYvq1tAs8TuqM\"", + }, + }, + }) + + partialMappingWithoutLables := newResource("", []port.EntityMapping{ + { + Identifier: ".metadata.name", + Blueprint: "\"k8s-export-test-bp\"", + Icon: "\"Microservice\"", + Team: "\"Test\"", + Properties: map[string]string{ + "text": "\"pod\"", + "num": "1", + "bool": "true", + }, + Relations: map[string]string{ + "k8s-relation": "\"e_AgPMYvq1tAs8TuqM\"", + }, + }, + }) + + controllerWithFullMapping := newFixture(t, "", "", "", fullMapping, []runtime.Object{}).controller + + // Same object and feature flag on - should return false + result := controllerWithFullMapping.shouldSendUpdateEvent(oldDeployment, oldDeployment, true) + assert.False(t, result, "Expected false when same object and feature flag is on") + + // Different object and feature flag on - should return true + result = controllerWithFullMapping.shouldSendUpdateEvent(oldDeployment, newDep, true) + assert.True(t, result, "Expected true when different object and feature flag is on") + + // feature flag off - should return true regardless of whether objects are same or different + result = controllerWithFullMapping.shouldSendUpdateEvent(oldDeployment, oldDeployment, false) + assert.True(t, result, "Expected true when same object and feature flag is off") + + result = controllerWithFullMapping.shouldSendUpdateEvent(oldDeployment, newDep, false) + assert.True(t, result, "Expected true when different object and feature flag is off") + + controllerWithPartialMapping := newFixture(t, "", "", "", partialMappingWithoutLables, []runtime.Object{}).controller + result = controllerWithPartialMapping.shouldSendUpdateEvent(oldDeployment, newDep, true) + assert.False(t, result, "Expected false when objects are different but mapping not include the change and feature flag is on") + +} From 37e0675a00cbda9f254ed16f94eb2b868cf3ceaa Mon Sep 17 00:00:00 2001 From: razsamuel Date: Thu, 16 May 2024 17:12:49 +0300 Subject: [PATCH 3/7] CR Fixes --- pkg/k8s/controller.go | 52 +++++++++++++++++++------------------- pkg/k8s/controller_test.go | 16 +++--------- 2 files changed, 29 insertions(+), 39 deletions(-) diff --git a/pkg/k8s/controller.go b/pkg/k8s/controller.go index 3863cbf..712f213 100644 --- a/pkg/k8s/controller.go +++ b/pkg/k8s/controller.go @@ -410,33 +410,33 @@ func (c *Controller) shouldSendUpdateEvent(old interface{}, new interface{}, upd if updateEntityOnlyOnDiff == false { return true - } else { - for _, kindConfig := range c.resource.KindConfigs { - oldEntities, err := c.getObjectEntities(old, kindConfig.Selector, kindConfig.Port.Entity.Mappings, kindConfig.Port.ItemsToParse) - if err != nil { - klog.Errorf("Error getting old entities: %v", err) - return true - } - newEntities, err := c.getObjectEntities(new, kindConfig.Selector, kindConfig.Port.Entity.Mappings, kindConfig.Port.ItemsToParse) - if err != nil { - klog.Errorf("Error getting new entities: %v", err) - return true - } - oldEntitiesHash, err := hashAllEntities(oldEntities) - if err != nil { - klog.Errorf("Error hashing old entities: %v", err) - return true - } - newEntitiesHash, err := hashAllEntities(newEntities) - if err != nil { - klog.Errorf("Error hashing new entities: %v", err) - return true - } + } + for _, kindConfig := range c.resource.KindConfigs { + oldEntities, err := c.getObjectEntities(old, kindConfig.Selector, kindConfig.Port.Entity.Mappings, kindConfig.Port.ItemsToParse) + if err != nil { + klog.Errorf("Error getting old entities: %v", err) + return true + } + newEntities, err := c.getObjectEntities(new, kindConfig.Selector, kindConfig.Port.Entity.Mappings, kindConfig.Port.ItemsToParse) + if err != nil { + klog.Errorf("Error getting new entities: %v", err) + return true + } + oldEntitiesHash, err := hashAllEntities(oldEntities) + if err != nil { + klog.Errorf("Error hashing old entities: %v", err) + return true + } + newEntitiesHash, err := hashAllEntities(newEntities) + if err != nil { + klog.Errorf("Error hashing new entities: %v", err) + return true + } - if oldEntitiesHash != newEntitiesHash { - return true - } + if oldEntitiesHash == newEntitiesHash { + return false } } - return false + + return true } diff --git a/pkg/k8s/controller_test.go b/pkg/k8s/controller_test.go index 98e9d63..af906fc 100644 --- a/pkg/k8s/controller_test.go +++ b/pkg/k8s/controller_test.go @@ -4,8 +4,6 @@ import ( "context" "fmt" "github.com/stretchr/testify/assert" - "k8s.io/client-go/kubernetes/fake" - "log" "reflect" "strings" "testing" @@ -348,17 +346,6 @@ func TestGetEntitiesSet(t *testing.T) { f.runControllerGetEntitiesSet(expectedEntitiesSet, false) } -func TestUpdateDeploymentWithoutDiff(t *testing.T) { - clientset := fake.NewSimpleClientset() - d := newDeployment() - a, err := clientset.AppsV1().Deployments("port-k8s-exporter").Update(context.Background(), d, metav1.UpdateOptions{}) - if err != nil { - return - } - log.Fatal(a) - -} - func TestUpdateHandler(t *testing.T) { oldDeployment := []runtime.Object{newUnstructured(newDeployment())}[0] newDep := []runtime.Object{newUnstructured(newDeploymentWithCustomLabels(map[string]string{"app": "port-k8s-exporter", "new": "label"}))}[0] @@ -419,4 +406,7 @@ func TestUpdateHandler(t *testing.T) { result = controllerWithPartialMapping.shouldSendUpdateEvent(oldDeployment, newDep, true) assert.False(t, result, "Expected false when objects are different but mapping not include the change and feature flag is on") + result = controllerWithPartialMapping.shouldSendUpdateEvent(oldDeployment, newDep, false) + assert.True(t, result, "Expected false when objects are different but mapping not include the change and feature flag is off") + } From a4c30db6b92adcf3e6223ca3aea8a502940bac94 Mon Sep 17 00:00:00 2001 From: razsamuel Date: Sun, 19 May 2024 17:06:05 +0300 Subject: [PATCH 4/7] Add cases --- pkg/k8s/controller_test.go | 141 ++++++++++++++++++++++--------------- 1 file changed, 86 insertions(+), 55 deletions(-) diff --git a/pkg/k8s/controller_test.go b/pkg/k8s/controller_test.go index af906fc..25f5c39 100644 --- a/pkg/k8s/controller_test.go +++ b/pkg/k8s/controller_test.go @@ -20,6 +20,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/dynamic/dynamicinformer" "k8s.io/client-go/tools/cache" @@ -103,11 +104,18 @@ func newDeployment() *appsv1.Deployment { } } -func newDeploymentWithCustomLabels(labels map[string]string) *appsv1.Deployment { +func newDeploymentWithCustomLabels(generation int64, + generateName string, + creationTimestamp v1.Time, + labels map[string]string, +) *appsv1.Deployment { return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "port-k8s-exporter", - Namespace: "port-k8s-exporter", + Name: "port-k8s-exporter", + Namespace: "port-k8s-exporter", + GenerateName: generateName, + Generation: generation, + CreationTimestamp: creationTimestamp, }, Spec: appsv1.DeploymentSpec{ Selector: &metav1.LabelSelector{ @@ -346,67 +354,90 @@ func TestGetEntitiesSet(t *testing.T) { f.runControllerGetEntitiesSet(expectedEntitiesSet, false) } -func TestUpdateHandler(t *testing.T) { - oldDeployment := []runtime.Object{newUnstructured(newDeployment())}[0] - newDep := []runtime.Object{newUnstructured(newDeploymentWithCustomLabels(map[string]string{"app": "port-k8s-exporter", "new": "label"}))}[0] - fullMapping := newResource("", []port.EntityMapping{ - { - Identifier: ".metadata.name", - Blueprint: "\"k8s-export-test-bp\"", - Icon: "\"Microservice\"", - Team: "\"Test\"", - Properties: map[string]string{ - "text": "\"pod\"", - "num": "1", - "bool": "true", - "obj": ".spec.selector", - "arr": ".spec.template.spec.containers", +func TestUpdateHandlerWithIndividualPropertyChanges(t *testing.T) { + + fullMapping := []port.Resource{ + newResource("", []port.EntityMapping{ + { + Identifier: ".metadata.name", + Blueprint: "\"k8s-export-test-bp\"", + Icon: "\"Microservice\"", + Team: "\"Test\"", + Properties: map[string]string{ + "labels": ".spec.selector", + "generation": ".metadata.generation", + "generateName": ".metadata.generateName", + "creationTimestamp": ".metadata.creationTimestamp", + }, + Relations: map[string]string{ + "k8s-relation": "\"e_AgPMYvq1tAs8TuqM\"", + }, }, - Relations: map[string]string{ - "k8s-relation": "\"e_AgPMYvq1tAs8TuqM\"", + }), + newResource("", []port.EntityMapping{ + + { + Identifier: ".metadata.name", + Blueprint: "\"k8s-export-test-bp\"", + Icon: "\"Microservice\"", + Team: "\"Test\"", + Properties: map[string]string{}, + Relations: map[string]string{}, }, - }, - }) - - partialMappingWithoutLables := newResource("", []port.EntityMapping{ - { - Identifier: ".metadata.name", - Blueprint: "\"k8s-export-test-bp\"", - Icon: "\"Microservice\"", - Team: "\"Test\"", - Properties: map[string]string{ - "text": "\"pod\"", - "num": "1", - "bool": "true", - }, - Relations: map[string]string{ - "k8s-relation": "\"e_AgPMYvq1tAs8TuqM\"", + { + Identifier: ".metadata.name", + Blueprint: "\"k8s-export-test-bp\"", + Icon: "\"Microservice\"", + Team: "\"Test\"", + Properties: map[string]string{ + "labels": ".spec.selector", + "generation": ".metadata.generation", + "generateName": ".metadata.generateName", + "creationTimestamp": ".metadata.creationTimestamp", + }, + Relations: map[string]string{ + "k8s-relation": "\"e_AgPMYvq1tAs8TuqM\"", + }, }, - }, - }) + }), + } - controllerWithFullMapping := newFixture(t, "", "", "", fullMapping, []runtime.Object{}).controller + for _, mapping := range fullMapping { - // Same object and feature flag on - should return false - result := controllerWithFullMapping.shouldSendUpdateEvent(oldDeployment, oldDeployment, true) - assert.False(t, result, "Expected false when same object and feature flag is on") + controllerWithFullMapping := newFixture(t, "", "", "", mapping, []runtime.Object{}).controller - // Different object and feature flag on - should return true - result = controllerWithFullMapping.shouldSendUpdateEvent(oldDeployment, newDep, true) - assert.True(t, result, "Expected true when different object and feature flag is on") + // Test changes in each individual property + properties := map[string]interface{}{ + "metadata.generation": int64(3), + "metadata.generateName": "new-port-k8s-exporter2", + "metadata.creationTimestamp": v1.Now().Add(1 * time.Hour).Format(time.RFC3339), + } - // feature flag off - should return true regardless of whether objects are same or different - result = controllerWithFullMapping.shouldSendUpdateEvent(oldDeployment, oldDeployment, false) - assert.True(t, result, "Expected true when same object and feature flag is off") + dep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new-port-k8s-exporter"})) + result := controllerWithFullMapping.shouldSendUpdateEvent(dep, dep, true) + assert.False(t, result, "Expected false when no changes and feature flag is on") + result = controllerWithFullMapping.shouldSendUpdateEvent(dep, dep, false) + assert.True(t, result, "Expected true when no changes and feature flag is off") - result = controllerWithFullMapping.shouldSendUpdateEvent(oldDeployment, newDep, false) - assert.True(t, result, "Expected true when different object and feature flag is off") + for property, value := range properties { + newDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new-port-k8s-exporter"})) + oldDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new-port-k8s-exporter"})) - controllerWithPartialMapping := newFixture(t, "", "", "", partialMappingWithoutLables, []runtime.Object{}).controller - result = controllerWithPartialMapping.shouldSendUpdateEvent(oldDeployment, newDep, true) - assert.False(t, result, "Expected false when objects are different but mapping not include the change and feature flag is on") + // Update the property in the new deployment + unstructured.SetNestedField(newDep.Object, value, strings.Split(property, ".")...) - result = controllerWithPartialMapping.shouldSendUpdateEvent(oldDeployment, newDep, false) - assert.True(t, result, "Expected false when objects are different but mapping not include the change and feature flag is off") + result := controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, true) + assert.True(t, result, fmt.Sprintf("Expected true when %s changes and feature flag is on", property)) + result = controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, false) + assert.True(t, result, fmt.Sprintf("Expected true when %s changes and feature flag is off", property)) + } + oldDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new-port-k8s-exporter"})) + newDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new label"})) + + result = controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, true) + assert.True(t, result, "Expected true when labels changes and feature flag is on") + result = controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, false) + assert.True(t, result, "Expected true when labels changes and feature flag is off") + } } From 6251f12cda8dd9b51ef3b064871759e9efc3fa51 Mon Sep 17 00:00:00 2001 From: razsamuel Date: Sun, 19 May 2024 17:22:39 +0300 Subject: [PATCH 5/7] generic --- pkg/k8s/controller_test.go | 42 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/pkg/k8s/controller_test.go b/pkg/k8s/controller_test.go index 25f5c39..2694b5d 100644 --- a/pkg/k8s/controller_test.go +++ b/pkg/k8s/controller_test.go @@ -356,6 +356,11 @@ func TestGetEntitiesSet(t *testing.T) { func TestUpdateHandlerWithIndividualPropertyChanges(t *testing.T) { + type Property struct { + Value interface{} + ShouldSendEvent bool + } + fullMapping := []port.Resource{ newResource("", []port.EntityMapping{ { @@ -407,37 +412,38 @@ func TestUpdateHandlerWithIndividualPropertyChanges(t *testing.T) { controllerWithFullMapping := newFixture(t, "", "", "", mapping, []runtime.Object{}).controller // Test changes in each individual property - properties := map[string]interface{}{ - "metadata.generation": int64(3), - "metadata.generateName": "new-port-k8s-exporter2", - "metadata.creationTimestamp": v1.Now().Add(1 * time.Hour).Format(time.RFC3339), + properties := map[string]Property{ + "metadata.name": {Value: "port-k8s-exporter", ShouldSendEvent: false}, + "something_without_mapping": {Value: "port-k8s-exporter", ShouldSendEvent: false}, + "metadata.generation": {Value: int64(3), ShouldSendEvent: true}, + "metadata.generateName": {Value: "new-port-k8s-exporter2", ShouldSendEvent: true}, + "metadata.creationTimestamp": {Value: v1.Now().Add(1 * time.Hour).Format(time.RFC3339), ShouldSendEvent: true}, } - dep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new-port-k8s-exporter"})) - result := controllerWithFullMapping.shouldSendUpdateEvent(dep, dep, true) - assert.False(t, result, "Expected false when no changes and feature flag is on") - result = controllerWithFullMapping.shouldSendUpdateEvent(dep, dep, false) - assert.True(t, result, "Expected true when no changes and feature flag is off") - for property, value := range properties { - newDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new-port-k8s-exporter"})) - oldDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new-port-k8s-exporter"})) + newDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "port-k8s-exporter"})) + oldDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "port-k8s-exporter"})) // Update the property in the new deployment - unstructured.SetNestedField(newDep.Object, value, strings.Split(property, ".")...) + unstructured.SetNestedField(newDep.Object, value.Value, strings.Split(property, ".")...) result := controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, true) - assert.True(t, result, fmt.Sprintf("Expected true when %s changes and feature flag is on", property)) + if value.ShouldSendEvent { + assert.True(t, result, fmt.Sprintf("Expected true when %s changes and feature flag is on", property)) + } else { + assert.False(t, result, fmt.Sprintf("Expected false when %s changes and feature flag is on", property)) + } result = controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, false) assert.True(t, result, fmt.Sprintf("Expected true when %s changes and feature flag is off", property)) } + // Add a case for json update because you can't edit the json directly + newDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "port-k8s-exporter"})) oldDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new-port-k8s-exporter"})) - newDep := newUnstructured(newDeploymentWithCustomLabels(2, "new-port-k8s-exporter", v1.Now(), map[string]string{"app": "new label"})) - result = controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, true) - assert.True(t, result, "Expected true when labels changes and feature flag is on") + result := controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, true) + assert.True(t, result, fmt.Sprintf("Expected true when labels changes and feature flag is on")) result = controllerWithFullMapping.shouldSendUpdateEvent(oldDep, newDep, false) - assert.True(t, result, "Expected true when labels changes and feature flag is off") + assert.True(t, result, fmt.Sprintf("Expected true when labels changes and feature flag is off")) } } From fc84cfeda5634aaa6dafcab4ab284dbcd848e431 Mon Sep 17 00:00:00 2001 From: razsamuel Date: Mon, 20 May 2024 10:46:19 +0300 Subject: [PATCH 6/7] fix that in case of only one resource config that get change we will also send event --- pkg/config/config.go | 2 +- pkg/k8s/controller.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 7e78a2d..4f2d7a6 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -39,7 +39,7 @@ func Init() { NewString(&ApplicationConfig.PortClientSecret, "port-client-secret", "", "Port client secret. Required.") NewBool(&ApplicationConfig.CreateDefaultResources, "create-default-resources", true, "Create default resources on installation. Optional.") NewBool(&ApplicationConfig.OverwriteConfigurationOnRestart, "overwrite-configuration-on-restart", false, "Overwrite the configuration in port on restarting the exporter. Optional.") - NewBool(&ApplicationConfig.UpdateEntityOnlyOnDiff, "update-entity-only-on-diff", false, "Optimization to reduce requests to port. Optional.") + NewBool(&ApplicationConfig.UpdateEntityOnlyOnDiff, "update-entity-only-on-diff", true, "Optimization to reduce requests to port. Optional.") // Deprecated NewBool(&ApplicationConfig.DeleteDependents, "delete-dependents", false, "Delete dependents. Optional.") diff --git a/pkg/k8s/controller.go b/pkg/k8s/controller.go index 712f213..1284021 100644 --- a/pkg/k8s/controller.go +++ b/pkg/k8s/controller.go @@ -433,10 +433,10 @@ func (c *Controller) shouldSendUpdateEvent(old interface{}, new interface{}, upd return true } - if oldEntitiesHash == newEntitiesHash { - return false + if oldEntitiesHash != newEntitiesHash { + return true } } - return true + return false } From afcafdfcdfa99e9d1b35e5ace96d64bd42e4c404 Mon Sep 17 00:00:00 2001 From: razsamuel Date: Mon, 20 May 2024 12:13:11 +0300 Subject: [PATCH 7/7] default key to false --- pkg/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 4f2d7a6..7e78a2d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -39,7 +39,7 @@ func Init() { NewString(&ApplicationConfig.PortClientSecret, "port-client-secret", "", "Port client secret. Required.") NewBool(&ApplicationConfig.CreateDefaultResources, "create-default-resources", true, "Create default resources on installation. Optional.") NewBool(&ApplicationConfig.OverwriteConfigurationOnRestart, "overwrite-configuration-on-restart", false, "Overwrite the configuration in port on restarting the exporter. Optional.") - NewBool(&ApplicationConfig.UpdateEntityOnlyOnDiff, "update-entity-only-on-diff", true, "Optimization to reduce requests to port. Optional.") + NewBool(&ApplicationConfig.UpdateEntityOnlyOnDiff, "update-entity-only-on-diff", false, "Optimization to reduce requests to port. Optional.") // Deprecated NewBool(&ApplicationConfig.DeleteDependents, "delete-dependents", false, "Delete dependents. Optional.")