[Metricbeat][Kubernetes Volume] Add pvc reference to distinguish ephemeral from persistent volumes by herrBez · Pull Request #38839 · elastic/beats

herrBez · 2024-04-10T15:45:00Z

Proposed commit message

[Metricbeat][Kubernetes Volume] Add pvc reference to distinguish ephemeral from persistent volumes

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.

Author's Checklist

[ ]

How to test this PR locally

Create a PVC and mount the PVC as a volume in a pod, e.g. (on GKE):

---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: pvc-demo
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 1Gi
  storageClassName: standard-rwo
---
kind: Pod
apiVersion: v1
metadata:
  name: pod-demo
spec:
  volumes:
    - name: pvc-demo-vol
      persistentVolumeClaim:
       claimName: pvc-demo
  containers:
    - name: pod-demo
      image: nginx
      resources:
        limits:
          cpu: 10m
          memory: 80Mi
        requests:
          cpu: 10m
          memory: 80Mi
      ports:
        - containerPort: 80
          name: "http-server"
      volumeMounts:
        - mountPath: "/usr/share/nginx/html"
          name: pvc-demo-vol

Deploy metricbeat and check that there exist documents in kubernetes.volume with the field kubernetes.persistentvolumeclaim.name.

Alternatively execute the test.

Related issues

Fixes Add pvc name and namespace to kubernetes volume metricset #6977

Use cases

The use-case is to being able to monitor persistent volumes and ignoring ephemeral volumes for threshold alerts.

As of now there is no way to distinguish persistent from ephemeral volumes. However, the endpoint called by the module $NODE_URL/summary/stats does actually report a pvcReference that allows to bind the pod's volume with a corresponding pvc.

Example Response:

...
 "volume": [
        {
          "time": "2024-04-09T17:34:17Z",
          "availableBytes": 31509590016,
          "capacityBytes": 31526391808,
          "usedBytes": 24576,
          "inodesFree": 1966069,
          "inodes": 1966080,
          "inodesUsed": 11,
          "name": "pvc-demo-vol",
          "pvcRef": {
            "name": "pvc-demo",
            "namespace": "default"
          }
        },
        {
          "time": "2024-04-09T17:34:17Z",
          "availableBytes": 83873792,
          "capacityBytes": 83886080,
          "usedBytes": 12288,
          "inodesFree": 502853,
          "inodes": 502862,
          "inodesUsed": 9,
          "name": "kube-api-access-h2cll"
        }
     ]
...

Screenshots

Logs

…w one

…nt with the kubernetes.namespace)

mergify · 2024-04-10T15:45:43Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @herrBez? 🙏.
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

elasticmachine · 2024-04-10T16:40:20Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 101 min 10 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

herrBez · 2024-04-17T14:46:14Z

cc @elastic/obs-ds-hosted-services

tetianakravchenko

could you please also add a changelog recorf?

tetianakravchenko · 2024-05-02T15:25:00Z

metricbeat/module/kubernetes/_meta/test/stats_summary.json

      "inodes": 473560,
      "inodesUsed": 9,
-      "name": "default-token-sg8x5"
+      "name": "default-token-sg8x5",


can you please add a separate sample with the pvcRef to keep in the test file both examples - one for persistentvolumeclaim and one for the ephemeral volume?

tetianakravchenko · 2024-05-02T15:30:26Z

a new field also should be added in the fields file: https://github.com/elastic/beats/blob/main/metricbeat/module/kubernetes/volume/_meta/fields.yml

herrBez · 2024-05-02T16:20:15Z

Hi,

I tried to address the raised point about the tests^^. I also tried specifying the field. What I am not sure about is that in the end I want to "reuse" kubernetes.persistentvolumeclaim.name which is part of the kubernetes.state_persistentvolumeclaims dataset. My doubt comes from the fact that we did not specify kubernetes.pod.name. WDYT?

herrBez · 2024-05-03T06:43:42Z

Apparently, adding the field broke the pipeline with the following error:

[2024-05-02T16:39:21.927Z] E Exception: export command returned with an error: Error generating Index Pattern: field <kubernetes.persistentvolumeclaim.name> is duplicated, remove it or set 'overwrite: true', {Name:name Type:keyword Description:PVC name. Format: Fields:[] MultiFields:[] Enabled: Analyzer:{Name: Definition:} SearchAnalyzer:{Name: Definition:} Norms:false Dynamic:{Value:} Index: DocValues: CopyTo: IgnoreAbove:0 AliasPath: MigrationAlias:false Dimension: DynamicTemplate:false Unit: MetricType: ObjectType: ObjectTypeMappingType: ScalingFactor:0 ObjectTypeParams:[] Analyzed: Count:0 Searchable: Aggregatable: Script: Pattern: InputFormat: OutputFormat: OutputPrecision: LabelTemplate: UrlTemplate:[] OpenLinkInCurrentTab: Overwrite:false DefaultField: Path:kubernetes.persistentvolumeclaim.name}, {"aggregatable":true,"analyzed":false,"count":0,"doc_values":true,"indexed":true,"name":"kubernetes.persistentvolumeclaim.name","scripted":false,"searchable":true,"type":"string"}.

MichaelKatsoulis · 2024-05-08T08:30:22Z

Apparently, adding the field broke the pipeline with the following error:

@herrBez in beats, the different kubernetes metricsets share the same index and mappings, so if the field is already declared, you don't need to declare it again. There is a duplicate key.
In integrations repo, when this field will be also added, then you need to add it kubernetes.volume data stream as well. The reason is that in agent each data stream has its own index and hence mappings.

…umeclaim.name

mergify · 2024-05-08T13:58:07Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b kubernetes-pvc-volume upstream/kubernetes-pvc-volume
git merge upstream/main
git push upstream kubernetes-pvc-volume

herrBez · 2024-05-08T14:55:41Z

Hi Michael, I addressed the required changes and the build works fine

MichaelKatsoulis · 2024-05-09T12:07:27Z

@herrBez Can you add a screenshot showing the new field being populated to Elasticsearch? You can use a view from discovery filtering by the kubernetes.volume metricset and showing the new field.

MichaelKatsoulis · 2024-05-09T12:18:18Z

metricbeat/module/kubernetes/volume/data.go

 			}
+			if volume.PvcRef.Name != "" {
+				kubernetes2.ShouldPut(volumeEvent, "persistentvolumeclaim.name", volume.PvcRef.Name, logger)
+			}


I have the feeling that this way the field that will be generated will be kubernetes.volume.persistentvolumeclaim.name . Is that what you want?

Wow, great static analysis :D.. Indeed, its creating the field you mention. I actually want to reuse the field kubernetes.persistentvolumeclaim.name to allow correlation, but I am honestly a little bit lost.

After some thinking I tried with:

kubernetes2.ShouldPut(volumeEvent, mb.ModuleDataKey+".persistentvolumeclaim.name", volume.PvcRef.Name, logger)

But it does not seem to work 🤔

Ok, the line I wrote in the comment is correct. For some reasons I were still using the old binary 🤦

I had the feeling 😂

…es.persistentvolume

herrBez · 2024-05-10T18:25:02Z

Here is the screenshot with the new version:

… kubernetes-pvc-volume

herrBez · 2024-05-14T15:34:08Z

Can I go ahead and merge it? Should I open a PR in the integration repository to add the field to the kubernetes.volume datastream?

MichaelKatsoulis · 2024-05-16T06:46:21Z

Can I go ahead and merge it? Should I open a PR in the integration repository to add the field to the kubernetes.volume datastream?

I will merge it. About the field in the integration, not yet. We do it after the release of beats. I will add this in our list cause we have some other fields to declare as well.

herrBez and others added 5 commits April 9, 2024 19:49

Kubernetes add pvc reference in the kubernetes.volume dataset if present

5b4a487

Reuse field kubernetes.persistentvolumeclaim instead of creating a ne…

ede64ae

…w one

Add only persistentvolumeclaim name and no namespace (that is redunda…

5026f50

…nt with the kubernetes.namespace)

Merge branch 'elastic:main' into kubernetes-pvc-volume

375e742

Add test to see if persistentvolumeclaim name is extracted

b0dd8a4

herrBez requested a review from a team as a code owner April 10, 2024 15:45

herrBez requested review from gsantoro and tetianakravchenko April 10, 2024 15:45

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 10, 2024

mergify bot assigned herrBez Apr 10, 2024

herrBez added the Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team label Apr 10, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 10, 2024

herrBez mentioned this pull request Apr 16, 2024

[Kubernetes Integration] Volume dashboard show if volume is persistent and to which pod it belongs elastic/integrations#9612

Closed

tetianakravchenko removed the request for review from gsantoro May 2, 2024 15:17

tetianakravchenko reviewed May 2, 2024

View reviewed changes

tetianakravchenko requested review from a team, MichaelKatsoulis and gizas and removed request for a team May 2, 2024 15:25

herrBez and others added 5 commits May 2, 2024 18:00

Add a separate test for pvc

d5cfb1f

Merge branch 'main' into kubernetes-pvc-volume

e1ba07b

Add changelog entry for the PR

0f78c41

Add the persistentvolume claim field

9a0cf91

Run mage update to update the fields

698eb17

Remove the duplicate definition of the field kubernetes.persistentvol…

39d255e

…umeclaim.name

Remove newline

8a6eeff

MichaelKatsoulis reviewed May 9, 2024

View reviewed changes

Fix key from kubernetes.volume.persistentvolumeclaim.name to kubernet…

a8b3092

…es.persistentvolume

Merge branch 'main' into kubernetes-pvc-volume

d206ac5

MichaelKatsoulis approved these changes May 13, 2024

View reviewed changes

gizas approved these changes May 13, 2024

View reviewed changes

herrBez and others added 5 commits May 13, 2024 16:44

Merge branch 'main' into kubernetes-pvc-volume

9e9a600

Fix unit test after changing the populated field name

cec10b2

Merge branch 'kubernetes-pvc-volume' of github.com:herrBez/beats into…

14cff40

… kubernetes-pvc-volume

Removed printf function

ef19ff0

Remove empty lines added by mistake

835d991

MichaelKatsoulis merged commit 9b338cd into elastic:main May 16, 2024

Conversation

herrBez commented Apr 10, 2024

Proposed commit message

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Uh oh!

mergify bot commented Apr 10, 2024

Uh oh!

elasticmachine commented Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

❕ Flaky test report

🤖 GitHub comments

Uh oh!

herrBez commented Apr 17, 2024

Uh oh!

tetianakravchenko left a comment

Choose a reason for hiding this comment

Uh oh!

tetianakravchenko May 2, 2024

Choose a reason for hiding this comment

Uh oh!

tetianakravchenko commented May 2, 2024

Uh oh!

herrBez commented May 2, 2024

Uh oh!

herrBez commented May 3, 2024

Uh oh!

MichaelKatsoulis commented May 8, 2024

Uh oh!

mergify bot commented May 8, 2024

Uh oh!

herrBez commented May 8, 2024

Uh oh!

MichaelKatsoulis commented May 9, 2024

Uh oh!

MichaelKatsoulis May 9, 2024

Choose a reason for hiding this comment

Uh oh!

herrBez May 10, 2024

Choose a reason for hiding this comment

Uh oh!

herrBez May 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelKatsoulis May 13, 2024

Choose a reason for hiding this comment

Uh oh!

herrBez commented May 10, 2024

Uh oh!

herrBez commented May 14, 2024

Uh oh!

MichaelKatsoulis commented May 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

elasticmachine commented Apr 10, 2024 •

edited

Loading

herrBez May 10, 2024 •

edited

Loading