From 30f061d44c04a139ec08ab837c61b9bbcab99d29 Mon Sep 17 00:00:00 2001 From: linkas45 Date: Thu, 19 Dec 2024 09:15:07 +0000 Subject: [PATCH 1/2] feat: Remove ReplicaSet 0 replicas filter --- cmd/dump/run.go | 2 +- internal/services/controller/controller.go | 33 ++++----------- .../services/controller/controller_test.go | 41 +------------------ internal/services/controller/worker.go | 1 - 4 files changed, 11 insertions(+), 66 deletions(-) diff --git a/cmd/dump/run.go b/cmd/dump/run.go index 7594a09f..d6926a05 100644 --- a/cmd/dump/run.go +++ b/cmd/dump/run.go @@ -66,7 +66,7 @@ func run(ctx context.Context) error { log = log.WithField("k8s_version", v.Full()) - delta, err := controller.CollectSingleSnapshot(ctx, log, clusterID, clientset, dynamicClient, metricsClient, cfg.Controller, v, "") + delta, err := controller.CollectSingleSnapshot(ctx, log, clusterID, clientset, dynamicClient, metricsClient, cfg.Controller, v) if err != nil { return err } diff --git a/internal/services/controller/controller.go b/internal/services/controller/controller.go index b2f96331..84395c1d 100644 --- a/internal/services/controller/controller.go +++ b/internal/services/controller/controller.go @@ -119,12 +119,11 @@ func CollectSingleSnapshot(ctx context.Context, metricsClient versioned.Interface, cfg *config.Controller, v version.Interface, - castwareNamespace string, ) (*castai.Delta, error) { f := informers.NewSharedInformerFactory(clientset, 0) df := dynamicinformer.NewDynamicSharedInformerFactory(dynamicClient, 0) - defaultInformers := getDefaultInformers(f, castwareNamespace) + defaultInformers := getDefaultInformers(f) conditionalInformers := getConditionalInformers(clientset, cfg, f, df, metricsClient, log) additionalTransformers := createAdditionalTransformers(cfg) @@ -199,7 +198,6 @@ func New( agentVersion *config.AgentVersion, healthzProvider *HealthzProvider, selfSubjectAccessReview authorizationtypev1.SelfSubjectAccessReviewInterface, - castwareNamespace string, ) *Controller { healthzProvider.Initializing() @@ -210,7 +208,7 @@ func New( df := dynamicinformer.NewDynamicSharedInformerFactory(dynamicClient, defaultResync) discovery := clientset.Discovery() - defaultInformers := getDefaultInformers(f, castwareNamespace) + defaultInformers := getDefaultInformers(f) conditionalInformers := getConditionalInformers(clientset, cfg, f, df, metricsClient, log) additionalTransformers := createAdditionalTransformers(cfg) @@ -935,7 +933,7 @@ type defaultInformer struct { filters filters.Filters } -func getDefaultInformers(f informers.SharedInformerFactory, castwareNamespace string) map[reflect.Type]defaultInformer { +func getDefaultInformers(f informers.SharedInformerFactory) map[reflect.Type]defaultInformer { return map[reflect.Type]defaultInformer{ reflect.TypeOf(&corev1.Node{}): {informer: f.Core().V1().Nodes().Informer()}, reflect.TypeOf(&corev1.Pod{}): {informer: f.Core().V1().Pods().Informer()}, @@ -944,26 +942,11 @@ func getDefaultInformers(f informers.SharedInformerFactory, castwareNamespace st reflect.TypeOf(&corev1.ReplicationController{}): {informer: f.Core().V1().ReplicationControllers().Informer()}, reflect.TypeOf(&corev1.Namespace{}): {informer: f.Core().V1().Namespaces().Informer()}, reflect.TypeOf(&appsv1.Deployment{}): {informer: f.Apps().V1().Deployments().Informer()}, - reflect.TypeOf(&appsv1.ReplicaSet{}): { - informer: f.Apps().V1().ReplicaSets().Informer(), - filters: filters.Filters{ - { - func(e castai.EventType, obj interface{}) bool { - replicaSet, ok := obj.(*appsv1.ReplicaSet) - if !ok { - return false - } - - return e == castai.EventDelete || replicaSet.Namespace == castwareNamespace || - (replicaSet.Spec.Replicas != nil && *replicaSet.Spec.Replicas > 0 && replicaSet.Status.Replicas > 0) - }, - }, - }, - }, - reflect.TypeOf(&appsv1.DaemonSet{}): {informer: f.Apps().V1().DaemonSets().Informer()}, - reflect.TypeOf(&appsv1.StatefulSet{}): {informer: f.Apps().V1().StatefulSets().Informer()}, - reflect.TypeOf(&storagev1.StorageClass{}): {informer: f.Storage().V1().StorageClasses().Informer()}, - reflect.TypeOf(&batchv1.Job{}): {informer: f.Batch().V1().Jobs().Informer()}, + reflect.TypeOf(&appsv1.ReplicaSet{}): {informer: f.Apps().V1().ReplicaSets().Informer()}, + reflect.TypeOf(&appsv1.DaemonSet{}): {informer: f.Apps().V1().DaemonSets().Informer()}, + reflect.TypeOf(&appsv1.StatefulSet{}): {informer: f.Apps().V1().StatefulSets().Informer()}, + reflect.TypeOf(&storagev1.StorageClass{}): {informer: f.Storage().V1().StorageClasses().Informer()}, + reflect.TypeOf(&batchv1.Job{}): {informer: f.Batch().V1().Jobs().Informer()}, reflect.TypeOf(&corev1.Service{}): { informer: f.Core().V1().Services().Informer(), filters: filters.Filters{ diff --git a/internal/services/controller/controller_test.go b/internal/services/controller/controller_test.go index fe2512c0..775cce01 100644 --- a/internal/services/controller/controller_test.go +++ b/internal/services/controller/controller_test.go @@ -220,7 +220,6 @@ func TestController_ShouldReceiveDeltasBasedOnAvailableResources(t *testing.T) { agentVersion, NewHealthzProvider(defaultHealthzCfg, log), fakeSelfSubjectAccessReviewsClient, - "castai-agent", ) if mockDiscovery != nil { @@ -382,7 +381,6 @@ func TestController_ShouldSendByInterval(t *testing.T) { agentVersion, NewHealthzProvider(defaultHealthzCfg, log), clientset.AuthorizationV1().SelfSubjectAccessReviews(), - "castai-agent", ) ctrl.Start(ctx.Done()) @@ -531,7 +529,6 @@ func TestController_ShouldKeepDeltaAfterDelete(t *testing.T) { agentVersion, NewHealthzProvider(defaultHealthzCfg, log), clientset.AuthorizationV1().SelfSubjectAccessReviews(), - "castai-agent", ) ctrl.Start(ctx.Done()) @@ -1188,16 +1185,7 @@ func TestDefaultInformers_MatchFilters(t *testing.T) { eventType castai.EventType expectedMatch bool }{ - "keep if replicaset in castware namespace": { - obj: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "castware", - }, - }, - eventType: castai.EventAdd, - expectedMatch: true, - }, - "discard if replicaset has zero replicas": { + "dont discard if replicaset has zero replicas": { obj: &appsv1.ReplicaSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", @@ -1210,30 +1198,6 @@ func TestDefaultInformers_MatchFilters(t *testing.T) { }, }, eventType: castai.EventAdd, - expectedMatch: false, - }, - "keep if replicaset has more than zero replicas": { - obj: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: appsv1.ReplicaSetSpec{ - Replicas: lo.ToPtr(int32(1)), - }, - Status: appsv1.ReplicaSetStatus{ - Replicas: 1, - }, - }, - eventType: castai.EventAdd, - expectedMatch: true, - }, - "keep if delete event": { - obj: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - }, - eventType: castai.EventDelete, expectedMatch: true, }, } @@ -1243,7 +1207,7 @@ func TestDefaultInformers_MatchFilters(t *testing.T) { r := require.New(t) f := informers.NewSharedInformerFactory(fake.NewSimpleClientset(data.obj), 0) - defaultInformers := getDefaultInformers(f, "castware") + defaultInformers := getDefaultInformers(f) objInformer := defaultInformers[reflect.TypeOf(data.obj)] match := objInformer.filters.Apply(data.eventType, data.obj) @@ -1294,7 +1258,6 @@ func TestCollectSingleSnapshot(t *testing.T) { PrepTimeout: 10 * time.Second, }, version, - "", ) r.NoError(err) r.NotNil(snapshot) diff --git a/internal/services/controller/worker.go b/internal/services/controller/worker.go index b15498a6..e8359620 100644 --- a/internal/services/controller/worker.go +++ b/internal/services/controller/worker.go @@ -60,7 +60,6 @@ func Loop( agentVersion, healthzProvider, clientset.AuthorizationV1().SelfSubjectAccessReviews(), - cfg.SelfPod.Namespace, ) ctrl.Start(ctrlCtx.Done()) From 96f356eaa1c3f48ac41aeb912361d4a4c36fe81a Mon Sep 17 00:00:00 2001 From: linkas45 Date: Thu, 19 Dec 2024 09:32:39 +0000 Subject: [PATCH 2/2] remove test --- .../services/controller/controller_test.go | 41 ------------------- 1 file changed, 41 deletions(-) diff --git a/internal/services/controller/controller_test.go b/internal/services/controller/controller_test.go index 775cce01..b267c493 100644 --- a/internal/services/controller/controller_test.go +++ b/internal/services/controller/controller_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "reflect" "strings" "sync" "sync/atomic" @@ -19,7 +18,6 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" "go.uber.org/goleak" - appsv1 "k8s.io/api/apps/v1" authorizationv1 "k8s.io/api/authorization/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" v1 "k8s.io/api/core/v1" @@ -34,7 +32,6 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" dynamic_fake "k8s.io/client-go/dynamic/fake" - "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" authfakev1 "k8s.io/client-go/kubernetes/typed/authorization/v1/fake" k8stesting "k8s.io/client-go/testing" @@ -1179,44 +1176,6 @@ func loadInitialHappyPathData(t *testing.T, scheme *runtime.Scheme) ([]sampleObj return objects, clientset, dynamicClient, metricsClient } -func TestDefaultInformers_MatchFilters(t *testing.T) { - tests := map[string]struct { - obj runtime.Object - eventType castai.EventType - expectedMatch bool - }{ - "dont discard if replicaset has zero replicas": { - obj: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - }, - Spec: appsv1.ReplicaSetSpec{ - Replicas: lo.ToPtr(int32(0)), - }, - Status: appsv1.ReplicaSetStatus{ - Replicas: 0, - }, - }, - eventType: castai.EventAdd, - expectedMatch: true, - }, - } - - for name, data := range tests { - t.Run(name, func(t *testing.T) { - r := require.New(t) - f := informers.NewSharedInformerFactory(fake.NewSimpleClientset(data.obj), 0) - - defaultInformers := getDefaultInformers(f) - objInformer := defaultInformers[reflect.TypeOf(data.obj)] - - match := objInformer.filters.Apply(data.eventType, data.obj) - - r.Equal(data.expectedMatch, match) - }) - } -} - func TestCollectSingleSnapshot(t *testing.T) { r := require.New(t)