From ee4539e62f3d8b20caad303233bbf3e9c88394f3 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Tue, 24 May 2022 08:56:03 +0200 Subject: [PATCH 01/18] feat: add namespaceSelector config --- internal/config/config.go | 49 ++++++++++++++++++++--------- internal/config/config_test.go | 9 ++++++ internal/config/testdata/config.yml | 4 +++ 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index b5b845ed0..10f19f5ff 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -53,6 +53,9 @@ type Config struct { Kubelet `mapstructure:"kubelet"` // KSM defines config options for the kube-state-metrics scraper. KSM `mapstructure:"ksm"` + + // NamespaceSelector defines custom monitoring filtering for namespaces. + NamespaceSelector *NamespaceSelector `mapstructure:"namespaceSelector"` } // HTTPSink stores the configuration for the HTTP sink. @@ -218,38 +221,54 @@ type MTLS struct { TLSSecretNamespace string `mapstructure:"secretNamespace"` } +// NamespaceSelector contains config options for filtering namespaces. +type NamespaceSelector struct { + // MatchLabels is a list of labels to filter namespaces with. + MatchLabels map[string]string `mapstructure:"matchLabels"` + // MatchExpressions is a list of namespaces selector requirements. + MatchExpressions []Expression `mapstructure:"matchExpressions"` +} + +type Expression struct { + Key string `mapstructure:"key" json:"key"` + Operator string `mapstructure:"operator" json:"operator"` + Values []interface{} `mapstructure:"values" json:"values"` +} + func LoadConfig(filePath string, fileName string) (*Config, error) { - v := viper.New() + // Update default delimiter as with the new namespaceSelector config, some labels may come in the form of + // newrelic.com/scrape, so the key was split in a sub-map on a "." basis. + v := viper.NewWithOptions(viper.KeyDelimiter("|")) // We need to assure that defaults have been set in order to bind env variables. // https://github.com/spf13/viper/issues/584 v.SetDefault("clusterName", "cluster") v.SetDefault("verbose", false) - v.SetDefault("kubelet.networkRouteFile", DefaultNetworkRouteFile) + v.SetDefault("kubelet|networkRouteFile", DefaultNetworkRouteFile) v.SetDefault("nodeName", "node") v.SetDefault("nodeIP", "node") // Sane connection defaults - v.SetDefault("sink.type", SinkTypeHTTP) - v.SetDefault("sink.http.port", 0) - v.SetDefault("sink.http.timeout", DefaultAgentTimeout) - v.SetDefault("sink.http.retries", DefaultRetries) + v.SetDefault("sink|type", SinkTypeHTTP) + v.SetDefault("sink|http|port", 0) + v.SetDefault("sink|http|timeout", DefaultAgentTimeout) + v.SetDefault("sink|http|retries", DefaultRetries) - v.SetDefault("kubelet.timeout", DefaultTimeout) - v.SetDefault("kubelet.retries", DefaultRetries) + v.SetDefault("kubelet|timeout", DefaultTimeout) + v.SetDefault("kubelet|retries", DefaultRetries) - v.SetDefault("controlPlane.timeout", DefaultTimeout) - v.SetDefault("controlPlane.retries", DefaultRetries) + v.SetDefault("controlPlane|timeout", DefaultTimeout) + v.SetDefault("controlPlane|retries", DefaultRetries) - v.SetDefault("ksm.timeout", DefaultTimeout) - v.SetDefault("ksm.retries", DefaultRetries) + v.SetDefault("ksm|timeout", DefaultTimeout) + v.SetDefault("ksm|retries", DefaultRetries) - v.SetDefault("ksm.discovery.backoffDelay", 7*time.Second) - v.SetDefault("ksm.discovery.timeout", 60*time.Second) + v.SetDefault("ksm|discovery|backoffDelay", 7*time.Second) + v.SetDefault("ksm|discovery|timeout", 60*time.Second) v.SetEnvPrefix("NRI_KUBERNETES") v.AutomaticEnv() - v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + v.SetEnvKeyReplacer(strings.NewReplacer("|", "_")) // Config File v.AddConfigPath(filePath) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b8186791a..8e65da1c3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -32,6 +32,15 @@ func TestLoadConfig(t *testing.T) { require.Equal(t, "fake-node", c.NodeName) }) }) + // This test checks that viper custom key delimiter is working as expected by using the old default dot delimiter + // as key. + t.Run("succeeds_when_dot_character_in_key", func(t *testing.T) { + t.Parallel() + + c, err := config.LoadConfig(fakeDataDir, workingData) + require.NoError(t, err) + require.Equal(t, "1", c.NamespaceSelector.MatchLabels["newrelic.com/scrape"]) + }) t.Run("fail_due_to_unexpected_data", func(t *testing.T) { t.Parallel() diff --git a/internal/config/testdata/config.yml b/internal/config/testdata/config.yml index 06a7c935d..1e6f0e24a 100644 --- a/internal/config/testdata/config.yml +++ b/internal/config/testdata/config.yml @@ -2,6 +2,10 @@ clusterName: dummy_cluster interval: 15 verbose: true +namespaceSelector: + matchLabels: + newrelic.com/scrape: true + sink: http: port: 8081 From d604f074f41ae0430f5714a1749ad40a6d710d66 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Mon, 30 May 2022 20:48:01 +0200 Subject: [PATCH 02/18] feat update values.yaml with namespaceSelector --- charts/newrelic-infrastructure/values.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/charts/newrelic-infrastructure/values.yaml b/charts/newrelic-infrastructure/values.yaml index 1b8d01e50..040bca643 100644 --- a/charts/newrelic-infrastructure/values.yaml +++ b/charts/newrelic-infrastructure/values.yaml @@ -49,6 +49,9 @@ common: # behave properly. Any non-nil value will override the `lowDataMode` default. # @default -- `15s` (See [Low data mode](README.md#low-data-mode)) interval: + # -- Config for filtering ksm and kubelet metrics by namespace. + namespaceSelector: {} + # -- Config for the Infrastructure agent. # Will be used by the forwarder sidecars and the agent running integrations. # See: https://docs.newrelic.com/docs/infrastructure/install-infrastructure-agent/configuration/infrastructure-agent-configuration-settings/ From c995fa84b5f3ab2c55cb15b2687ddb1172816f92 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Mon, 30 May 2022 22:56:53 +0200 Subject: [PATCH 03/18] feat: add namespace filterer --- internal/config/config.go | 38 +++- internal/discovery/namespace_filter.go | 94 +++++++++ internal/discovery/namespace_filter_test.go | 200 ++++++++++++++++++++ 3 files changed, 329 insertions(+), 3 deletions(-) create mode 100644 internal/discovery/namespace_filter.go create mode 100644 internal/discovery/namespace_filter_test.go diff --git a/internal/config/config.go b/internal/config/config.go index 10f19f5ff..4d734dc58 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,9 +1,12 @@ package config import ( + "fmt" + "strconv" "strings" "time" + log "github.com/sirupsen/logrus" "github.com/spf13/viper" ) @@ -229,10 +232,39 @@ type NamespaceSelector struct { MatchExpressions []Expression `mapstructure:"matchExpressions"` } +// Expression hold the values to generate an expression to filter namespaces by MatchExpressions. type Expression struct { - Key string `mapstructure:"key" json:"key"` - Operator string `mapstructure:"operator" json:"operator"` - Values []interface{} `mapstructure:"values" json:"values"` + // Key it the key of the label. + Key string `mapstructure:"key" json:"key"` + // Operator holds either an inclusion (NotIn) or exclusion (In) value. + Operator string `mapstructure:"operator" json:"operator"` + // Values is a slice of values related to the key. + Values []interface{} `mapstructure:"values" json:"values"` +} + +func (e *Expression) String() string { + var values []string + + for _, val := range e.Values { + var str string + switch v := val.(type) { + case string: + str = v + case bool: + str = strconv.FormatBool(v) + case int: + str = strconv.FormatInt(int64(v), 10) + case float32, float64: + str = fmt.Sprintf("%f", v) + default: + log.Errorf("parsing expression value: %v, type %v", val, v) + continue + } + + values = append(values, str) + } + + return fmt.Sprintf("%s %s (%s)", e.Key, strings.ToLower(e.Operator), strings.Join(values, ",")) } func LoadConfig(filePath string, fileName string) (*Config, error) { diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go new file mode 100644 index 000000000..2eb243112 --- /dev/null +++ b/internal/discovery/namespace_filter.go @@ -0,0 +1,94 @@ +package discovery + +import ( + "github.com/newrelic/nri-kubernetes/v3/internal/config" + + log "github.com/sirupsen/logrus" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" + listersv1 "k8s.io/client-go/listers/core/v1" +) + +// NamespaceFilterer provides an interface to filter namespaces. +type NamespaceFilterer interface { + IsAllowed(namespace string) bool +} + +// NamespaceFilter is a struct holding pointers to the config and the namespace lister. +type NamespaceFilter struct { + c *config.Config + lister listersv1.NamespaceLister +} + +// NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter. +func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, options ...informers.SharedInformerOption) (*NamespaceFilter, chan<- struct{}) { + stopCh := make(chan struct{}) + + factory := informers.NewSharedInformerFactoryWithOptions(client, defaultResyncDuration, options...) + + lister := factory.Core().V1().Namespaces().Lister() + + factory.Start(stopCh) + factory.WaitForCacheSync(stopCh) + + return &NamespaceFilter{ + c: c, + lister: lister, + }, stopCh +} + +// IsAllowed checks given any namespace, if it's allowed to be scraped by using the NamespaceLister +func (nf *NamespaceFilter) IsAllowed(namespace string) bool { + // By default, we scrape every namespace. + if nf.c.NamespaceSelector == nil { + return true + } + + // Scrape namespaces by honoring the matchLabels values. + if nf.c.NamespaceSelector.MatchLabels != nil { + namespaceList, err := nf.lister.List(labels.SelectorFromSet(nf.c.NamespaceSelector.MatchLabels)) + if err != nil { + log.Errorf("listing namespaces with MatchLabels: %v", err) + return false + } + + return containsNamespace(namespace, namespaceList) + } + + // Scrape namespaces by honoring the matchExpressions values. + // Multiple expressions are evaluated with a logical AND between them. + if nf.c.NamespaceSelector.MatchExpressions != nil { + for _, expression := range nf.c.NamespaceSelector.MatchExpressions { + selector, err := labels.Parse(expression.String()) + if err != nil { + log.Errorf("parsing labels: %v", err) + return false + } + + namespaceList, err := nf.lister.List(selector) + if err != nil { + log.Errorf("listing namespaces with MatchExpressions: %v", err) + return false + } + + if !containsNamespace(namespace, namespaceList) { + return false + } + } + } + + return true +} + +// containsNamespace checks if a namespaces is contained in a given list of namespaces. +func containsNamespace(namespace string, namespaceList []*v1.Namespace) bool { + for _, n := range namespaceList { + if n.Name == namespace { + return true + } + } + + return false +} diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go new file mode 100644 index 000000000..68f02893f --- /dev/null +++ b/internal/discovery/namespace_filter_test.go @@ -0,0 +1,200 @@ +package discovery_test + +import ( + "context" + "testing" + "time" + + "github.com/newrelic/nri-kubernetes/v3/internal/config" + "github.com/newrelic/nri-kubernetes/v3/internal/discovery" + "github.com/stretchr/testify/require" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/wait" + testclient "k8s.io/client-go/kubernetes/fake" +) + +const namespaceName = "test_namespace" + +func TestNamespaceFilterer_IsAllowed(t *testing.T) { + t.Parallel() + + type testData struct { + namespaceLabels labels.Set + namespaceSelector config.NamespaceSelector + expected bool + } + + testCases := map[string]testData{ + "namespace_allowed_by_default": { + expected: true, + }, + "match_labels_included_namespace_allowed": { + namespaceLabels: labels.Set{ + "newrelic.com/scrape": "true", + "ohhh": "xxx", + }, + namespaceSelector: config.NamespaceSelector{ + MatchLabels: map[string]string{ + "newrelic.com/scrape": "true", + "ohhh": "xxx", + }, + }, + expected: true, + }, + "match_labels_excluded_namespaces_not_allowed": { + namespaceLabels: labels.Set{"newrelic.com/scrape": "false"}, + namespaceSelector: config.NamespaceSelector{ + MatchLabels: map[string]string{ + "newrelic.com/scrape": "true", + }, + }, + expected: false, + }, + "match_expressions_using_not_in_operator_allow_not_included_namespaces": { + namespaceLabels: labels.Set{"newrelic.com/scrape": "true"}, + namespaceSelector: config.NamespaceSelector{ + MatchExpressions: []config.Expression{ + { + Key: "newrelic.com/scrape", + Operator: "NotIn", + Values: []interface{}{false}, + }, + }, + }, + expected: true, + }, + "match_expressions_using_not_in_operator_not_allow_included_namespaces": { + namespaceLabels: labels.Set{"newrelic.com/scrape": "true"}, + namespaceSelector: config.NamespaceSelector{ + MatchExpressions: []config.Expression{ + { + Key: "newrelic.com/scrape", + Operator: "NotIn", + Values: []interface{}{true}, + }, + }, + }, + expected: false, + }, + "match_expressions_using_in_operator_allow_included_namespaces": { + namespaceLabels: labels.Set{"newrelic.com/scrape": "true"}, + namespaceSelector: config.NamespaceSelector{ + MatchExpressions: []config.Expression{ + { + Key: "newrelic.com/scrape", + Operator: "In", + Values: []interface{}{true}, + }, + }, + }, + expected: true, + }, + "match_expressions_using_in_operator_not_allow_excluded_namespaces": { + namespaceLabels: labels.Set{"newrelic.com/scrape": "false"}, + namespaceSelector: config.NamespaceSelector{ + MatchExpressions: []config.Expression{ + { + Key: "newrelic.com/scrape", + Operator: "In", + Values: []interface{}{"true"}, + }, + }, + }, + expected: false, + }, + "match_expressions_using_multiple_expressions_allow_included_namespaces": { + namespaceLabels: labels.Set{"test_label": "1234"}, + namespaceSelector: config.NamespaceSelector{ + MatchExpressions: []config.Expression{ + { + Key: "newrelic.com/scrape", + Operator: "NotIn", + Values: []interface{}{"false"}, + }, + { + Key: "test_label", + Operator: "In", + Values: []interface{}{1234}, + }, + }, + }, + expected: true, + }, + } + + for testName, testData := range testCases { + testData := testData + t.Run(testName, func(t *testing.T) { + t.Parallel() + + client := testclient.NewSimpleClientset() + _, err := client.CoreV1().Namespaces().Create( + context.Background(), + fakeNamespaceWithNameAndLabels(namespaceName, testData.namespaceLabels), + metav1.CreateOptions{}, + ) + require.NoError(t, err) + c := config.Config{NamespaceSelector: &testData.namespaceSelector} + + nsFilter, _ := discovery.NewNamespaceFilter(&c, client) + + require.Equal(t, testData.expected, nsFilter.IsAllowed(namespaceName)) + }) + } +} + +func TestNamespaceFilter_CacheSync(t *testing.T) { + t.Parallel() + + anotherNamespaceName := "another_namespace" + client := testclient.NewSimpleClientset() + + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(5*time.Second)) + t.Cleanup(cancel) + + // Create a namespace with a specific label. + _, err := client.CoreV1().Namespaces().Create( + ctx, + fakeNamespaceWithNameAndLabels(namespaceName, labels.Set{"test_label": "1234"}), + metav1.CreateOptions{}, + ) + require.NoError(t, err) + + // Create the namespace filter. + nsFilter, _ := discovery.NewNamespaceFilter(&config.Config{ + NamespaceSelector: &config.NamespaceSelector{ + MatchLabels: map[string]string{ + "test_label": "123", + }, + }, + }, client) + // Check that recently created namespace is not allowed. + require.Equal(t, false, nsFilter.IsAllowed(namespaceName)) + + // Create a new namespace that can be filtered with the previous given config. + _, err = client.CoreV1().Namespaces().Create( + ctx, + fakeNamespaceWithNameAndLabels(anotherNamespaceName, labels.Set{"test_label": "123"}), + metav1.CreateOptions{}, + ) + require.NoError(t, err) + + // Give some room to the informer to sync, and check that the new namespace is filtered properly. + if err := wait.PollImmediateUntilWithContext(ctx, 1*time.Second, func(context.Context) (bool, error) { + return nsFilter.IsAllowed(anotherNamespaceName), nil + }); err != nil { + t.Fatalf("Timed out waiting for the informer to sync: %v", err) + } +} + +func fakeNamespaceWithNameAndLabels(name string, l labels.Set) *corev1.Namespace { + return &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: l, + }, + } +} From 2f03281a3cc330515c6bde0a8379424301ca982c Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Tue, 31 May 2022 07:48:44 +0200 Subject: [PATCH 04/18] chore: bump chart to v.3.5.3 --- charts/newrelic-infrastructure/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/newrelic-infrastructure/Chart.yaml b/charts/newrelic-infrastructure/Chart.yaml index ab4ffd9af..a4f7976eb 100644 --- a/charts/newrelic-infrastructure/Chart.yaml +++ b/charts/newrelic-infrastructure/Chart.yaml @@ -8,7 +8,7 @@ sources: - https://github.com/newrelic/nri-kubernetes/tree/master/charts/newrelic-infrastructure - https://github.com/newrelic/infrastructure-agent/ -version: 3.5.2 +version: 3.5.3 appVersion: 3.2.0 dependencies: From ad591014a660dc3bf895c1bb9bb8a803bbe8e5a4 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Tue, 31 May 2022 08:11:34 +0200 Subject: [PATCH 05/18] test: update helm chart tests --- .../tests/configmap_cp_scraper_test.yaml | 4 ++++ .../tests/configmap_ksm_scraper_test.yaml | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/charts/newrelic-infrastructure/tests/configmap_cp_scraper_test.yaml b/charts/newrelic-infrastructure/tests/configmap_cp_scraper_test.yaml index 43337212e..073710145 100644 --- a/charts/newrelic-infrastructure/tests/configmap_cp_scraper_test.yaml +++ b/charts/newrelic-infrastructure/tests/configmap_cp_scraper_test.yaml @@ -15,6 +15,7 @@ tests: path: data.nri-kubernetes\.yml value: |- interval: 15s + namespaceSelector: {} controlPlane: retries: 3 timeout: 10s @@ -38,6 +39,7 @@ tests: path: data.nri-kubernetes\.yml value: |- interval: 15s + namespaceSelector: {} controlPlane: retries: 3 timeout: 10s @@ -71,6 +73,7 @@ tests: path: data.nri-kubernetes\.yml value: |- interval: 15s + namespaceSelector: {} controlPlane: retries: 3 timeout: 10s @@ -142,6 +145,7 @@ tests: path: data.nri-kubernetes\.yml value: |- interval: 15s + namespaceSelector: {} controlPlane: retries: 3 timeout: 10s diff --git a/charts/newrelic-infrastructure/tests/configmap_ksm_scraper_test.yaml b/charts/newrelic-infrastructure/tests/configmap_ksm_scraper_test.yaml index 224eda5ee..aea67e23d 100644 --- a/charts/newrelic-infrastructure/tests/configmap_ksm_scraper_test.yaml +++ b/charts/newrelic-infrastructure/tests/configmap_ksm_scraper_test.yaml @@ -34,6 +34,7 @@ tests: path: data.nri-kubernetes\.yml value: |- interval: 15s + namespaceSelector: {} ksm: enabled: true port: 22 @@ -52,6 +53,7 @@ tests: path: data.nri-kubernetes\.yml value: |- interval: 15s + namespaceSelector: {} ksm: enabled: true retries: 3 @@ -70,6 +72,7 @@ tests: path: data.nri-kubernetes\.yml value: |- interval: 15s + namespaceSelector: {} ksm: enabled: true port: 22 @@ -94,6 +97,7 @@ tests: path: data.nri-kubernetes\.yml value: |- interval: 15s + namespaceSelector: {} ksm: distributed: true enabled: true From 37ffb092acb2ec708e25b3116f06a70006e29ef0 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Tue, 31 May 2022 11:08:28 +0200 Subject: [PATCH 06/18] refactor: address several PR comments --- internal/discovery/namespace_filter.go | 9 +++++---- internal/discovery/namespace_filter_test.go | 7 +++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index 2eb243112..75b2c217f 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -22,7 +22,8 @@ type NamespaceFilter struct { lister listersv1.NamespaceLister } -// NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter. +// NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter and a channel to close the informer +// gracefully. func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, options ...informers.SharedInformerOption) (*NamespaceFilter, chan<- struct{}) { stopCh := make(chan struct{}) @@ -51,7 +52,7 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { namespaceList, err := nf.lister.List(labels.SelectorFromSet(nf.c.NamespaceSelector.MatchLabels)) if err != nil { log.Errorf("listing namespaces with MatchLabels: %v", err) - return false + return true } return containsNamespace(namespace, namespaceList) @@ -64,13 +65,13 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { selector, err := labels.Parse(expression.String()) if err != nil { log.Errorf("parsing labels: %v", err) - return false + return true } namespaceList, err := nf.lister.List(selector) if err != nil { log.Errorf("listing namespaces with MatchExpressions: %v", err) - return false + return true } if !containsNamespace(namespace, namespaceList) { diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go index 68f02893f..ae304e377 100644 --- a/internal/discovery/namespace_filter_test.go +++ b/internal/discovery/namespace_filter_test.go @@ -183,11 +183,10 @@ func TestNamespaceFilter_CacheSync(t *testing.T) { require.NoError(t, err) // Give some room to the informer to sync, and check that the new namespace is filtered properly. - if err := wait.PollImmediateUntilWithContext(ctx, 1*time.Second, func(context.Context) (bool, error) { + err = wait.PollImmediateUntilWithContext(ctx, 1*time.Second, func(context.Context) (bool, error) { return nsFilter.IsAllowed(anotherNamespaceName), nil - }); err != nil { - t.Fatalf("Timed out waiting for the informer to sync: %v", err) - } + }) + require.NoError(t, err, "Timed out waiting for the informer to sync") } func fakeNamespaceWithNameAndLabels(name string, l labels.Set) *corev1.Namespace { From 641db92044983fd0abdf4b92c53e69a880243f46 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Tue, 31 May 2022 11:14:17 +0200 Subject: [PATCH 07/18] chore: bump chart to v.3.5.4 --- charts/newrelic-infrastructure/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/newrelic-infrastructure/Chart.yaml b/charts/newrelic-infrastructure/Chart.yaml index a4f7976eb..6928d0e7e 100644 --- a/charts/newrelic-infrastructure/Chart.yaml +++ b/charts/newrelic-infrastructure/Chart.yaml @@ -8,7 +8,7 @@ sources: - https://github.com/newrelic/nri-kubernetes/tree/master/charts/newrelic-infrastructure - https://github.com/newrelic/infrastructure-agent/ -version: 3.5.3 +version: 3.5.4 appVersion: 3.2.0 dependencies: From f67c113f604ba54240328a1796f1b376b12e17f6 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Wed, 1 Jun 2022 08:38:16 +0200 Subject: [PATCH 08/18] refactor: encapsulate stop chan closing in the informer --- internal/discovery/namespace_filter.go | 22 +++++++++++++++++---- internal/discovery/namespace_filter_test.go | 14 ++++++++++--- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index 75b2c217f..74bd3d6c9 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -1,6 +1,8 @@ package discovery import ( + "errors" + "github.com/newrelic/nri-kubernetes/v3/internal/config" log "github.com/sirupsen/logrus" @@ -20,11 +22,11 @@ type NamespaceFilterer interface { type NamespaceFilter struct { c *config.Config lister listersv1.NamespaceLister + stopCh chan<- struct{} } -// NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter and a channel to close the informer -// gracefully. -func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, options ...informers.SharedInformerOption) (*NamespaceFilter, chan<- struct{}) { +// NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter. +func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, options ...informers.SharedInformerOption) *NamespaceFilter { stopCh := make(chan struct{}) factory := informers.NewSharedInformerFactoryWithOptions(client, defaultResyncDuration, options...) @@ -37,7 +39,8 @@ func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, options . return &NamespaceFilter{ c: c, lister: lister, - }, stopCh + stopCh: stopCh, + } } // IsAllowed checks given any namespace, if it's allowed to be scraped by using the NamespaceLister @@ -83,6 +86,17 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { return true } +// Close closes the stop channel and implements the Closer interface. +func (nf *NamespaceFilter) Close() error { + if nf.stopCh == nil { + return errors.New("invalid channel") + } + + close(nf.stopCh) + + return nil +} + // containsNamespace checks if a namespaces is contained in a given list of namespaces. func containsNamespace(namespace string, namespaceList []*v1.Namespace) bool { for _, n := range namespaceList { diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go index ae304e377..578232e51 100644 --- a/internal/discovery/namespace_filter_test.go +++ b/internal/discovery/namespace_filter_test.go @@ -139,7 +139,11 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { require.NoError(t, err) c := config.Config{NamespaceSelector: &testData.namespaceSelector} - nsFilter, _ := discovery.NewNamespaceFilter(&c, client) + nsFilter := discovery.NewNamespaceFilter(&c, client) + + t.Cleanup(func() { + nsFilter.Close() + }) require.Equal(t, testData.expected, nsFilter.IsAllowed(namespaceName)) }) @@ -153,7 +157,6 @@ func TestNamespaceFilter_CacheSync(t *testing.T) { client := testclient.NewSimpleClientset() ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(5*time.Second)) - t.Cleanup(cancel) // Create a namespace with a specific label. _, err := client.CoreV1().Namespaces().Create( @@ -164,7 +167,7 @@ func TestNamespaceFilter_CacheSync(t *testing.T) { require.NoError(t, err) // Create the namespace filter. - nsFilter, _ := discovery.NewNamespaceFilter(&config.Config{ + nsFilter := discovery.NewNamespaceFilter(&config.Config{ NamespaceSelector: &config.NamespaceSelector{ MatchLabels: map[string]string{ "test_label": "123", @@ -174,6 +177,11 @@ func TestNamespaceFilter_CacheSync(t *testing.T) { // Check that recently created namespace is not allowed. require.Equal(t, false, nsFilter.IsAllowed(namespaceName)) + t.Cleanup(func() { + cancel() + nsFilter.Close() + }) + // Create a new namespace that can be filtered with the previous given config. _, err = client.CoreV1().Namespaces().Create( ctx, From 3f558b7cb9c5a01a8b5fff641506b2131c8fab68 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Thu, 2 Jun 2022 16:36:26 +0200 Subject: [PATCH 09/18] feat: add isAllowed result cache layer --- internal/discovery/namespace_filter.go | 21 ++- internal/discovery/namespace_filter_test.go | 150 +++++++++++++++++++- internal/storer/storer.go | 5 + 3 files changed, 171 insertions(+), 5 deletions(-) diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index 74bd3d6c9..0d9588396 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/newrelic/nri-kubernetes/v3/internal/config" + "github.com/newrelic/nri-kubernetes/v3/internal/storer" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" @@ -23,10 +24,11 @@ type NamespaceFilter struct { c *config.Config lister listersv1.NamespaceLister stopCh chan<- struct{} + storer storer.Storer } // NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter. -func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, options ...informers.SharedInformerOption) *NamespaceFilter { +func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, storer storer.Storer, options ...informers.SharedInformerOption) *NamespaceFilter { stopCh := make(chan struct{}) factory := informers.NewSharedInformerFactoryWithOptions(client, defaultResyncDuration, options...) @@ -40,6 +42,7 @@ func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, options . c: c, lister: lister, stopCh: stopCh, + storer: storer, } } @@ -50,6 +53,12 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { return true } + // Check if the namespace is already in the cache. + var allowed bool + if _, err := nf.storer.Get(namespace, &allowed); err == nil { + return allowed + } + // Scrape namespaces by honoring the matchLabels values. if nf.c.NamespaceSelector.MatchLabels != nil { namespaceList, err := nf.lister.List(labels.SelectorFromSet(nf.c.NamespaceSelector.MatchLabels)) @@ -58,7 +67,12 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { return true } - return containsNamespace(namespace, namespaceList) + allowed = containsNamespace(namespace, namespaceList) + + // Save the namespace in the cache. + _ = nf.storer.Set(namespace, allowed) + + return allowed } // Scrape namespaces by honoring the matchExpressions values. @@ -83,6 +97,9 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { } } + // Save the namespace in the cache. + _ = nf.storer.Set(namespace, true) + return true } diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go index 578232e51..45a7bbb41 100644 --- a/internal/discovery/namespace_filter_test.go +++ b/internal/discovery/namespace_filter_test.go @@ -2,13 +2,18 @@ package discovery_test import ( "context" + "reflect" + "sync" "testing" "time" + "github.com/newrelic/infra-integrations-sdk/persist" "github.com/newrelic/nri-kubernetes/v3/internal/config" "github.com/newrelic/nri-kubernetes/v3/internal/discovery" + "github.com/newrelic/nri-kubernetes/v3/internal/storer" "github.com/stretchr/testify/require" + log "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -137,9 +142,71 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { metav1.CreateOptions{}, ) require.NoError(t, err) + c := config.Config{NamespaceSelector: &testData.namespaceSelector} + nsFilter := discovery.NewNamespaceFilter(&c, client, newStorerMock(false, persist.ErrNotFound)) + + t.Cleanup(func() { + nsFilter.Close() + }) + + require.Equal(t, testData.expected, nsFilter.IsAllowed(namespaceName)) + }) + } +} + +func TestNamespaceFilterer_GetFromCache(t *testing.T) { + t.Parallel() + + type testData struct { + namespaceLabels labels.Set + namespaceSelector config.NamespaceSelector + storer storer.Storer + expected bool + } + + testCases := map[string]testData{ + "namespace_already_in_cache_allowed": { + namespaceLabels: labels.Set{}, + namespaceSelector: config.NamespaceSelector{}, + storer: newStorerMock(true, nil), + expected: true, + }, + "namespace_already_in_cache_not_allowed": { + namespaceLabels: labels.Set{}, + namespaceSelector: config.NamespaceSelector{}, + storer: newStorerMock(false, nil), + expected: false, + }, + "namespace_cache_miss_fallback_to_call_informer": { + namespaceLabels: labels.Set{ + "newrelic.com/scrape": "true", + }, + namespaceSelector: config.NamespaceSelector{ + MatchLabels: map[string]string{ + "newrelic.com/scrape": "true", + }, + }, + storer: newStorerMock(false, persist.ErrNotFound), + expected: true, + }, + } - nsFilter := discovery.NewNamespaceFilter(&c, client) + for testName, testData := range testCases { + testData := testData + t.Run(testName, func(t *testing.T) { + t.Parallel() + + client := testclient.NewSimpleClientset() + _, err := client.CoreV1().Namespaces().Create( + context.Background(), + fakeNamespaceWithNameAndLabels(namespaceName, testData.namespaceLabels), + metav1.CreateOptions{}, + ) + require.NoError(t, err) + + c := config.Config{NamespaceSelector: &testData.namespaceSelector} + nsFilter := discovery.NewNamespaceFilter(&c, client, testData.storer) t.Cleanup(func() { nsFilter.Close() @@ -150,7 +217,47 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { } } -func TestNamespaceFilter_CacheSync(t *testing.T) { +func TestNamespaceFilterer_SetCache(t *testing.T) { + t.Parallel() + + storer := storer.NewInMemoryStore(storer.DefaultTTL, storer.DefaultInterval, log.StandardLogger()) + + namespaceLabels := labels.Set{"test_label": "1234"} + namespaceSelector := config.NamespaceSelector{ + MatchLabels: map[string]string{ + "test_label": "1234", + }, + } + + client := testclient.NewSimpleClientset() + _, err := client.CoreV1().Namespaces().Create( + context.Background(), + fakeNamespaceWithNameAndLabels(namespaceName, namespaceLabels), + metav1.CreateOptions{}, + ) + require.NoError(t, err) + + c := config.Config{NamespaceSelector: &namespaceSelector} + nsFilter := discovery.NewNamespaceFilter(&c, client, storer) + + t.Cleanup(func() { + nsFilter.Close() + }) + + var allowed bool + _, err = storer.Get(namespaceName, &allowed) + // Empty cache first time, namespace not found. + require.ErrorIs(t, err, persist.ErrNotFound) + + nsFilter.IsAllowed(namespaceName) + + _, err = storer.Get(namespaceName, &allowed) + // Cache should be populated with the IsAllowed result. + require.NoError(t, err) + require.Equal(t, true, allowed) +} + +func TestNamespaceFilter_InformerCacheSync(t *testing.T) { t.Parallel() anotherNamespaceName := "another_namespace" @@ -173,7 +280,10 @@ func TestNamespaceFilter_CacheSync(t *testing.T) { "test_label": "123", }, }, - }, client) + }, + client, + newStorerMock(false, persist.ErrNotFound), + ) // Check that recently created namespace is not allowed. require.Equal(t, false, nsFilter.IsAllowed(namespaceName)) @@ -197,6 +307,40 @@ func TestNamespaceFilter_CacheSync(t *testing.T) { require.NoError(t, err, "Timed out waiting for the informer to sync") } +// storerMock implements Grouper interface returning mocked metrics which might change over subsequent calls. +type storerMock struct { + allowed bool + err error + + locker *sync.RWMutex +} + +func newStorerMock(allowed bool, err error) *storerMock { + return &storerMock{ + allowed: allowed, + err: err, + locker: &sync.RWMutex{}, + } +} + +func (s *storerMock) Set(_ string, _ interface{}) int64 { + return time.Now().Unix() +} + +func (s *storerMock) Get(_ string, valuePtr interface{}) (int64, error) { + s.locker.RLock() + defer s.locker.RUnlock() + + entry := s.allowed + + // Using reflection to indirectly set the value passed as reference + varToPopulate := reflect.Indirect(reflect.ValueOf(valuePtr)) + valueToSet := reflect.Indirect(reflect.ValueOf(entry)) + varToPopulate.Set(valueToSet) + + return time.Now().Unix(), s.err +} + func fakeNamespaceWithNameAndLabels(name string, l labels.Set) *corev1.Namespace { return &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/storer/storer.go b/internal/storer/storer.go index 65803eeab..766b34725 100644 --- a/internal/storer/storer.go +++ b/internal/storer/storer.go @@ -19,6 +19,11 @@ const ( DefaultInterval = 15 * time.Minute ) +type Storer interface { + Set(key string, value interface{}) int64 + Get(key string, valuePtr interface{}) (int64, error) +} + // InMemoryStore is similar to the sdk one, the main difference is cleanCache method executed each interval. type InMemoryStore struct { cachedData map[string]jsonEntry From ccdd7e20e710e890723333e9b9f3a8f5bf153429 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Fri, 3 Jun 2022 09:19:42 +0200 Subject: [PATCH 10/18] fix: address several comments --- internal/config/config.go | 6 ++-- internal/discovery/namespace_filter.go | 22 +++++++------- internal/discovery/namespace_filter_test.go | 33 ++++++++++++++------- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 4d734dc58..2da1431b0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -235,11 +235,11 @@ type NamespaceSelector struct { // Expression hold the values to generate an expression to filter namespaces by MatchExpressions. type Expression struct { // Key it the key of the label. - Key string `mapstructure:"key" json:"key"` + Key string `mapstructure:"key"` // Operator holds either an inclusion (NotIn) or exclusion (In) value. - Operator string `mapstructure:"operator" json:"operator"` + Operator string `mapstructure:"operator"` // Values is a slice of values related to the key. - Values []interface{} `mapstructure:"values" json:"values"` + Values []interface{} `mapstructure:"values"` } func (e *Expression) String() string { diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index 0d9588396..5b597e0b2 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -21,14 +21,15 @@ type NamespaceFilterer interface { // NamespaceFilter is a struct holding pointers to the config and the namespace lister. type NamespaceFilter struct { - c *config.Config + c *config.NamespaceSelector lister listersv1.NamespaceLister stopCh chan<- struct{} storer storer.Storer + logger *log.Logger } // NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter. -func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, storer storer.Storer, options ...informers.SharedInformerOption) *NamespaceFilter { +func NewNamespaceFilter(c *config.NamespaceSelector, client kubernetes.Interface, storer storer.Storer, logger *log.Logger, options ...informers.SharedInformerOption) *NamespaceFilter { stopCh := make(chan struct{}) factory := informers.NewSharedInformerFactoryWithOptions(client, defaultResyncDuration, options...) @@ -43,13 +44,14 @@ func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, storer st lister: lister, stopCh: stopCh, storer: storer, + logger: logger, } } // IsAllowed checks given any namespace, if it's allowed to be scraped by using the NamespaceLister func (nf *NamespaceFilter) IsAllowed(namespace string) bool { // By default, we scrape every namespace. - if nf.c.NamespaceSelector == nil { + if nf.c == nil { return true } @@ -60,10 +62,10 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { } // Scrape namespaces by honoring the matchLabels values. - if nf.c.NamespaceSelector.MatchLabels != nil { - namespaceList, err := nf.lister.List(labels.SelectorFromSet(nf.c.NamespaceSelector.MatchLabels)) + if nf.c.MatchLabels != nil { + namespaceList, err := nf.lister.List(labels.SelectorFromSet(nf.c.MatchLabels)) if err != nil { - log.Errorf("listing namespaces with MatchLabels: %v", err) + nf.logger.Errorf("listing namespaces with MatchLabels: %v", err) return true } @@ -77,17 +79,17 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { // Scrape namespaces by honoring the matchExpressions values. // Multiple expressions are evaluated with a logical AND between them. - if nf.c.NamespaceSelector.MatchExpressions != nil { - for _, expression := range nf.c.NamespaceSelector.MatchExpressions { + if nf.c.MatchExpressions != nil { + for _, expression := range nf.c.MatchExpressions { selector, err := labels.Parse(expression.String()) if err != nil { - log.Errorf("parsing labels: %v", err) + nf.logger.Errorf("parsing labels: %v", err) return true } namespaceList, err := nf.lister.List(selector) if err != nil { - log.Errorf("listing namespaces with MatchExpressions: %v", err) + nf.logger.Errorf("listing namespaces with MatchExpressions: %v", err) return true } diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go index 45a7bbb41..7f7a4c862 100644 --- a/internal/discovery/namespace_filter_test.go +++ b/internal/discovery/namespace_filter_test.go @@ -36,15 +36,19 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { "namespace_allowed_by_default": { expected: true, }, + "namespace_allowed_with_labels_and_no_selector": { + namespaceLabels: labels.Set{ + "newrelic.com/scrape": "true", + }, + expected: true, + }, "match_labels_included_namespace_allowed": { namespaceLabels: labels.Set{ "newrelic.com/scrape": "true", - "ohhh": "xxx", }, namespaceSelector: config.NamespaceSelector{ MatchLabels: map[string]string{ "newrelic.com/scrape": "true", - "ohhh": "xxx", }, }, expected: true, @@ -143,8 +147,12 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { ) require.NoError(t, err) - c := config.Config{NamespaceSelector: &testData.namespaceSelector} - nsFilter := discovery.NewNamespaceFilter(&c, client, newStorerMock(false, persist.ErrNotFound)) + nsFilter := discovery.NewNamespaceFilter( + &testData.namespaceSelector, + client, + newStorerMock(false, persist.ErrNotFound), + nil, + ) t.Cleanup(func() { nsFilter.Close() @@ -205,8 +213,12 @@ func TestNamespaceFilterer_GetFromCache(t *testing.T) { ) require.NoError(t, err) - c := config.Config{NamespaceSelector: &testData.namespaceSelector} - nsFilter := discovery.NewNamespaceFilter(&c, client, testData.storer) + nsFilter := discovery.NewNamespaceFilter( + &testData.namespaceSelector, + client, + testData.storer, + nil, + ) t.Cleanup(func() { nsFilter.Close() @@ -237,8 +249,7 @@ func TestNamespaceFilterer_SetCache(t *testing.T) { ) require.NoError(t, err) - c := config.Config{NamespaceSelector: &namespaceSelector} - nsFilter := discovery.NewNamespaceFilter(&c, client, storer) + nsFilter := discovery.NewNamespaceFilter(&namespaceSelector, client, storer, nil) t.Cleanup(func() { nsFilter.Close() @@ -274,15 +285,15 @@ func TestNamespaceFilter_InformerCacheSync(t *testing.T) { require.NoError(t, err) // Create the namespace filter. - nsFilter := discovery.NewNamespaceFilter(&config.Config{ - NamespaceSelector: &config.NamespaceSelector{ + nsFilter := discovery.NewNamespaceFilter( + &config.NamespaceSelector{ MatchLabels: map[string]string{ "test_label": "123", }, }, - }, client, newStorerMock(false, persist.ErrNotFound), + nil, ) // Check that recently created namespace is not allowed. require.Equal(t, false, nsFilter.IsAllowed(namespaceName)) From 7158e361f86e9713a3d2497ab1ff194a9d33b044 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Fri, 3 Jun 2022 14:56:02 +0200 Subject: [PATCH 11/18] refactor: storer cache --- go.mod | 1 + go.sum | 1 + internal/discovery/namespace_filter.go | 55 ++++-- internal/discovery/namespace_filter_test.go | 184 +++++++------------- 4 files changed, 105 insertions(+), 136 deletions(-) diff --git a/go.mod b/go.mod index 38c4fe56d..190746a7d 100644 --- a/go.mod +++ b/go.mod @@ -52,6 +52,7 @@ require ( github.com/spf13/cast v1.4.1 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect + github.com/stretchr/objx v0.1.1 // indirect github.com/subosito/gotenv v1.2.0 // indirect golang.org/x/net v0.0.0-20220412020605-290c469a71a5 // indirect golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 // indirect diff --git a/go.sum b/go.sum index 0a05f1641..d58312d6c 100644 --- a/go.sum +++ b/go.sum @@ -395,6 +395,7 @@ github.com/spf13/viper v1.11.0 h1:7OX/1FS6n7jHD1zGrZTM7WtY13ZELRyosK4k93oPr44= github.com/spf13/viper v1.11.0/go.mod h1:djo0X/bA5+tYVoCn+C7cAYJGcVn/qYLFTG8gdUsX7Zk= github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index 5b597e0b2..d8b551fae 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -2,6 +2,7 @@ package discovery import ( "errors" + "time" "github.com/newrelic/nri-kubernetes/v3/internal/config" "github.com/newrelic/nri-kubernetes/v3/internal/storer" @@ -14,6 +15,8 @@ import ( listersv1 "k8s.io/client-go/listers/core/v1" ) +const defaultNamespaceResyncDuration = 10 * time.Minute + // NamespaceFilterer provides an interface to filter namespaces. type NamespaceFilterer interface { IsAllowed(namespace string) bool @@ -24,15 +27,14 @@ type NamespaceFilter struct { c *config.NamespaceSelector lister listersv1.NamespaceLister stopCh chan<- struct{} - storer storer.Storer logger *log.Logger } // NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter. -func NewNamespaceFilter(c *config.NamespaceSelector, client kubernetes.Interface, storer storer.Storer, logger *log.Logger, options ...informers.SharedInformerOption) *NamespaceFilter { +func NewNamespaceFilter(c *config.NamespaceSelector, client kubernetes.Interface, logger *log.Logger, options ...informers.SharedInformerOption) *NamespaceFilter { stopCh := make(chan struct{}) - factory := informers.NewSharedInformerFactoryWithOptions(client, defaultResyncDuration, options...) + factory := informers.NewSharedInformerFactoryWithOptions(client, defaultNamespaceResyncDuration, options...) lister := factory.Core().V1().Namespaces().Lister() @@ -43,7 +45,6 @@ func NewNamespaceFilter(c *config.NamespaceSelector, client kubernetes.Interface c: c, lister: lister, stopCh: stopCh, - storer: storer, logger: logger, } } @@ -55,12 +56,6 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { return true } - // Check if the namespace is already in the cache. - var allowed bool - if _, err := nf.storer.Get(namespace, &allowed); err == nil { - return allowed - } - // Scrape namespaces by honoring the matchLabels values. if nf.c.MatchLabels != nil { namespaceList, err := nf.lister.List(labels.SelectorFromSet(nf.c.MatchLabels)) @@ -69,12 +64,7 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { return true } - allowed = containsNamespace(namespace, namespaceList) - - // Save the namespace in the cache. - _ = nf.storer.Set(namespace, allowed) - - return allowed + return containsNamespace(namespace, namespaceList) } // Scrape namespaces by honoring the matchExpressions values. @@ -99,9 +89,6 @@ func (nf *NamespaceFilter) IsAllowed(namespace string) bool { } } - // Save the namespace in the cache. - _ = nf.storer.Set(namespace, true) - return true } @@ -116,6 +103,36 @@ func (nf *NamespaceFilter) Close() error { return nil } +// CachedNamespaceFilter is a wrapper of the NamespaceFilterer and the cache. +type CachedNamespaceFilter struct { + NsFilter NamespaceFilterer + cache storer.Storer +} + +// NewCachedNamespaceFilter create a new CachedNamespaceFilter, wrapping the cache and the NamespaceFilterer. +func NewCachedNamespaceFilter(ns NamespaceFilterer, storer storer.Storer) *CachedNamespaceFilter { + return &CachedNamespaceFilter{ + NsFilter: ns, + cache: storer, + } +} + +// IsAllowed check the cache and calls the underlying NamespaceFilter if the result is not found. +func (cnf *CachedNamespaceFilter) IsAllowed(namespace string) bool { + // Check if the namespace is already in the cache. + var allowed bool + if _, err := cnf.cache.Get(namespace, &allowed); err == nil { + return allowed + } + + allowed = cnf.NsFilter.IsAllowed(namespace) + + // Save the namespace in the cache. + _ = cnf.cache.Set(namespace, allowed) + + return allowed +} + // containsNamespace checks if a namespaces is contained in a given list of namespaces. func containsNamespace(namespace string, namespaceList []*v1.Namespace) bool { for _, n := range namespaceList { diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go index 7f7a4c862..e077564e1 100644 --- a/internal/discovery/namespace_filter_test.go +++ b/internal/discovery/namespace_filter_test.go @@ -2,18 +2,15 @@ package discovery_test import ( "context" - "reflect" - "sync" "testing" "time" - "github.com/newrelic/infra-integrations-sdk/persist" "github.com/newrelic/nri-kubernetes/v3/internal/config" "github.com/newrelic/nri-kubernetes/v3/internal/discovery" "github.com/newrelic/nri-kubernetes/v3/internal/storer" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - log "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -147,55 +144,75 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { ) require.NoError(t, err) - nsFilter := discovery.NewNamespaceFilter( + ns := discovery.NewNamespaceFilter( &testData.namespaceSelector, client, - newStorerMock(false, persist.ErrNotFound), nil, ) t.Cleanup(func() { - nsFilter.Close() + ns.Close() }) - require.Equal(t, testData.expected, nsFilter.IsAllowed(namespaceName)) + require.Equal(t, testData.expected, ns.IsAllowed(namespaceName)) }) } } -func TestNamespaceFilterer_GetFromCache(t *testing.T) { +func TestNamespaceFilterer_Cache(t *testing.T) { t.Parallel() type testData struct { - namespaceLabels labels.Set - namespaceSelector config.NamespaceSelector - storer storer.Storer - expected bool + warmCache func(cache *storer.InMemoryStore) + prepare func(nsFilterMock *NamespaceFilterMock) + assert func(expected bool, cnsf *discovery.CachedNamespaceFilter) + expected bool } testCases := map[string]testData{ + "namespace_cache_miss_fallback_to_call_informer": { + warmCache: func(cache *storer.InMemoryStore) {}, + prepare: func(nsFilterMock *NamespaceFilterMock) { + nsFilterMock.On("IsAllowed", namespaceName).Return(true).Once() + }, + assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + }, + expected: true, + }, "namespace_already_in_cache_allowed": { - namespaceLabels: labels.Set{}, - namespaceSelector: config.NamespaceSelector{}, - storer: newStorerMock(true, nil), - expected: true, + warmCache: func(cache *storer.InMemoryStore) { + cache.Set(namespaceName, true) + }, + prepare: func(nsFilterMock *NamespaceFilterMock) { + nsFilterMock.AssertNotCalled(t, "IsAllowed") + }, + assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + }, + expected: true, }, "namespace_already_in_cache_not_allowed": { - namespaceLabels: labels.Set{}, - namespaceSelector: config.NamespaceSelector{}, - storer: newStorerMock(false, nil), - expected: false, + warmCache: func(cache *storer.InMemoryStore) { + cache.Set(namespaceName, false) + }, + prepare: func(nsFilterMock *NamespaceFilterMock) { + nsFilterMock.AssertNotCalled(t, "IsAllowed") + }, + assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + }, + expected: false, }, - "namespace_cache_miss_fallback_to_call_informer": { - namespaceLabels: labels.Set{ - "newrelic.com/scrape": "true", + "namespace_cache_miss_subsequent_call_uses_cache": { + warmCache: func(cache *storer.InMemoryStore) {}, + prepare: func(nsFilterMock *NamespaceFilterMock) { + nsFilterMock.On("IsAllowed", namespaceName).Return(true).Once() }, - namespaceSelector: config.NamespaceSelector{ - MatchLabels: map[string]string{ - "newrelic.com/scrape": "true", - }, + assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) }, - storer: newStorerMock(false, persist.ErrNotFound), expected: true, }, } @@ -205,69 +222,24 @@ func TestNamespaceFilterer_GetFromCache(t *testing.T) { t.Run(testName, func(t *testing.T) { t.Parallel() - client := testclient.NewSimpleClientset() - _, err := client.CoreV1().Namespaces().Create( - context.Background(), - fakeNamespaceWithNameAndLabels(namespaceName, testData.namespaceLabels), - metav1.CreateOptions{}, - ) - require.NoError(t, err) + nsFilterMock := newNamespaceFilterMock() - nsFilter := discovery.NewNamespaceFilter( - &testData.namespaceSelector, - client, - testData.storer, - nil, + cache := storer.NewInMemoryStore(storer.DefaultTTL, storer.DefaultInterval, nil) + testData.warmCache(cache) + testData.prepare(nsFilterMock) + + cnsf := discovery.NewCachedNamespaceFilter( + nsFilterMock, + cache, ) - t.Cleanup(func() { - nsFilter.Close() - }) + testData.assert(testData.expected, cnsf) - require.Equal(t, testData.expected, nsFilter.IsAllowed(namespaceName)) + mock.AssertExpectationsForObjects(t, nsFilterMock) }) } } -func TestNamespaceFilterer_SetCache(t *testing.T) { - t.Parallel() - - storer := storer.NewInMemoryStore(storer.DefaultTTL, storer.DefaultInterval, log.StandardLogger()) - - namespaceLabels := labels.Set{"test_label": "1234"} - namespaceSelector := config.NamespaceSelector{ - MatchLabels: map[string]string{ - "test_label": "1234", - }, - } - - client := testclient.NewSimpleClientset() - _, err := client.CoreV1().Namespaces().Create( - context.Background(), - fakeNamespaceWithNameAndLabels(namespaceName, namespaceLabels), - metav1.CreateOptions{}, - ) - require.NoError(t, err) - - nsFilter := discovery.NewNamespaceFilter(&namespaceSelector, client, storer, nil) - - t.Cleanup(func() { - nsFilter.Close() - }) - - var allowed bool - _, err = storer.Get(namespaceName, &allowed) - // Empty cache first time, namespace not found. - require.ErrorIs(t, err, persist.ErrNotFound) - - nsFilter.IsAllowed(namespaceName) - - _, err = storer.Get(namespaceName, &allowed) - // Cache should be populated with the IsAllowed result. - require.NoError(t, err) - require.Equal(t, true, allowed) -} - func TestNamespaceFilter_InformerCacheSync(t *testing.T) { t.Parallel() @@ -285,22 +257,21 @@ func TestNamespaceFilter_InformerCacheSync(t *testing.T) { require.NoError(t, err) // Create the namespace filter. - nsFilter := discovery.NewNamespaceFilter( + ns := discovery.NewNamespaceFilter( &config.NamespaceSelector{ MatchLabels: map[string]string{ "test_label": "123", }, }, client, - newStorerMock(false, persist.ErrNotFound), nil, ) // Check that recently created namespace is not allowed. - require.Equal(t, false, nsFilter.IsAllowed(namespaceName)) + require.Equal(t, false, ns.IsAllowed(namespaceName)) t.Cleanup(func() { cancel() - nsFilter.Close() + ns.Close() }) // Create a new namespace that can be filtered with the previous given config. @@ -313,43 +284,22 @@ func TestNamespaceFilter_InformerCacheSync(t *testing.T) { // Give some room to the informer to sync, and check that the new namespace is filtered properly. err = wait.PollImmediateUntilWithContext(ctx, 1*time.Second, func(context.Context) (bool, error) { - return nsFilter.IsAllowed(anotherNamespaceName), nil + return ns.IsAllowed(anotherNamespaceName), nil }) require.NoError(t, err, "Timed out waiting for the informer to sync") } -// storerMock implements Grouper interface returning mocked metrics which might change over subsequent calls. -type storerMock struct { - allowed bool - err error - - locker *sync.RWMutex -} - -func newStorerMock(allowed bool, err error) *storerMock { - return &storerMock{ - allowed: allowed, - err: err, - locker: &sync.RWMutex{}, - } +type NamespaceFilterMock struct { + mock.Mock } -func (s *storerMock) Set(_ string, _ interface{}) int64 { - return time.Now().Unix() +func newNamespaceFilterMock() *NamespaceFilterMock { + return &NamespaceFilterMock{} } -func (s *storerMock) Get(_ string, valuePtr interface{}) (int64, error) { - s.locker.RLock() - defer s.locker.RUnlock() - - entry := s.allowed - - // Using reflection to indirectly set the value passed as reference - varToPopulate := reflect.Indirect(reflect.ValueOf(valuePtr)) - valueToSet := reflect.Indirect(reflect.ValueOf(entry)) - varToPopulate.Set(valueToSet) - - return time.Now().Unix(), s.err +func (ns *NamespaceFilterMock) IsAllowed(namespace string) bool { + args := ns.Called(namespace) + return args.Bool(0) } func fakeNamespaceWithNameAndLabels(name string, l labels.Set) *corev1.Namespace { From e57ba695a157903790cc9878ad0f6cbf032beb9f Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Mon, 6 Jun 2022 18:44:49 +0200 Subject: [PATCH 12/18] refactor: update NamespaceFilterer interface to be more generic --- internal/discovery/namespace_filter.go | 105 +++++++++++++------- internal/discovery/namespace_filter_test.go | 33 +++--- 2 files changed, 89 insertions(+), 49 deletions(-) diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index d8b551fae..aaca65534 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -6,6 +6,7 @@ import ( "github.com/newrelic/nri-kubernetes/v3/internal/config" "github.com/newrelic/nri-kubernetes/v3/internal/storer" + "github.com/newrelic/nri-kubernetes/v3/src/definition" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" @@ -17,9 +18,9 @@ import ( const defaultNamespaceResyncDuration = 10 * time.Minute -// NamespaceFilterer provides an interface to filter namespaces. -type NamespaceFilterer interface { - IsAllowed(namespace string) bool +// Filterer provider a interface to match from a given RawMetrics. +type Filterer interface { + Match(metrics definition.RawMetrics) bool } // NamespaceFilter is a struct holding pointers to the config and the namespace lister. @@ -49,43 +50,66 @@ func NewNamespaceFilter(c *config.NamespaceSelector, client kubernetes.Interface } } -// IsAllowed checks given any namespace, if it's allowed to be scraped by using the NamespaceLister -func (nf *NamespaceFilter) IsAllowed(namespace string) bool { - // By default, we scrape every namespace. +// Match checks given any metrics, if a namespace it's allowed to be scraped given a certain match labels or +// expressions configuration. +func (nf *NamespaceFilter) Match(metrics definition.RawMetrics) bool { + namespaceRaw, ok := metrics["namespace"] + if !ok { + return true + } + + namespace, ok := namespaceRaw.(string) + if !ok { + log.Tracef("Allowing %q as namespace as invalid type provided %t", namespaceRaw, namespaceRaw) + return true + } + if nf.c == nil { + log.Tracef("Allowing %q namespace as selector is nil", namespace) return true } - // Scrape namespaces by honoring the matchLabels values. if nf.c.MatchLabels != nil { - namespaceList, err := nf.lister.List(labels.SelectorFromSet(nf.c.MatchLabels)) + log.Tracef("Filtering %q namespace by MatchLabels", namespace) + return nf.matchNamespaceByLabels(namespace) + } + + if nf.c.MatchExpressions != nil { + log.Tracef("Filtering %q namespace by MatchExpressions", namespace) + return nf.matchNamespaceByExpressions(namespace) + } + + return true +} + +// matchNamespaceByLabels filters a namespace using the selector from the MatchLabels config. +func (nf *NamespaceFilter) matchNamespaceByLabels(namespace string) bool { + namespaceList, err := nf.lister.List(labels.SelectorFromSet(nf.c.MatchLabels)) + if err != nil { + nf.logger.Errorf("listing namespaces with MatchLabels: %v", err) + return true + } + + return containsNamespace(namespace, namespaceList) +} + +// matchNamespaceByExpressions filters a namespace using the selector from the MatchExpressions config. +func (nf *NamespaceFilter) matchNamespaceByExpressions(namespace string) bool { + for _, expression := range nf.c.MatchExpressions { + selector, err := labels.Parse(expression.String()) if err != nil { - nf.logger.Errorf("listing namespaces with MatchLabels: %v", err) + nf.logger.Errorf("parsing labels: %v", err) return true } - return containsNamespace(namespace, namespaceList) - } + namespaceList, err := nf.lister.List(selector) + if err != nil { + nf.logger.Errorf("listing namespaces with MatchExpressions: %v", err) + return true + } - // Scrape namespaces by honoring the matchExpressions values. - // Multiple expressions are evaluated with a logical AND between them. - if nf.c.MatchExpressions != nil { - for _, expression := range nf.c.MatchExpressions { - selector, err := labels.Parse(expression.String()) - if err != nil { - nf.logger.Errorf("parsing labels: %v", err) - return true - } - - namespaceList, err := nf.lister.List(selector) - if err != nil { - nf.logger.Errorf("listing namespaces with MatchExpressions: %v", err) - return true - } - - if !containsNamespace(namespace, namespaceList) { - return false - } + if !containsNamespace(namespace, namespaceList) { + return false } } @@ -105,27 +129,38 @@ func (nf *NamespaceFilter) Close() error { // CachedNamespaceFilter is a wrapper of the NamespaceFilterer and the cache. type CachedNamespaceFilter struct { - NsFilter NamespaceFilterer + NsFilter Filterer cache storer.Storer } // NewCachedNamespaceFilter create a new CachedNamespaceFilter, wrapping the cache and the NamespaceFilterer. -func NewCachedNamespaceFilter(ns NamespaceFilterer, storer storer.Storer) *CachedNamespaceFilter { +func NewCachedNamespaceFilter(ns Filterer, storer storer.Storer) *CachedNamespaceFilter { return &CachedNamespaceFilter{ NsFilter: ns, cache: storer, } } -// IsAllowed check the cache and calls the underlying NamespaceFilter if the result is not found. -func (cnf *CachedNamespaceFilter) IsAllowed(namespace string) bool { +// Match check the cache and calls the underlying NamespaceFilter if the result is not found. +func (cnf *CachedNamespaceFilter) Match(metrics definition.RawMetrics) bool { + namespaceRaw, ok := metrics["namespace"] + if !ok { + return true + } + + namespace, ok := namespaceRaw.(string) + if !ok { + log.Tracef("Allowing %q from cache as namespace invalid type provided %t", namespaceRaw, namespaceRaw) + return true + } + // Check if the namespace is already in the cache. var allowed bool if _, err := cnf.cache.Get(namespace, &allowed); err == nil { return allowed } - allowed = cnf.NsFilter.IsAllowed(namespace) + allowed = cnf.NsFilter.Match(metrics) // Save the namespace in the cache. _ = cnf.cache.Set(namespace, allowed) diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go index e077564e1..550f7b40f 100644 --- a/internal/discovery/namespace_filter_test.go +++ b/internal/discovery/namespace_filter_test.go @@ -8,6 +8,7 @@ import ( "github.com/newrelic/nri-kubernetes/v3/internal/config" "github.com/newrelic/nri-kubernetes/v3/internal/discovery" "github.com/newrelic/nri-kubernetes/v3/internal/storer" + "github.com/newrelic/nri-kubernetes/v3/src/definition" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -23,6 +24,8 @@ const namespaceName = "test_namespace" func TestNamespaceFilterer_IsAllowed(t *testing.T) { t.Parallel() + metrics := definition.RawMetrics{"namespace": namespaceName} + type testData struct { namespaceLabels labels.Set namespaceSelector config.NamespaceSelector @@ -154,7 +157,7 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { ns.Close() }) - require.Equal(t, testData.expected, ns.IsAllowed(namespaceName)) + require.Equal(t, testData.expected, ns.Match(metrics)) }) } } @@ -162,6 +165,8 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { func TestNamespaceFilterer_Cache(t *testing.T) { t.Parallel() + metrics := definition.RawMetrics{"namespace": namespaceName} + type testData struct { warmCache func(cache *storer.InMemoryStore) prepare func(nsFilterMock *NamespaceFilterMock) @@ -173,10 +178,10 @@ func TestNamespaceFilterer_Cache(t *testing.T) { "namespace_cache_miss_fallback_to_call_informer": { warmCache: func(cache *storer.InMemoryStore) {}, prepare: func(nsFilterMock *NamespaceFilterMock) { - nsFilterMock.On("IsAllowed", namespaceName).Return(true).Once() + nsFilterMock.On("Match", metrics).Return(true).Once() }, assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { - require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + require.Equal(t, expected, cnsf.Match(metrics)) }, expected: true, }, @@ -185,10 +190,10 @@ func TestNamespaceFilterer_Cache(t *testing.T) { cache.Set(namespaceName, true) }, prepare: func(nsFilterMock *NamespaceFilterMock) { - nsFilterMock.AssertNotCalled(t, "IsAllowed") + nsFilterMock.AssertNotCalled(t, "Match") }, assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { - require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + require.Equal(t, expected, cnsf.Match(metrics)) }, expected: true, }, @@ -197,21 +202,21 @@ func TestNamespaceFilterer_Cache(t *testing.T) { cache.Set(namespaceName, false) }, prepare: func(nsFilterMock *NamespaceFilterMock) { - nsFilterMock.AssertNotCalled(t, "IsAllowed") + nsFilterMock.AssertNotCalled(t, "Match") }, assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { - require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + require.Equal(t, expected, cnsf.Match(metrics)) }, expected: false, }, "namespace_cache_miss_subsequent_call_uses_cache": { warmCache: func(cache *storer.InMemoryStore) {}, prepare: func(nsFilterMock *NamespaceFilterMock) { - nsFilterMock.On("IsAllowed", namespaceName).Return(true).Once() + nsFilterMock.On("Match", metrics).Return(true).Once() }, assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { - require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) - require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + require.Equal(t, expected, cnsf.Match(metrics)) + require.Equal(t, expected, cnsf.Match(metrics)) }, expected: true, }, @@ -267,7 +272,7 @@ func TestNamespaceFilter_InformerCacheSync(t *testing.T) { nil, ) // Check that recently created namespace is not allowed. - require.Equal(t, false, ns.IsAllowed(namespaceName)) + require.Equal(t, false, ns.Match(definition.RawMetrics{"namespace": namespaceName})) t.Cleanup(func() { cancel() @@ -284,7 +289,7 @@ func TestNamespaceFilter_InformerCacheSync(t *testing.T) { // Give some room to the informer to sync, and check that the new namespace is filtered properly. err = wait.PollImmediateUntilWithContext(ctx, 1*time.Second, func(context.Context) (bool, error) { - return ns.IsAllowed(anotherNamespaceName), nil + return ns.Match(definition.RawMetrics{"namespace": anotherNamespaceName}), nil }) require.NoError(t, err, "Timed out waiting for the informer to sync") } @@ -297,8 +302,8 @@ func newNamespaceFilterMock() *NamespaceFilterMock { return &NamespaceFilterMock{} } -func (ns *NamespaceFilterMock) IsAllowed(namespace string) bool { - args := ns.Called(namespace) +func (ns *NamespaceFilterMock) Match(metrics definition.RawMetrics) bool { + args := ns.Called(metrics) return args.Bool(0) } From 95f8b9d068328ecb9f115091592e9d6ab91efe9a Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Tue, 7 Jun 2022 12:59:03 +0200 Subject: [PATCH 13/18] refactor: revert to NamespaceFilter interface --- internal/discovery/namespace_filter.go | 44 +++++---------------- internal/discovery/namespace_filter_test.go | 33 +++++++--------- 2 files changed, 24 insertions(+), 53 deletions(-) diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index aaca65534..f723af287 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -6,7 +6,6 @@ import ( "github.com/newrelic/nri-kubernetes/v3/internal/config" "github.com/newrelic/nri-kubernetes/v3/internal/storer" - "github.com/newrelic/nri-kubernetes/v3/src/definition" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" @@ -18,9 +17,9 @@ import ( const defaultNamespaceResyncDuration = 10 * time.Minute -// Filterer provider a interface to match from a given RawMetrics. -type Filterer interface { - Match(metrics definition.RawMetrics) bool +// NamespaceFilterer provides an interface to filter from a given namespace. +type NamespaceFilterer interface { + IsAllowed(namespace string) bool } // NamespaceFilter is a struct holding pointers to the config and the namespace lister. @@ -50,20 +49,8 @@ func NewNamespaceFilter(c *config.NamespaceSelector, client kubernetes.Interface } } -// Match checks given any metrics, if a namespace it's allowed to be scraped given a certain match labels or -// expressions configuration. -func (nf *NamespaceFilter) Match(metrics definition.RawMetrics) bool { - namespaceRaw, ok := metrics["namespace"] - if !ok { - return true - } - - namespace, ok := namespaceRaw.(string) - if !ok { - log.Tracef("Allowing %q as namespace as invalid type provided %t", namespaceRaw, namespaceRaw) - return true - } - +// IsAllowed checks if a namespace is allowed to be scraped given a certain match labels or expressions configuration. +func (nf *NamespaceFilter) IsAllowed(namespace string) bool { if nf.c == nil { log.Tracef("Allowing %q namespace as selector is nil", namespace) return true @@ -129,38 +116,27 @@ func (nf *NamespaceFilter) Close() error { // CachedNamespaceFilter is a wrapper of the NamespaceFilterer and the cache. type CachedNamespaceFilter struct { - NsFilter Filterer + NsFilter NamespaceFilterer cache storer.Storer } // NewCachedNamespaceFilter create a new CachedNamespaceFilter, wrapping the cache and the NamespaceFilterer. -func NewCachedNamespaceFilter(ns Filterer, storer storer.Storer) *CachedNamespaceFilter { +func NewCachedNamespaceFilter(ns NamespaceFilterer, storer storer.Storer) *CachedNamespaceFilter { return &CachedNamespaceFilter{ NsFilter: ns, cache: storer, } } -// Match check the cache and calls the underlying NamespaceFilter if the result is not found. -func (cnf *CachedNamespaceFilter) Match(metrics definition.RawMetrics) bool { - namespaceRaw, ok := metrics["namespace"] - if !ok { - return true - } - - namespace, ok := namespaceRaw.(string) - if !ok { - log.Tracef("Allowing %q from cache as namespace invalid type provided %t", namespaceRaw, namespaceRaw) - return true - } - +// IsAllowed check the cache and calls the underlying NamespaceFilter if the result is not found. +func (cnf *CachedNamespaceFilter) IsAllowed(namespace string) bool { // Check if the namespace is already in the cache. var allowed bool if _, err := cnf.cache.Get(namespace, &allowed); err == nil { return allowed } - allowed = cnf.NsFilter.Match(metrics) + allowed = cnf.NsFilter.IsAllowed(namespace) // Save the namespace in the cache. _ = cnf.cache.Set(namespace, allowed) diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go index 550f7b40f..e077564e1 100644 --- a/internal/discovery/namespace_filter_test.go +++ b/internal/discovery/namespace_filter_test.go @@ -8,7 +8,6 @@ import ( "github.com/newrelic/nri-kubernetes/v3/internal/config" "github.com/newrelic/nri-kubernetes/v3/internal/discovery" "github.com/newrelic/nri-kubernetes/v3/internal/storer" - "github.com/newrelic/nri-kubernetes/v3/src/definition" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -24,8 +23,6 @@ const namespaceName = "test_namespace" func TestNamespaceFilterer_IsAllowed(t *testing.T) { t.Parallel() - metrics := definition.RawMetrics{"namespace": namespaceName} - type testData struct { namespaceLabels labels.Set namespaceSelector config.NamespaceSelector @@ -157,7 +154,7 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { ns.Close() }) - require.Equal(t, testData.expected, ns.Match(metrics)) + require.Equal(t, testData.expected, ns.IsAllowed(namespaceName)) }) } } @@ -165,8 +162,6 @@ func TestNamespaceFilterer_IsAllowed(t *testing.T) { func TestNamespaceFilterer_Cache(t *testing.T) { t.Parallel() - metrics := definition.RawMetrics{"namespace": namespaceName} - type testData struct { warmCache func(cache *storer.InMemoryStore) prepare func(nsFilterMock *NamespaceFilterMock) @@ -178,10 +173,10 @@ func TestNamespaceFilterer_Cache(t *testing.T) { "namespace_cache_miss_fallback_to_call_informer": { warmCache: func(cache *storer.InMemoryStore) {}, prepare: func(nsFilterMock *NamespaceFilterMock) { - nsFilterMock.On("Match", metrics).Return(true).Once() + nsFilterMock.On("IsAllowed", namespaceName).Return(true).Once() }, assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { - require.Equal(t, expected, cnsf.Match(metrics)) + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) }, expected: true, }, @@ -190,10 +185,10 @@ func TestNamespaceFilterer_Cache(t *testing.T) { cache.Set(namespaceName, true) }, prepare: func(nsFilterMock *NamespaceFilterMock) { - nsFilterMock.AssertNotCalled(t, "Match") + nsFilterMock.AssertNotCalled(t, "IsAllowed") }, assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { - require.Equal(t, expected, cnsf.Match(metrics)) + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) }, expected: true, }, @@ -202,21 +197,21 @@ func TestNamespaceFilterer_Cache(t *testing.T) { cache.Set(namespaceName, false) }, prepare: func(nsFilterMock *NamespaceFilterMock) { - nsFilterMock.AssertNotCalled(t, "Match") + nsFilterMock.AssertNotCalled(t, "IsAllowed") }, assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { - require.Equal(t, expected, cnsf.Match(metrics)) + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) }, expected: false, }, "namespace_cache_miss_subsequent_call_uses_cache": { warmCache: func(cache *storer.InMemoryStore) {}, prepare: func(nsFilterMock *NamespaceFilterMock) { - nsFilterMock.On("Match", metrics).Return(true).Once() + nsFilterMock.On("IsAllowed", namespaceName).Return(true).Once() }, assert: func(expected bool, cnsf *discovery.CachedNamespaceFilter) { - require.Equal(t, expected, cnsf.Match(metrics)) - require.Equal(t, expected, cnsf.Match(metrics)) + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) + require.Equal(t, expected, cnsf.IsAllowed(namespaceName)) }, expected: true, }, @@ -272,7 +267,7 @@ func TestNamespaceFilter_InformerCacheSync(t *testing.T) { nil, ) // Check that recently created namespace is not allowed. - require.Equal(t, false, ns.Match(definition.RawMetrics{"namespace": namespaceName})) + require.Equal(t, false, ns.IsAllowed(namespaceName)) t.Cleanup(func() { cancel() @@ -289,7 +284,7 @@ func TestNamespaceFilter_InformerCacheSync(t *testing.T) { // Give some room to the informer to sync, and check that the new namespace is filtered properly. err = wait.PollImmediateUntilWithContext(ctx, 1*time.Second, func(context.Context) (bool, error) { - return ns.Match(definition.RawMetrics{"namespace": anotherNamespaceName}), nil + return ns.IsAllowed(anotherNamespaceName), nil }) require.NoError(t, err, "Timed out waiting for the informer to sync") } @@ -302,8 +297,8 @@ func newNamespaceFilterMock() *NamespaceFilterMock { return &NamespaceFilterMock{} } -func (ns *NamespaceFilterMock) Match(metrics definition.RawMetrics) bool { - args := ns.Called(metrics) +func (ns *NamespaceFilterMock) IsAllowed(namespace string) bool { + args := ns.Called(namespace) return args.Bool(0) } From 89b8e6004d8f746a9c2ce9cdaa28c96d7ca696e4 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Tue, 7 Jun 2022 17:55:59 +0200 Subject: [PATCH 14/18] feat: add namespace cache --- internal/discovery/namespace_cache.go | 90 +++++++++++++++++++++ internal/discovery/namespace_cache_test.go | 70 ++++++++++++++++ internal/discovery/namespace_filter.go | 30 +++---- internal/discovery/namespace_filter_test.go | 22 ++--- internal/storer/storer.go | 5 -- 5 files changed, 183 insertions(+), 34 deletions(-) create mode 100644 internal/discovery/namespace_cache.go create mode 100644 internal/discovery/namespace_cache_test.go diff --git a/internal/discovery/namespace_cache.go b/internal/discovery/namespace_cache.go new file mode 100644 index 000000000..70e225673 --- /dev/null +++ b/internal/discovery/namespace_cache.go @@ -0,0 +1,90 @@ +package discovery + +import ( + "sync" + "time" + + "github.com/sirupsen/logrus" +) + +const ( + // DefaultTTL is default ttl of the cache entries. + DefaultTTL = 10 * time.Minute + // DefaultInterval is default interval to execute the "garbage collection" of the cache. + DefaultInterval = 15 * time.Minute +) + +type cachedData map[string]bool + +// NamespaceCache provides an interface to add and retrieve namespaces from the cache. +type NamespaceCache interface { + Put(namespace string, match bool) + Match(namespace string) (bool, bool) +} + +type NamespaceInMemoryStore struct { + cache cachedData + locker *sync.RWMutex + logger *logrus.Logger + ttl time.Duration + lastVacuum time.Time + ticker *time.Ticker + stopChannel chan struct{} +} + +func NewNamespaceInMemoryStore(interval time.Duration, logger *logrus.Logger) *NamespaceInMemoryStore { + cm := &NamespaceInMemoryStore{ + cache: make(cachedData), + locker: &sync.RWMutex{}, + logger: logger, + // ticker interval should be slightly smaller than the integration interval. + ticker: time.NewTicker(interval), + stopChannel: make(chan struct{}), + } + + go func() { + for { + select { + case <-cm.ticker.C: + cm.vacuum() + case <-cm.stopChannel: + return + } + } + }() + + return cm +} + +func (m *NamespaceInMemoryStore) Put(namespace string, match bool) { + m.locker.Lock() + defer m.locker.Unlock() + + m.cache[namespace] = match +} + +func (m *NamespaceInMemoryStore) Match(namespace string) (bool, bool) { + m.locker.Lock() + defer m.locker.Unlock() + + match, found := m.cache[namespace] + + return match, found +} + +// StopVacuum Stops the goroutine in charge of the vacuum of the cache. +func (m *NamespaceInMemoryStore) StopVacuum() { + m.logger.Debugf("stopping namespace cache vacuum goroutine") + m.ticker.Stop() + close(m.stopChannel) +} + +// vacuum removes the cached data entries on each interval. +func (m *NamespaceInMemoryStore) vacuum() { + m.locker.Lock() + defer m.locker.Unlock() + + m.logger.Debugf("cleaning cache: len %d ...", len(m.cache)) + m.cache = make(cachedData) + m.logger.Debugf("cache cleaned: len %d ...", len(m.cache)) +} diff --git a/internal/discovery/namespace_cache_test.go b/internal/discovery/namespace_cache_test.go new file mode 100644 index 000000000..3c304a38e --- /dev/null +++ b/internal/discovery/namespace_cache_test.go @@ -0,0 +1,70 @@ +package discovery_test + +import ( + "testing" + "time" + + "github.com/newrelic/nri-kubernetes/v3/internal/discovery" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" +) + +const ( + testValue = true + testNewValue = false + testKey = "test_namespace" +) + +func Test_NamespaceCache(t *testing.T) { + t.Parallel() + + t.Run("is_set", func(t *testing.T) { + t.Parallel() + cache := discovery.NewNamespaceInMemoryStore(time.Second, logrus.New()) + + cache.Put(testKey, testValue) + match, found := cache.Match(testKey) + + require.Equal(t, testValue, match) + require.Equal(t, true, found) + + t.Run("and_overwritten", func(t *testing.T) { + cache.Put(testKey, testNewValue) + match, found = cache.Match(testKey) + + require.Equal(t, testNewValue, match) + require.Equal(t, true, found) + }) + + t.Run("and_after_interval_is_garbage_collected", func(t *testing.T) { + cache.Put(testKey, testValue) + time.Sleep(time.Second * 3) + _, found = cache.Match(testKey) + + require.Equal(t, false, found) + }) + }) + + t.Run("miss_returns_namespace_not_found", func(t *testing.T) { + t.Parallel() + + cache := discovery.NewNamespaceInMemoryStore(time.Second, logrus.New()) + match, found := cache.Match(testKey) + require.Equal(t, false, match) + require.Equal(t, false, found) + }) + + t.Run("does_not_delete_old_entries_if_stopped", func(t *testing.T) { + t.Parallel() + cache := discovery.NewNamespaceInMemoryStore(time.Second, logrus.New()) + cache.StopVacuum() + + for i := 0; i < 2; i++ { + cache.Put(testKey, testValue) + time.Sleep(time.Millisecond * 200) + match, _ := cache.Match(testKey) + + require.Equal(t, match, testValue) + } + }) +} diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index f723af287..cc2c92536 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -5,7 +5,6 @@ import ( "time" "github.com/newrelic/nri-kubernetes/v3/internal/config" - "github.com/newrelic/nri-kubernetes/v3/internal/storer" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" @@ -114,34 +113,29 @@ func (nf *NamespaceFilter) Close() error { return nil } -// CachedNamespaceFilter is a wrapper of the NamespaceFilterer and the cache. +// CachedNamespaceFilter holds a NamespaceCache around the NamespaceFilterer. type CachedNamespaceFilter struct { - NsFilter NamespaceFilterer - cache storer.Storer + cache NamespaceCache + filter NamespaceFilterer } // NewCachedNamespaceFilter create a new CachedNamespaceFilter, wrapping the cache and the NamespaceFilterer. -func NewCachedNamespaceFilter(ns NamespaceFilterer, storer storer.Storer) *CachedNamespaceFilter { +func NewCachedNamespaceFilter(filter NamespaceFilterer, cache NamespaceCache) *CachedNamespaceFilter { return &CachedNamespaceFilter{ - NsFilter: ns, - cache: storer, + filter: filter, + cache: cache, } } -// IsAllowed check the cache and calls the underlying NamespaceFilter if the result is not found. -func (cnf *CachedNamespaceFilter) IsAllowed(namespace string) bool { - // Check if the namespace is already in the cache. - var allowed bool - if _, err := cnf.cache.Get(namespace, &allowed); err == nil { - return allowed +func (cm *CachedNamespaceFilter) IsAllowed(namespace string) bool { + if match, found := cm.cache.Match(namespace); found { + return match } - allowed = cnf.NsFilter.IsAllowed(namespace) + match := cm.filter.IsAllowed(namespace) + cm.cache.Put(namespace, match) - // Save the namespace in the cache. - _ = cnf.cache.Set(namespace, allowed) - - return allowed + return match } // containsNamespace checks if a namespaces is contained in a given list of namespaces. diff --git a/internal/discovery/namespace_filter_test.go b/internal/discovery/namespace_filter_test.go index e077564e1..df48d59ab 100644 --- a/internal/discovery/namespace_filter_test.go +++ b/internal/discovery/namespace_filter_test.go @@ -7,7 +7,7 @@ import ( "github.com/newrelic/nri-kubernetes/v3/internal/config" "github.com/newrelic/nri-kubernetes/v3/internal/discovery" - "github.com/newrelic/nri-kubernetes/v3/internal/storer" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -163,7 +163,7 @@ func TestNamespaceFilterer_Cache(t *testing.T) { t.Parallel() type testData struct { - warmCache func(cache *storer.InMemoryStore) + warmCache func(c *discovery.NamespaceInMemoryStore) prepare func(nsFilterMock *NamespaceFilterMock) assert func(expected bool, cnsf *discovery.CachedNamespaceFilter) expected bool @@ -171,7 +171,7 @@ func TestNamespaceFilterer_Cache(t *testing.T) { testCases := map[string]testData{ "namespace_cache_miss_fallback_to_call_informer": { - warmCache: func(cache *storer.InMemoryStore) {}, + warmCache: func(c *discovery.NamespaceInMemoryStore) {}, prepare: func(nsFilterMock *NamespaceFilterMock) { nsFilterMock.On("IsAllowed", namespaceName).Return(true).Once() }, @@ -181,8 +181,8 @@ func TestNamespaceFilterer_Cache(t *testing.T) { expected: true, }, "namespace_already_in_cache_allowed": { - warmCache: func(cache *storer.InMemoryStore) { - cache.Set(namespaceName, true) + warmCache: func(c *discovery.NamespaceInMemoryStore) { + c.Put(namespaceName, true) }, prepare: func(nsFilterMock *NamespaceFilterMock) { nsFilterMock.AssertNotCalled(t, "IsAllowed") @@ -193,8 +193,8 @@ func TestNamespaceFilterer_Cache(t *testing.T) { expected: true, }, "namespace_already_in_cache_not_allowed": { - warmCache: func(cache *storer.InMemoryStore) { - cache.Set(namespaceName, false) + warmCache: func(c *discovery.NamespaceInMemoryStore) { + c.Put(namespaceName, false) }, prepare: func(nsFilterMock *NamespaceFilterMock) { nsFilterMock.AssertNotCalled(t, "IsAllowed") @@ -205,7 +205,7 @@ func TestNamespaceFilterer_Cache(t *testing.T) { expected: false, }, "namespace_cache_miss_subsequent_call_uses_cache": { - warmCache: func(cache *storer.InMemoryStore) {}, + warmCache: func(c *discovery.NamespaceInMemoryStore) {}, prepare: func(nsFilterMock *NamespaceFilterMock) { nsFilterMock.On("IsAllowed", namespaceName).Return(true).Once() }, @@ -224,13 +224,13 @@ func TestNamespaceFilterer_Cache(t *testing.T) { nsFilterMock := newNamespaceFilterMock() - cache := storer.NewInMemoryStore(storer.DefaultTTL, storer.DefaultInterval, nil) - testData.warmCache(cache) + c := discovery.NewNamespaceInMemoryStore(discovery.DefaultInterval, logrus.New()) + testData.warmCache(c) testData.prepare(nsFilterMock) cnsf := discovery.NewCachedNamespaceFilter( nsFilterMock, - cache, + c, ) testData.assert(testData.expected, cnsf) diff --git a/internal/storer/storer.go b/internal/storer/storer.go index 766b34725..65803eeab 100644 --- a/internal/storer/storer.go +++ b/internal/storer/storer.go @@ -19,11 +19,6 @@ const ( DefaultInterval = 15 * time.Minute ) -type Storer interface { - Set(key string, value interface{}) int64 - Get(key string, valuePtr interface{}) (int64, error) -} - // InMemoryStore is similar to the sdk one, the main difference is cleanCache method executed each interval. type InMemoryStore struct { cachedData map[string]jsonEntry From c28d9bd4f0faa4815221090d323763d6eb91d3c8 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Tue, 7 Jun 2022 21:24:36 +0200 Subject: [PATCH 15/18] refactor: update expression String() to return error --- internal/config/config.go | 8 +++----- internal/discovery/namespace_filter.go | 9 ++++++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 2da1431b0..875bb9323 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,7 +6,6 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" "github.com/spf13/viper" ) @@ -242,7 +241,7 @@ type Expression struct { Values []interface{} `mapstructure:"values"` } -func (e *Expression) String() string { +func (e *Expression) String() (string, error) { var values []string for _, val := range e.Values { @@ -257,14 +256,13 @@ func (e *Expression) String() string { case float32, float64: str = fmt.Sprintf("%f", v) default: - log.Errorf("parsing expression value: %v, type %v", val, v) - continue + return "", fmt.Errorf("parsing expression invalid value: %v, type: %T", val, v) } values = append(values, str) } - return fmt.Sprintf("%s %s (%s)", e.Key, strings.ToLower(e.Operator), strings.Join(values, ",")) + return fmt.Sprintf("%s %s (%s)", e.Key, strings.ToLower(e.Operator), strings.Join(values, ",")), nil } func LoadConfig(filePath string, fileName string) (*Config, error) { diff --git a/internal/discovery/namespace_filter.go b/internal/discovery/namespace_filter.go index cc2c92536..7b6ae1f13 100644 --- a/internal/discovery/namespace_filter.go +++ b/internal/discovery/namespace_filter.go @@ -82,7 +82,13 @@ func (nf *NamespaceFilter) matchNamespaceByLabels(namespace string) bool { // matchNamespaceByExpressions filters a namespace using the selector from the MatchExpressions config. func (nf *NamespaceFilter) matchNamespaceByExpressions(namespace string) bool { for _, expression := range nf.c.MatchExpressions { - selector, err := labels.Parse(expression.String()) + val, err := expression.String() + if err != nil { + nf.logger.Error(err) + return true + } + + selector, err := labels.Parse(val) if err != nil { nf.logger.Errorf("parsing labels: %v", err) return true @@ -127,6 +133,7 @@ func NewCachedNamespaceFilter(filter NamespaceFilterer, cache NamespaceCache) *C } } +// IsAllowed looks for a match in the cache first, otherwise calls the filter. func (cm *CachedNamespaceFilter) IsAllowed(namespace string) bool { if match, found := cm.cache.Match(namespace); found { return match From 41e75891a250c10f062d391f5218e08850cb941c Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Wed, 8 Jun 2022 10:15:42 +0200 Subject: [PATCH 16/18] refactor: remove unused cache ttl --- internal/discovery/namespace_cache.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/discovery/namespace_cache.go b/internal/discovery/namespace_cache.go index 70e225673..5c42274c8 100644 --- a/internal/discovery/namespace_cache.go +++ b/internal/discovery/namespace_cache.go @@ -8,8 +8,6 @@ import ( ) const ( - // DefaultTTL is default ttl of the cache entries. - DefaultTTL = 10 * time.Minute // DefaultInterval is default interval to execute the "garbage collection" of the cache. DefaultInterval = 15 * time.Minute ) @@ -26,7 +24,6 @@ type NamespaceInMemoryStore struct { cache cachedData locker *sync.RWMutex logger *logrus.Logger - ttl time.Duration lastVacuum time.Time ticker *time.Ticker stopChannel chan struct{} From e2fe8a97006e96411e5a4c35d774fb788a004e19 Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Wed, 8 Jun 2022 15:09:32 +0200 Subject: [PATCH 17/18] docs: add comment in values.aml for namespaceSelector --- charts/newrelic-infrastructure/values.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/charts/newrelic-infrastructure/values.yaml b/charts/newrelic-infrastructure/values.yaml index 55bada047..6f812e017 100644 --- a/charts/newrelic-infrastructure/values.yaml +++ b/charts/newrelic-infrastructure/values.yaml @@ -51,6 +51,13 @@ common: interval: # -- Config for filtering ksm and kubelet metrics by namespace. namespaceSelector: {} + # If you want to include only namespaces with a given label you could do so by adding: + # matchLabels: + # newrelic.com/scrape: true + # Otherwhise you can build more complex filters and include or exclude certain namespaces by adding one or multiple + # expressions that are anded, for instance: + # matchExpressions: + # - {key: newrelic.com/scrape, operator: NotIn, values: [false]} # -- Config for the Infrastructure agent. # Will be used by the forwarder sidecars and the agent running integrations. From 848c22e910a9f335fd74d360dbacf6c939b4a5bb Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Wed, 8 Jun 2022 15:14:10 +0200 Subject: [PATCH 18/18] fix: comment typo --- charts/newrelic-infrastructure/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/newrelic-infrastructure/values.yaml b/charts/newrelic-infrastructure/values.yaml index 6f812e017..c28f21424 100644 --- a/charts/newrelic-infrastructure/values.yaml +++ b/charts/newrelic-infrastructure/values.yaml @@ -54,7 +54,7 @@ common: # If you want to include only namespaces with a given label you could do so by adding: # matchLabels: # newrelic.com/scrape: true - # Otherwhise you can build more complex filters and include or exclude certain namespaces by adding one or multiple + # Otherwise you can build more complex filters and include or exclude certain namespaces by adding one or multiple # expressions that are anded, for instance: # matchExpressions: # - {key: newrelic.com/scrape, operator: NotIn, values: [false]}