Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create the config and namespace filterer #457

Merged
merged 19 commits into from
Jun 8, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/newrelic-infrastructure/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ tests:
path: data.nri-kubernetes\.yml
value: |-
interval: 15s
namespaceSelector: {}
controlPlane:
retries: 3
timeout: 10s
Expand All @@ -38,6 +39,7 @@ tests:
path: data.nri-kubernetes\.yml
value: |-
interval: 15s
namespaceSelector: {}
controlPlane:
retries: 3
timeout: 10s
Expand Down Expand Up @@ -71,6 +73,7 @@ tests:
path: data.nri-kubernetes\.yml
value: |-
interval: 15s
namespaceSelector: {}
controlPlane:
retries: 3
timeout: 10s
Expand Down Expand Up @@ -142,6 +145,7 @@ tests:
path: data.nri-kubernetes\.yml
value: |-
interval: 15s
namespaceSelector: {}
controlPlane:
retries: 3
timeout: 10s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ tests:
path: data.nri-kubernetes\.yml
value: |-
interval: 15s
namespaceSelector: {}
ksm:
enabled: true
port: 22
Expand All @@ -52,6 +53,7 @@ tests:
path: data.nri-kubernetes\.yml
value: |-
interval: 15s
namespaceSelector: {}
ksm:
enabled: true
retries: 3
Expand All @@ -70,6 +72,7 @@ tests:
path: data.nri-kubernetes\.yml
value: |-
interval: 15s
namespaceSelector: {}
ksm:
enabled: true
port: 22
Expand All @@ -94,6 +97,7 @@ tests:
path: data.nri-kubernetes\.yml
value: |-
interval: 15s
namespaceSelector: {}
ksm:
distributed: true
enabled: true
Expand Down
3 changes: 3 additions & 0 deletions charts/newrelic-infrastructure/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
marcsanmi marked this conversation as resolved.
Show resolved Hide resolved
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/
Expand Down
81 changes: 66 additions & 15 deletions internal/config/config.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package config

import (
"fmt"
"strconv"
"strings"
"time"

log "github.com/sirupsen/logrus"
"github.com/spf13/viper"
)

Expand Down Expand Up @@ -53,6 +56,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.
Expand Down Expand Up @@ -218,38 +224,83 @@ 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"`
}

// 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"`
marcsanmi marked this conversation as resolved.
Show resolved Hide resolved
// 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)
marcsanmi marked this conversation as resolved.
Show resolved Hide resolved
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) {
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)
Expand Down
9 changes: 9 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
4 changes: 4 additions & 0 deletions internal/config/testdata/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ clusterName: dummy_cluster
interval: 15
verbose: true

namespaceSelector:
matchLabels:
newrelic.com/scrape: true

sink:
http:
port: 8081
Expand Down
109 changes: 109 additions & 0 deletions internal/discovery/namespace_filter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package discovery

import (
"errors"

"github.com/newrelic/nri-kubernetes/v3/internal/config"

log "github.com/sirupsen/logrus"
marcsanmi marked this conversation as resolved.
Show resolved Hide resolved
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
marcsanmi marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this interface is too specific. What would be the advantage of having a filter specific to namespaces, as opposed to a filter that takes in fetch.RawMetrics and returns a bool? NamespaceFilter could be a particular implementation of this, looking for metrics["namespace"]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I updated the interface definition and implementation here e57ba69.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After talking with @alvarocabanas and @sigilioso, we decided to move back again to the initial definition. Implementing the solution suggested above would imply some refactor in order to avoid cyclic dependencies between definition and discovery packages and to make the code maintainable.

Done in 95f8b9d.

We can talk about this when you come back 🙂


// NamespaceFilter is a struct holding pointers to the config and the namespace lister.
type NamespaceFilter struct {
c *config.Config
marcsanmi marked this conversation as resolved.
Show resolved Hide resolved
lister listersv1.NamespaceLister
stopCh chan<- struct{}
}

// NewNamespaceFilter inits the namespace lister and returns a new NamespaceFilter.
marcsanmi marked this conversation as resolved.
Show resolved Hide resolved
func NewNamespaceFilter(c *config.Config, client kubernetes.Interface, options ...informers.SharedInformerOption) *NamespaceFilter {
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: stopCh,
}
}

// IsAllowed checks given any namespace, if it's allowed to be scraped by using the NamespaceLister
func (nf *NamespaceFilter) IsAllowed(namespace string) bool {
marcsanmi marked this conversation as resolved.
Show resolved Hide resolved
// 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 true
}

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 true
}

namespaceList, err := nf.lister.List(selector)
if err != nil {
log.Errorf("listing namespaces with MatchExpressions: %v", err)
return true
}

if !containsNamespace(namespace, namespaceList) {
return false
}
}
}

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 {
if n.Name == namespace {
return true
}
}

return false
}