Fix bug with override path in cgroups by fearful-symmetry · Pull Request #27620 · elastic/beats

fearful-symmetry · 2021-08-26T21:35:03Z

What does this PR do?

This is a fix for #27598

In cases where cgroupsHierarchyOverride is set to /, and ignoreRootCgroups is set, we'll just completely bypass any cgroups reporting.

Why is it important?

It's a bug that was breaking containers.

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

Build metricbeat (or any other beat) for docker
run on linux
set http.enabled to true in the beat config
From inside the docker container, do curl localhost:5066/stats?pretty
Make sure cgroup metrics are reported.

Related issues

Fixes libbeat/cmd/instance: cgroup metrics no longer being reported #27598

elasticmachine · 2021-08-26T21:35:06Z

Pinging @elastic/integrations (Team:Integrations)

elasticmachine · 2021-08-27T00:17:52Z

💚 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

Start Time: 2021-08-31T02:58:21.884+0000
Duration: 148 min 56 sec
Commit: 6f50223

Test stats 🧪

Test	Results
Failed	0
Passed	53721
Skipped	5325
Total	59046

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	53721
Skipped	5325
Total	59046

axw

Thanks @fearful-symmetry!

matschaffer · 2021-08-27T03:32:23Z

Is there a handy place I can snag the linux binary? I should be able to test it on my cloud deployment.

ruflin · 2021-08-30T07:53:07Z

@matschaffer Not before the merge of the PR. But I can build it for you or tell you how to build it ;-)

…verride-logic-fix

fearful-symmetry · 2021-08-30T18:20:10Z

/test

mergify · 2021-08-30T21:04:20Z

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 cgroupsHierarchyOverride-logic-fix upstream/cgroupsHierarchyOverride-logic-fix
git merge upstream/master
git push upstream cgroupsHierarchyOverride-logic-fix

…verride-logic-fix

matschaffer · 2021-08-31T00:44:08Z

@ruflin either way. Pretty sure you also have all the access you need to try to squeeze a build into a test deployment. Since @axw approved maybe we can just wait for the next BC.

axw · 2021-08-31T01:33:51Z

FWIW, I tested this with this small program - ran it before/after changes in a Docker container:

package main

import (
        "fmt"
        "log"

        "github.com/elastic/beats/v7/libbeat/cmd/instance/metrics"
        "github.com/elastic/beats/v7/libbeat/monitoring"
)

func main() {
        if err := metrics.SetupMetrics("testing"); err != nil {
                log.Fatal(err)
        }
        snapshot := monitoring.CollectFlatSnapshot(monitoring.Default, monitoring.Full, false)
        for name, value := range snapshot.Ints {
                fmt.Println(name, value)
        }
}

I think we're good, and can just wait for the next BC after this merges.

fearful-symmetry · 2021-08-31T01:53:52Z

Yep, sorry for the delay, once again fighting with CI...

fearful-symmetry · 2021-08-31T02:57:55Z

/test

fearful-symmetry · 2021-08-31T06:05:14Z

FINALLY

* fix bug with override path * add changelog (cherry picked from commit f073c45)

* fix bug with override path * add changelog (cherry picked from commit f073c45) Co-authored-by: Alex K <8418476+fearful-symmetry@users.noreply.github.com>

* master: Forward port 7.14.1 changelog to master (elastic#27687) Addressing multiple dashboard issues: deps loading once, field conversion, etc. (elastic#27669) Remove adaptive queue sizes from agent's spec files (elastic#27653) Osquerybeat: Improve testability and unit test coverage (elastic#27591) Osquerybeat: lockdown flagsfile, prevent global defaults (elastic#27611) Import the references of dashboard assets using the Saved Objects API (elastic#27647) Fix bug with override path in cgroups (elastic#27620) Allow Kibana client to authorize with Elasticsearch API key (elastic#27540) Filebeat auditd: Fix Top Exec Commands dashboard visualization (elastic#27638) [elastic-agent] Fix docker tar.gz generation for complete image (elastic#27621) Follow up changes in dashboards in mage check && fix minor issue (elastic#27553) [Heartbeat] Fix bug where `enabled: false` is ignored. (elastic#27615) Support kube_state_metrics v2.0.0 (elastic#27552)

* fix bug with override path * add changelog

fix bug with override path

55fbd2c

fearful-symmetry added bug Team:Integrations Label for the Integrations team backport-v7.15.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify labels Aug 26, 2021

fearful-symmetry requested review from a team and axw August 26, 2021 21:35

fearful-symmetry self-assigned this Aug 26, 2021

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 Aug 26, 2021

axw approved these changes Aug 27, 2021

View reviewed changes

fearful-symmetry added 2 commits August 30, 2021 08:58

Merge remote-tracking branch 'upstream/master' into cgroupsHierarchyO…

78e7513

…verride-logic-fix

add changelog

d170380

Merge remote-tracking branch 'upstream/master' into cgroupsHierarchyO…

6f50223

…verride-logic-fix

fearful-symmetry merged commit f073c45 into elastic:master Aug 31, 2021

mergify bot mentioned this pull request Aug 31, 2021

[7.x](backport #27620) Fix bug with override path in cgroups #27660

Merged

mergify bot pushed a commit that referenced this pull request Aug 31, 2021

Fix bug with override path in cgroups (#27620)

75736a9

* fix bug with override path * add changelog (cherry picked from commit f073c45)

mergify bot pushed a commit that referenced this pull request Aug 31, 2021

Fix bug with override path in cgroups (#27620)

541a34f

* fix bug with override path * add changelog (cherry picked from commit f073c45)

mergify bot mentioned this pull request Aug 31, 2021

[7.15](backport #27620) Fix bug with override path in cgroups #27661

Merged

Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021

Fix bug with override path in cgroups (elastic#27620)

06ebe62

* fix bug with override path * add changelog

Conversation

fearful-symmetry commented Aug 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

Uh oh!

elasticmachine commented Aug 26, 2021

Uh oh!

elasticmachine commented Aug 27, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

Uh oh!

axw left a comment

Choose a reason for hiding this comment

Uh oh!

matschaffer commented Aug 27, 2021

Uh oh!

ruflin commented Aug 30, 2021

Uh oh!

fearful-symmetry commented Aug 30, 2021

Uh oh!

mergify bot commented Aug 30, 2021

Uh oh!

matschaffer commented Aug 31, 2021

Uh oh!

axw commented Aug 31, 2021

Uh oh!

fearful-symmetry commented Aug 31, 2021

Uh oh!

fearful-symmetry commented Aug 31, 2021

Uh oh!

fearful-symmetry commented Aug 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fearful-symmetry commented Aug 26, 2021 •

edited

Loading

elasticmachine commented Aug 27, 2021 •

edited by jenkins-beats-ci bot

Loading