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

Disable logging when pulling on python integration tests #20397

Merged
merged 2 commits into from
Aug 5, 2020

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Aug 3, 2020

Docker compose library is quite verbose, and it prints many messages
when logging is enabled. On integration tests we make a pull before
trying to build the images in case the image is already pre-built. If
this pull doesn't work, the image is built, so we ignore errors on this
pull. But, even when ignoring errors, these errors are logged, and when
investigating problems with tests this may lead to think that the
problem is with the unavailability of some image. Disable logging on the
compose logger while this previous pull is being done.

What does this PR do?

Disables logging on errors happening when trying to pull images on
integration tests.

Note: Docker compose logging is still too verbose, but this is a collateral effect
of the test runner. This will improve after #16883.

Why is it important?

Errors when pulling images on integration tests are ignored, because
the image can still be built, but they are still logged, what is misleading.

See for example this thread: #19739 (comment)

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.

Logs

Before, if an image is not available, but it can be built, and the test fails for other reason, this is logged, what is confusing.

compose.service: ERROR: 404 Client Error: Not Found ("manifest for docker.elastic.co/integrations-ci/beats-redis:5.0.5-2 not found: manifest unknown: manifest unknown")

After this change no errors are logged by the pull if the test fails, logs regarding the later build are still logged:

compose.service: WARNING: Image for service redis was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.

Docker compose library is quite verbose, and it prints many messages
when logging is enabled. On integration tests we make a pull before
trying to build the images in case the image is already pre-built. If
this pull doesn't work, the image is built, so we ignore errors on this
pull. But, even when ignoring errors, these errors are logged, and when
investigating problems with tests this may lead to think that the
problem is with the unavailability of some image. Disable logging on the
compose logger while this previous pull is being done.
@jsoriano jsoriano requested a review from ycombinator August 3, 2020 10:54
@jsoriano jsoriano self-assigned this Aug 3, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@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 Aug 3, 2020
@jsoriano jsoriano added needs_backport PR is waiting to be backported to other branches. v7.10.0 labels Aug 3, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 3, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20397 updated]

  • Start Time: 2020-08-03T11:08:39.698+0000

  • Duration: 76 min 18 sec

Test stats 🧪

Test Results
Failed 0
Passed 17812
Skipped 1920
Total 19732

@@ -231,3 +237,14 @@ def service_log_contains(cls, service, msg):
if line.find(msg.encode("utf-8")) >= 0:
counter += 1
return counter > 0


@contextmanager
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice use of context managers!

Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

@jsoriano jsoriano merged commit 6d8acd0 into elastic:master Aug 5, 2020
@jsoriano jsoriano deleted the disable-compose-pull-logs branch August 5, 2020 10:27
jsoriano added a commit to jsoriano/beats that referenced this pull request Aug 5, 2020
Docker compose library is quite verbose, and it prints many messages
when logging is enabled. On integration tests we make a pull before
trying to build the images in case the image is already pre-built. If
this pull doesn't work, the image is built, so we ignore errors on this
pull. But, even when ignoring errors, these errors are logged, and when
investigating problems with tests this may lead to think that the
problem is with the unavailability of some image. Disable logging on the
compose logger while this previous pull is being done.

(cherry picked from commit 6d8acd0)
@jsoriano jsoriano removed the needs_backport PR is waiting to be backported to other branches. label Aug 5, 2020
v1v added a commit to v1v/beats that referenced this pull request Aug 6, 2020
…ne-2.0

* upstream/master:
  [docs] Promote ingest management to beta (elastic#20295)
  Upgrade elasticsearch client library used in tests (elastic#20405)
  Disable logging when pulling on python integration tests (elastic#20397)
  Remove pillow from testing requirements.txt (elastic#20407)
  [Filebeat][ATP Module]Setting user agent field required by the API (elastic#20440)
  [Ingest Manager] Send datastreams fields (elastic#20402)
  Add event.ingested to all Filebeat modules (elastic#20386)
  [Elastic Agent] Fix agent control socket path to always be less than 107 characters (elastic#20426)
  Improve cgroup_regex docs with examples (elastic#20425)
  Makes `metrics` config option required in app_insights (elastic#20406)
  Ensure install scripts only install if needed (elastic#20349)
  Update container name for the azure filesets (elastic#19899)
  Group same timestamp metrics values in app_insights metricset (elastic#20403)
  add_process_metadata processor adds container id even if process metadata not accessible (elastic#19767)
  Support "cluster" scope in Metricbeat elasticsearch module (elastic#18547)
  [Filebeat][SophosXG Module] Renaming module and fileset (elastic#20396)
  Update Suricata dashboards (elastic#20394)
  [Elastic Agent] Improve version, restart, enroll CLI commands (elastic#20359)
  Prepare home directories for docker images in a different stage (elastic#20356)
v1v added a commit to v1v/beats that referenced this pull request Aug 6, 2020
…allation

* upstream/master: (23 commits)
  [docs] Promote ingest management to beta (elastic#20295)
  Upgrade elasticsearch client library used in tests (elastic#20405)
  Disable logging when pulling on python integration tests (elastic#20397)
  Remove pillow from testing requirements.txt (elastic#20407)
  [Filebeat][ATP Module]Setting user agent field required by the API (elastic#20440)
  [Ingest Manager] Send datastreams fields (elastic#20402)
  Add event.ingested to all Filebeat modules (elastic#20386)
  [Elastic Agent] Fix agent control socket path to always be less than 107 characters (elastic#20426)
  Improve cgroup_regex docs with examples (elastic#20425)
  Makes `metrics` config option required in app_insights (elastic#20406)
  Ensure install scripts only install if needed (elastic#20349)
  Update container name for the azure filesets (elastic#19899)
  Group same timestamp metrics values in app_insights metricset (elastic#20403)
  add_process_metadata processor adds container id even if process metadata not accessible (elastic#19767)
  Support "cluster" scope in Metricbeat elasticsearch module (elastic#18547)
  [Filebeat][SophosXG Module] Renaming module and fileset (elastic#20396)
  Update Suricata dashboards (elastic#20394)
  [Elastic Agent] Improve version, restart, enroll CLI commands (elastic#20359)
  Prepare home directories for docker images in a different stage (elastic#20356)
  New multiline mode in Filebeat: while_pattern (elastic#19662)
  ...
jsoriano added a commit that referenced this pull request Aug 6, 2020
…0445)

Docker compose library is quite verbose, and it prints many messages
when logging is enabled. On integration tests we make a pull before
trying to build the images in case the image is already pre-built. If
this pull doesn't work, the image is built, so we ignore errors on this
pull. But, even when ignoring errors, these errors are logged, and when
investigating problems with tests this may lead to think that the
problem is with the unavailability of some image. Disable logging on the
compose logger while this previous pull is being done.

(cherry picked from commit 6d8acd0)
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
Docker compose library is quite verbose, and it prints many messages
when logging is enabled. On integration tests we make a pull before
trying to build the images in case the image is already pre-built. If
this pull doesn't work, the image is built, so we ignore errors on this
pull. But, even when ignoring errors, these errors are logged, and when
investigating problems with tests this may lead to think that the
problem is with the unavailability of some image. Disable logging on the
compose logger while this previous pull is being done.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants