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

feat: provide container logs on container startup failures #1297

Conversation

mdelapenya
Copy link
Member

@mdelapenya mdelapenya commented Jun 21, 2023

  • chore: move waitFor to the default container hooks
  • chore: print wait for values in the log
  • feat: add container logs when the starting/started hooks of the container fails

What does this PR do?

This PR adds 3 things:

  1. it extract the logic to start the wait strategies from the Start container method to the default life cycle hooks, as the first PostStart function.

  2. it adds the string representation of the wait strategy struct to the log, using the %+v formatter, which will add fields and values. I'm doing it in this manner for simplicity: to avoid adding a String method to all the wait strategies, with custom output message. But I'm OK if we want to do that in this PR or in a follow up one.

2023/06/20 08:43:28 🚧 Waiting for container id bc42707acea8 image: fcd1ad29-af53-4055-9143-1b36c4fdea43:a13d99be-4bf6-4df3-adb9-08d78baacba7. Waiting for: &{timeout: Log:Ready to accept connections!!!! Occurrence:1 PollInterval:100ms}

  1. it adds a private method to the DockerContainer struct to print out its container's logs. It will use the defined logger for the container, as set in the GenericContainerRequest struct. This is important as we are adding a test to demonstrate this behaviour, and in it we are adding a test-logger that stores the logs in an array of strings, therefore we are able to test that the logger is receiving the container logs.

The c.printLogs method will be called on any failure occurring at both the Starting and Started hooks: we have access to the container here, so we can get its logs.

2023/06/20 08:43:28 container logs:
        1:C 20 Jun 2023 06:43:28.010 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
        1:C 20 Jun 2023 06:43:28.010 # Redis version=5.0.14, bits=64, commit=00000000, modified=0, pid=1, just started
        1:C 20 Jun 2023 06:43:28.010 # Warning: no config file specified, using the default config. In order to specify a config file use redis-server /path/to/redis.conf
        1:M 20 Jun 2023 06:43:28.011 * Running mode=standalone, port=6379.
        1:M 20 Jun 2023 06:43:28.011 # Server initialized
        1:M 20 Jun 2023 06:43:28.011 # WARNING you have Transparent Huge Pages (THP) support enabled in your kernel. This will create latency and memory usage issues with Redis. To fix this issue run the command 'echo never > /sys/kernel/mm/transparent_hugepage/enabled' as root, and add it to your /etc/rc.local in order to retain the setting after a reboot. Redis must be restarted after THP is disabled.
        1:M 20 Jun 2023 06:43:28.012 * Ready to accept connections

Why is it important?

Many times the container fails the wait strategy and the user does not have feedback on why. Printing the logs will help them troubleshoot any error at that time.

Related issues

Follow-ups

We could implement the Stringer interface in all the existing wait strategies, adding custom and explicit info for each. Nowadays, with the current proposal, the pointer types will be printed as their memory representation (e.g. timeout, which TBH I do not know why it's a pointer)

Another follow-up would be adding the print container logs on the rest of hooks where we have access to a running container: stopping/stopped/terminating/terminated 🤔 . I can do it in this PR if needed

@mdelapenya mdelapenya requested a review from a team as a code owner June 21, 2023 09:26
@mdelapenya mdelapenya added the feature New functionality or new behaviors on the existing one label Jun 21, 2023
@mdelapenya mdelapenya self-assigned this Jun 21, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jun 21, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@netlify
Copy link

netlify bot commented Jun 21, 2023

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit a180227
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/6492c25517a6320008c34bcf
😎 Deploy Preview https://deploy-preview-1297--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

docker.go Show resolved Hide resolved
@mdelapenya
Copy link
Member Author

I'm going to merge this as is, keeping the idea of using the other lifecycle hooks for log errors as follow-up discussions

@mdelapenya mdelapenya merged commit eca8ba8 into testcontainers:main Jun 22, 2023
@mdelapenya mdelapenya deleted the provide-logs-on-container-startup-failure branch June 22, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or new behaviors on the existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: Provide logs up until the context deadline when failing to wait for condition
2 participants