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

ci: use dockerLogs step #810

Merged
merged 4 commits into from
Jun 4, 2020
Merged

ci: use dockerLogs step #810

merged 4 commits into from
Jun 4, 2020

Conversation

v1v
Copy link
Member

@v1v v1v commented Jun 2, 2020

What

It archives the docker container details for every stage that runs docker.

How to use it

  • Go to the artifact/docker-info folder
  • Look for the stage that you'd like to go deeper with
  • Open the file docker-containers.txt
  • Look for the container ID
  • <id>.log is the logs for the container with id
  • <id>-cmd.txt is the docker container run parameters
  • <id>-inspect.json is the output of the docker inspect command

Tests

image

image

(BO view)

image

Issues

Closes #806

@v1v v1v self-assigned this Jun 2, 2020
@v1v v1v added the automation label Jun 2, 2020
@apmmachine
Copy link
Contributor

apmmachine commented Jun 2, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #810 updated]

  • Start Time: 2020-06-02T16:49:35.480+0000

  • Duration: 72 min 4 sec

Test stats 🧪

Test Results
Failed 0
Passed 932
Skipped 10
Total 942

Steps errors

Expand to view the steps failures

  • Name: Bundlesize

    • Description: #!/bin/bash set -o pipefail npm run bundlesize|tee bundlesize.txt

    • Duration: 1 min 32 sec

    • Start Time: 2020-06-02T16:54:45.657+0000

    • log

  • Name: Start Elastic Stack 8.0.0-SNAPSHOT - @elastic/apm-rum-core - saucelabs

    • Description:

    • Duration: 4 min 58 sec

    • Start Time: 2020-06-02T17:15:59.360+0000

    • log

  • Name: Error signal

    • Description:

    • Duration: 0 min 0 sec

    • Start Time: 2020-06-02T17:19:57.525+0000

    • log

@codecov-commenter
Copy link

Codecov Report

Merging #810 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #810   +/-   ##
=======================================
  Coverage   92.66%   92.66%           
=======================================
  Files          50       50           
  Lines        2291     2291           
  Branches      456      456           
=======================================
  Hits         2123     2123           
  Misses        165      165           
  Partials        3        3           

Jenkinsfile Outdated Show resolved Hide resolved
@v1v v1v marked this pull request as ready for review June 2, 2020 16:50
@v1v v1v requested review from vigneshshanmugam, hmdhk and a team June 2, 2020 16:50
@v1v v1v added the ci label Jun 2, 2020
sleep randomNumber(min: 5, max: 10)
if(env.MODE == 'saucelabs'){
withSaucelabsEnv(){
try {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is required to be able to run a post-build step within this method. We could potentially add this section to the post-build always step. What do you prefer?

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, I'm afraid we cannot do the post-block though, the runAllScopes runs several scopes in a loop :(

Copy link
Contributor

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Maybe it's a good idea to have it in a post block, to always have it if needed

Copy link
Contributor

@cachedout cachedout left a comment

Choose a reason for hiding this comment

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

I'm worried about this one. Won't docker inspect will dump all environment details? I'm concerned that we might end up leaking sensitive data. Even if that isn't the case now, we might accidentally forget that this is in place in the future.

@v1v
Copy link
Member Author

v1v commented Jun 3, 2020

I'm worried about this one. Won't docker inspect will dump all environment details? I'm concerned that we might end up leaking sensitive data. Even if that isn't the case now, we might accidentally forget that this is in place in the future.

What do you suggest?

@cachedout
Copy link
Contributor

What do you suggest?

Sorry, I didn't see this comment earlier. I don't think we should ever do a docker inspect on a production CI job. I see that elastic/apm-pipeline-library#580 removes this capability so I think we are on the same page here. 👍

@kuisathaverat
Copy link
Contributor

I'm worried about this one. Won't docker inspect will dump all environment details? I'm concerned that we might end up leaking sensitive data. Even if that isn't the case now, we might accidentally forget that this is in place in the future.

should not be secrets references in the docker-compose.yml, and the ones that can contain are related to the local containers and do not give access anywhere. In some cases, we need the inspect to see the status of the container when it died, we can remove the environment from the inspect output when we generate the file.

@cachedout
Copy link
Contributor

we can remove the environment from the inspect output when we generate the file.

Parsing the output and removing the Envrionment value would satisfy my concerns. 👍

@kuisathaverat
Copy link
Contributor

this makes the thing

docker inspect 60754aee9a26 |jq 'del(.[].Config.Env)'

@v1v v1v merged commit 7ff7026 into elastic:master Jun 4, 2020
@v1v v1v deleted the feature/docker-logs branch June 4, 2020 07:13
v1v added a commit to v1v/apm-agent-rum-js that referenced this pull request Jul 3, 2020
* upstream/master: (23 commits)
  feat(rum-core): capture XHR/Fetch spans using resource timing (elastic#825)
  docs: update set-up.asciidoc (elastic#814)
  chore: remove compressed size gh workflow (elastic#828)
  feat: use page visibilityState for browser responsiveness check (elastic#813)
  ci(jenkins): report bundlesize as a GitHub comment (elastic#826)
  docs: release notes for 5.2.1 (elastic#824)
  chore(release): publish
  fix(rum-core): protect aganist buggy navigation timing data (elastic#819)
  fix(rum-core): protect aganist buggy navigation timing data (elastic#819)
  chore(rum-core): use startTime for LCP marks (elastic#815)
  fix(rum-core): capture tbt after all task entries are observed (elastic#803)
  feat(rum-react): use correct path when route is path array (elastic#800)
  ci: enable benchmark on a PR basis (elastic#812)
  ci: use dockerLogs step (elastic#810)
  fix: env var invalid type (elastic#809)
  fix: workarount for elastic/beats#18858 (elastic#807)
  docs: add release notes for 5.2.0 (elastic#801)
  chore(release): publish
  fix(rum-core): consider user defined type of high precedence (elastic#798)
  fix(rum): use single instance of apm across all packages (elastic#796)
  ...
David-Development pushed a commit to David-Development/apm-agent-rum-js that referenced this pull request Oct 20, 2021
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.

[CI] add an step to grab the Docker container logs
6 participants