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

[Kubernetes Provider] Apply namespace filter to watchers #39881

Merged
merged 3 commits into from
Jun 13, 2024

Conversation

constanca-m
Copy link
Contributor

@constanca-m constanca-m commented Jun 12, 2024

Proposed commit message

I also added the option HonorReSyncs to make sure we don't lose any events.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Run your own metricbeat as stated here. The way I tested this change is better explained in Results section.

Related issues

Results

I have a cluster with the following resources:

c@c:~/$ kubectl get pods --all-namespaces
NAMESPACE            NAME                                         READY   STATUS    RESTARTS   AGE
default              nginx                                        1/1     Running   0          14s
default              redis-deployment-648bf5585b-2dcr2            1/1     Running   0          14s
default              redis-deployment-648bf5585b-ll9nd            1/1     Running   0          14s
default              redis-deployment-648bf5585b-w96c2            1/1     Running   0          14s
kube-system          coredns-5d78c9869d-xn6x6                     1/1     Running   0          46m
kube-system          coredns-5d78c9869d-ztcd8                     1/1     Running   0          46m
kube-system          etcd-kind-control-plane                      1/1     Running   0          47m
kube-system          kindnet-968kl                                1/1     Running   0          46m
kube-system          kube-apiserver-kind-control-plane            1/1     Running   0          47m
kube-system          kube-controller-manager-kind-control-plane   1/1     Running   0          47m
kube-system          kube-proxy-wlcfk                             1/1     Running   0          46m
kube-system          kube-scheduler-kind-control-plane            1/1     Running   0          47m
kube-system          kube-state-metrics-5f89fb6d84-dtxwd          1/1     Running   0          46m
kube-system          metricbeat-qdwn6                             1/1     Running   0          10m
local-path-storage   local-path-provisioner-6bc4bddd6b-cmvxd      1/1     Running   0          46m

If I apply namespace: default, I expect to only see pods from that namespace. I have deployed 3 resources in the default namespace: a cronjob, a deployment and a pod (to test all three watchers).

The resources manifest used is this one.
apiVersion: v1
kind: Pod
metadata:
  name: nginx
spec:
  containers:
  - name: nginx
    image: nginx:1.14.2
    ports:
    - containerPort: 80
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: redis-deployment
  labels:
    app: redis
spec:
  replicas: 3
  selector:
    matchLabels:
      app: redis
  template:
    metadata:
      labels:
        app: redis
    spec:
      containers:
      - name: redis
        image: redis:latest
        ports:
        - containerPort: 6379
        resources:
          requests:
            memory: "64Mi"
            cpu: "250m"
          limits:
            memory: "128Mi"
            cpu: "500m"
---
apiVersion: batch/v1
kind: CronJob
metadata:
  name: hello-world-cronjob
spec:
  schedule: "*/5 * * * *"  # This schedule runs the job every 5 minutes
  jobTemplate:
    spec:
      template:
        spec:
          containers:
          - name: hello
            image: busybox
            args:
            - /bin/sh
            - -c
            - echo "Hello, World!"
          restartPolicy: OnFailure

Filtering by namespace

I set `namespace: default` in the Kubernetes provider like this.
    metricbeat.autodiscover:
      providers:
        - type: kubernetes
          scope: cluster
          unique: false
          namespace: default
          add_resource_metadata:
            deployment: true
            cronjob: true
          templates:
            - config:
                - module: kubernetes
                  metricsets:
                    - pod
                  period: 10s
                  host: ${NODE_NAME}
                  hosts: ["https://${NODE_NAME}:10250"]
                  bearer_token_file: /var/run/secrets/kubernetes.io/serviceaccount/token
                  ssl.verification_mode: "none"

I left my metricbeat running for a while. Then I checked how many pods were emitting events and from which namespaces they come from:

Screenshot from 2024-06-12 15-53-29

They all come from default namespace as expected.

Not filtering by namespace

I now removed the namespace: default from my kubernetes provider.

I let metricbeat run for a while. This time I can see:

image

We receive events from all namespaces, since it is not filtered.

Signed-off-by: constanca <constanca.manteigas@elastic.co>
@constanca-m constanca-m added the Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team label Jun 12, 2024
@constanca-m constanca-m requested a review from a team June 12, 2024 14:21
@constanca-m constanca-m self-assigned this Jun 12, 2024
@constanca-m constanca-m requested a review from a team as a code owner June 12, 2024 14:21
@constanca-m constanca-m requested review from faec and leehinman June 12, 2024 14:21
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jun 12, 2024
Copy link
Contributor

mergify bot commented Jun 12, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @constanca-m? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

Signed-off-by: constanca <constanca.manteigas@elastic.co>
@constanca-m constanca-m requested review from a team, gizas and tetianakravchenko and removed request for a team June 12, 2024 14:23
@pierrehilbert pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jun 12, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

Signed-off-by: constanca <constanca.manteigas@elastic.co>
@@ -196,7 +193,7 @@ Example:

`unique`:: (Optional) Defaults to `false`. Marking an autodiscover provider as unique results into
making the provider to enable the provided templates only when it will gain the leader lease.
This setting can only be combined with `cluster` scope. When `unique` is enabled enabled, `resource`
This setting can only be combined with `cluster` scope. When `unique` is enabled, `resource`
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related with this PR but what do we mean when we say here that with unique: true the add_resource_metadata settings are not taken into account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If unique: true then we are not using watchers, I think that's why they are not taken into account.

Namespace: config.Namespace,
SyncTimeout: config.SyncPeriod,
Namespace: config.Namespace,
HonorReSyncs: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we need to make this configurable?
I dont find a place where we have documented the use of resyncs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is just for devs, indeed.

@constanca-m constanca-m merged commit 2f91cf1 into elastic:main Jun 13, 2024
114 checks passed
@constanca-m constanca-m deleted the k8s-provider-add-namespace branch June 13, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants