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

[Ingest Manager] Prevent closing closed reader #20214

Merged
merged 2 commits into from
Jul 23, 2020

Conversation

michalpristas
Copy link
Contributor

@michalpristas michalpristas commented Jul 23, 2020

What does this PR do?

With #20127 config reader is closed on read. no need to do that in code. removes all occurences of close

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.

@michalpristas michalpristas added bug review needs_backport PR is waiting to be backported to other branches. Team:Ingest Management Ingest Management:beta1 Group issues for ingest management beta1 labels Jul 23, 2020
@michalpristas michalpristas self-assigned this Jul 23, 2020
@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 Jul 23, 2020
@michalpristas michalpristas changed the title prevent closing closed [Ingest Manager] Prevent closing closed reader Jul 23, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 23, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20214 event]

  • Start Time: 2020-07-23T17:54:21.815+0000

  • Duration: 67 min 42 sec

@michalpristas michalpristas merged commit 96fa64d into elastic:master Jul 23, 2020
@mdelapenya
Copy link
Contributor

I thing this change is causing the e2e tests to fail: https://beats-ci.elastic.co/blue/organizations/jenkins/e2e-tests%2Fe2e-testing-mbp/detail/master/91/pipeline#step-240-log-144

[2020-07-24T06:04:44.021Z] fail to enroll: acquiring metadata failed: close /elastic-agent-8.0.0-SNAPSHOT-linux-x86_64/fleet.yml: file already closed
[2020-07-24T06:04:44.283Z] time="2020-07-24T06:04:44Z" level=error msg="Could not execute command in container" command="[elastic-agent enroll http://kibana:5601 dm5oc2YzTUJTdTFjYTVBYWRRZm46dkxScTQ2ODJTR2V6RzRJRUN0MC1vUQ== -f --insecure]" error="Could not run compose file: [/var/lib/jenkins/workspace/e2e-tests_e2e-testing-mbp_master/.op/compose/profiles/ingest-manager/docker-compose.yml /var/lib/jenkins/workspace/e2e-tests_e2e-testing-mbp_master/.op/compose/services/centos/docker-compose.yml] - Local Docker compose exited abnormally whilst running docker-compose: [exec -T centos elastic-agent enroll http://kibana:5601 dm5oc2YzTUJTdTFjYTVBYWRRZm46dkxScTQ2ODJTR2V6RzRJRUN0MC1vUQ== -f --insecure]. exit status 1" service=centos

We download the elastic-agent TAR file and mount a volume in the container so that we can untar it in the running Centos container.

And for the stand-alone mode, the elastic-agent docker image fails too: https://beats-ci.elastic.co/blue/organizations/jenkins/e2e-tests%2Fe2e-testing-mbp/detail/master/91/pipeline/207#step-240-log-144

@michalpristas
Copy link
Contributor Author

@mdelapenya the error you're describing is cause by #20127 this one should fix it.

@mdelapenya
Copy link
Contributor

Cool then! Do you know when the build artifacts will be generated?

v1v added a commit to v1v/beats that referenced this pull request Jul 27, 2020
…ne-2.0

* upstream/master: (41 commits)
  adding possibility to override content-type checks, it was breaking certain webhooks that is not able to set content-headers at all. Still defaults to application/json (elastic#20232)
  fix: use a fixed worker type for tests (elastic#20130)
  [Ingest Manager] Prepare packaging for endpoint and asc files (elastic#20186)
  [Packetbeat] HTTP: Improve support for 100-continue elastic#15830 (elastic#19349)
  Increase index.max_docvalue_fields_search to 200 (elastic#20218)
  [Ingest Manager] Prevent closing closed reader (elastic#20214)
  [Metricbeat] Use MySQL Host Parser in Query metricset (elastic#20191)
  Testing: Ignore timestamp from cylance/protect dataset (elastic#20211)
  [Filebeat] Ignore cylance.protect timestamps while testing (elastic#20207)
  [CI] remove codecov step (elastic#20102)
  [docs] Indicate that SYSTEM user is required on Windows to use Endpoint (elastic#20172)
  Remove f5/firepass rsa2elk fileset (elastic#20160)
  [Elastic Agent] Improve GRPC stop to be more relaxed. (elastic#20118)
  Fix fileset field prefixing (elastic#20170)
  Fix terminating pod autodiscover issue (elastic#20084)
  Call host parser only once when building light metricsets (elastic#20149)
  [CI] fix null string with contains (elastic#20182)
  [Ingest Manager] Fix failing unit tests on windows (elastic#20127)
  [Filebeat] Update crowdstrike module (elastic#20138)
  [docs] Add x-pack role to relevant metricsets (elastic#20167)
  ...
@ph
Copy link
Contributor

ph commented Jul 27, 2020

The release manager build the artifact daily, but we have a ci job that build beats and elastic-agent on every commit.

@mdelapenya
Copy link
Contributor

This URL hasn't changed for days: https://artifacts-api.elastic.co/v1/search/8.0.0-SNAPSHOT/elastic-agent

Do you know where are we pushing those commit-based artifacts (docker image and binaries)?

@ph
Copy link
Contributor

ph commented Jul 28, 2020

@mdelapenya Yes, you are right, looking at CI it's been failing for a few days. There a build going on.

@ph
Copy link
Contributor

ph commented Jul 28, 2020

@mdelapenya It was failing because of beats 04:50:47 org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':buildBeatsSnapshot'. Next build should fix the issue it include 59ddf55

melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Ingest Management:beta1 Group issues for ingest management beta1 needs_backport PR is waiting to be backported to other branches. review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants