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

[WIP] Ignores ErrInputNotFinished when updating config #31928

Closed
wants to merge 2 commits into from

Conversation

belimawr
Copy link
Contributor

@belimawr belimawr commented Jun 14, 2022

DO NOT REVIEW

I added a bunch of debug and experiment code here just to to lose it for the time being.

Current status

I'm creating this PR very early to see if any integration test will break.

Ignores common.InputNotFinished when running undre Elastic-Agent and
updating configuration.

A few refactorings were needed:

  • "github.com/hashicorp/go-multierror" was replaced by
    "github.com/joeshaw/multierror" that is already used by other parts of
    Beats. This makes the error wrapping more consistent
  • common.InputNotFinished implements the Is method, making it
    compatible with errors.Is.

What does this PR do?

Why is it important?

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

Related issues

Use cases

Screenshots

Logs

Ignores `common.InputNotFinished` when running undre Elastic-Agent and
updating configuration.

A few refactorings were needed:
* "github.com/hashicorp/go-multierror" was replaced by
"github.com/joeshaw/multierror" that is already used by other parts of
Beats. This makes the error wrapping more consistent
* common.InputNotFinished implements the `Is` method, making it
compatible with `errors.Is`.
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 14, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 14, 2022

💔 Build Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-06-16T09:13:02.756+0000

  • Duration: 17 min 3 sec

Test stats 🧪

Test Results
Failed 0
Passed 3
Skipped 0
Total 3

Steps errors 19

Expand to view the steps failures

Show only the first 10 steps failures

x-pack/filebeat-checks - make -C x-pack/filebeat check;make -C x-pack/filebeat update;make -C file
  • Took 3 min 54 sec . View more details here
  • Description: make -C x-pack/filebeat check;make -C x-pack/filebeat update;make -C filebeat check;make -C filebeat update;make check-no-changes;
x-pack/functionbeat-checks - make -C x-pack/functionbeat check;make -C x-pack/functionbeat update;
  • Took 1 min 29 sec . View more details here
  • Description: make -C x-pack/functionbeat check;make -C x-pack/functionbeat update;make check-no-changes;
x-pack/heartbeat-checks - make -C x-pack/heartbeat check;make -C x-pack/heartbeat update;make -C h
  • Took 2 min 17 sec . View more details here
  • Description: make -C x-pack/heartbeat check;make -C x-pack/heartbeat update;make -C heartbeat check;make -C heartbeat update;make check-no-changes;
x-pack/libbeat-checks - make -C x-pack/libbeat check;make -C x-pack/libbeat update;make check-no-c
  • Took 0 min 8 sec . View more details here
  • Description: make -C x-pack/libbeat check;make -C x-pack/libbeat update;make check-no-changes;
x-pack/metricbeat-checks - make -C x-pack/metricbeat check;make -C x-pack/metricbeat update;make -
  • Took 3 min 25 sec . View more details here
  • Description: make -C x-pack/metricbeat check;make -C x-pack/metricbeat update;make -C metricbeat check;make -C metricbeat update;make check-no-changes;
x-pack/osquerybeat-checks - make -C x-pack/osquerybeat check;make -C x-pack/osquerybeat update;mak
  • Took 1 min 29 sec . View more details here
  • Description: make -C x-pack/osquerybeat check;make -C x-pack/osquerybeat update;make check-no-changes;
x-pack/packetbeat-checks - make -C packetbeat check;make -C packetbeat update;make check-no-change
  • Took 2 min 39 sec . View more details here
  • Description: make -C packetbeat check;make -C packetbeat update;make check-no-changes;
x-pack/winlogbeat-checks-immutable && ubuntu-18 - make -C x-pack/winlogbeat check;make -C x-pack/wi
  • Took 2 min 8 sec . View more details here
  • Description: make -C x-pack/winlogbeat check;make -C x-pack/winlogbeat update;make -C winlogbeat check;make -C winlogbeat update;make check-no-changes;
make check-default
  • Took 2 min 50 sec . View more details here
  • Description: make check-default
Error signal
  • Took 0 min 0 sec . View more details here
  • Description: Error 'hudson.AbortException: script returned exit code 2'

💚 Flaky test report

Tests succeeded.

🤖 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!)

@belimawr belimawr added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jun 14, 2022
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 14, 2022
@@ -85,7 +85,7 @@ func (r *RunnerList) Reload(configs []*reload.ConfigWithMeta) error {
for hash, runner := range stopList {
r.logger.Debugf("Stopping runner: %s", runner)
delete(r.runners, hash)
go runner.Stop()
runner.Stop() // we need to wait this to finish before starting new inputs
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we? It did not fix the problem we had, so I do not want to change this to be blocking. Why "fix" something that is not broken?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed.

This PR still a draft of a draft, it still needs cleaning up. If we decide to keep working on that I'll keep it async.

@belimawr
Copy link
Contributor Author

076e702 contains a HUGE amount of debug code, it's quite messy. If I keep working on this PR, I'll clean up everything before requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-ci Skip the build in the CI but linting 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.

4 participants