fix(influxdb): Respect custom waitStrategy by marcinmilewski93 · Pull Request #2845 · testcontainers/testcontainers-go

marcinmilewski93 · 2024-10-22T08:22:11Z

What does this PR do?

The purpose of this PR is to respect the provided waitStrategy for the InfluxDB container, rather than overriding it by default. This change allows the use of a custom waitStrategy when specified; if none is provided, the default waitStrategy will be applied, maintaining the same configuration as before this modification.

Why is it important?

In the current implementation, it's not possible to use a custom waitStrategy and in my case the default strategy marks the container as ready too early. The container is still not ready to receive writes as you can see below. Customizing the waitStrategy for example to check the /health status solved this issue. Example log output when container is not ready yet:

2024/10/22 10:38:54 ✅ Container started: d5d6612ccd27
2024/10/22 10:38:54 ⏳ Waiting for container id d5d6612ccd27 image: influxdb:2.0.7. Waiting for: &{timeout:<nil> Log:Listening log_id=[0-9a-zA-Z_~]+ service=tcp-listener transport=http IsRegexp:true Occurrence:1 PollInterval:100ms}
2024/10/22 10:39:04 🔔 Container is ready: d5d6612ccd27
2024-10-22T10:39:23.408+07:00|2|ERROR||,,,testing-invalid|9469|goroutine:1783|internal/runner.go:93|Post "http://localhost:49155/api/v2/write?bucket=bucket1&org=influxdata&precision=ns": read tcp [::1]:43726->[::1]:49155: read: connection reset by peer
2024/10/22 10:39:24 [E]! Write error: Post "http://localhost:49155/api/v2/write?bucket=lbucket1&org=influxdata&precision=ns": read tcp [::1]:55852->[::1]:49155: read: connection reset by peer
2024/10/22 10:39:24 unable to send metrics to InfluxDB. err=Post "http://localhost:49155/api/v2/write?bucket=bucket1&org=influxdata&precision=ns": read tcp [::1]:55852->[::1]:49155: read: connection reset by peer

After this modification different strategy could be employed, for example, to verify if InfluxDB is ready for queries and writes as you can see on below health check response:

{
  "name": "influxdb",
  "message": "ready for queries and writes",
  "status": "pass",
  "checks": [],
  "version": "2.0.7",
  "commit": "2a45f0c037"
}

Related issues

None

How to test this PR

I have added a test that uses a custom waitStrategy to verify this behavior.

fix(influxdb): Respect passed waitStrategy

netlify · 2024-10-22T08:22:28Z

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	`dc16bfe`
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/672a6234841d760008d84c0b
😎 Deploy Preview	https://deploy-preview-2845--testcontainers-go.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

stevenh

Thanks for PR, tried reproducing but must be missing something

modules/influxdb/influxdb_test.go

Co-authored-by: Steven Hartland <[email protected]>

modules/influxdb/influxdb.go

…nfluxdb2 (#2)

modules/influxdb/influxdb.go

stevenh

Looking cleaner, did some testing locally which triggered some additional suggestions.

modules/influxdb/influxdb.go

Move Shutdown check to WithInitDb

mdelapenya

LGTM, thanks!

stevenh

Almost there, just two suggestions

modules/influxdb/influxdb.go

Co-authored-by: Steven Hartland <[email protected]>

mdelapenya

Added a final nit about a comment.

Other than that, LGTM, great job @marcinmilewski93 during the review 🙇

modules/influxdb/influxdb.go

mdelapenya · 2024-11-20T11:20:19Z

Thanks everybody here for the thorough review. Great job!

* main: (76 commits) fix(influxdb): Respect custom waitStrategy (testcontainers#2845) fix: only upload to sonar on ubuntu-latest (testcontainers#2891) fix: build artifact name properly (testcontainers#2890) fix: do not run sonar upload when ryuk is disabled (testcontainers#2889) fix: update GH actions for uploading/downloading artifacts (testcontainers#2888) feat(ci): Enable master moby with rootless (testcontainers#2880) fix(redpanda): temporary file use chore(deps): bump actions/download-artifact from 3.0.2 to 4.1.8 (testcontainers#2676) chore(deps): bump actions/upload-artifact from 3.1.3 to 4.4.3 (testcontainers#2885) fix!: port forwarding clean up and make private (testcontainers#2881) chore: resolve AWS deprecations for localstack (testcontainers#2879) docs: fix new lifecycle hooks section (testcontainers#2875) fix: host access port instability (testcontainers#2867) feat: add build to life cycle hooks (testcontainers#2653) fix: typo in containerd integration (testcontainers#2873) chore(deps): bump mkdocs-include-markdown-plugin from 6.0.4 to 6.2.2 (testcontainers#2806) chore: use testify instead of t.Error (testcontainers#2871) ci: enable perfsprint linter (testcontainers#2872) chore(deps): bump mkdocs-markdownextradata-plugin from 0.2.5 to 0.2.6 (testcontainers#2807) chore: use testcontainers.RequireContainerExec (testcontainers#2870) ...

* main: docs: better contribution guidelines (testcontainers#2893) fix(influxdb): Respect custom waitStrategy (testcontainers#2845)

* main: (234 commits) chore(ci): add Github labels based on PR title (testcontainers#2914) chore(gha): Use official setup-docker-action (testcontainers#2913) chore(ci): enforce conventional commits syntax in PR titles (testcontainers#2911) feat(nats): WithConfigFile - pass a configuration file to nats server (testcontainers#2905) chore: enable implicit default logger only in testing with -v (testcontainers#2877) fix: container binds syntax (testcontainers#2899) refactor(cockroachdb): to use request driven options (testcontainers#2883) chore(deps): bump actions/setup-go from 5.0.0 to 5.1.0 (testcontainers#2904) chore(deps): bump ossf/scorecard-action from 2.3.1 to 2.4.0 (testcontainers#2903) chore(deps): bump test-summary/action from 2.3 to 2.4 (testcontainers#2902) feat(wait): strategy walk (testcontainers#2895) feat(wait): tls strategy (testcontainers#2896) docs: better contribution guidelines (testcontainers#2893) fix(influxdb): Respect custom waitStrategy (testcontainers#2845) fix: only upload to sonar on ubuntu-latest (testcontainers#2891) fix: build artifact name properly (testcontainers#2890) fix: do not run sonar upload when ryuk is disabled (testcontainers#2889) fix: update GH actions for uploading/downloading artifacts (testcontainers#2888) feat(ci): Enable master moby with rootless (testcontainers#2880) fix(redpanda): temporary file use ...

Respect custom waitStrategy for InfluxDB (#1)

e667542

fix(influxdb): Respect passed waitStrategy

marcinmilewski93 requested a review from a team as a code owner October 22, 2024 08:22

stevenh requested changes Oct 22, 2024

View reviewed changes

modules/influxdb/influxdb_test.go Outdated Show resolved Hide resolved

modules/influxdb/influxdb_test.go Outdated Show resolved Hide resolved

Refactor test to use require for container state assertion

3b1e129

Co-authored-by: Steven Hartland <[email protected]>

mdelapenya reviewed Oct 23, 2024

View reviewed changes

modules/influxdb/influxdb.go Outdated Show resolved Hide resolved

marcinmilewski93 requested a review from stevenh October 25, 2024 03:03

marcinmilewski93 added 3 commits October 29, 2024 08:16

Change default wait strategy from checking logs to health check for i…

3a7ddf5

…nfluxdb2 (#2)

/health check waitStrategy for influx1 and influx2

28c28ee

Undo health endpoint test after default strategy was changed

825667c

mdelapenya reviewed Oct 29, 2024

View reviewed changes

modules/influxdb/influxdb.go Outdated Show resolved Hide resolved

stevenh requested changes Oct 29, 2024

View reviewed changes

modules/influxdb/influxdb.go Outdated Show resolved Hide resolved

modules/influxdb/influxdb.go Outdated Show resolved Hide resolved

modules/influxdb/influxdb.go Outdated Show resolved Hide resolved

Update http health check to verify JSON status value (#3)

56d1e62

marcinmilewski93 requested review from mdelapenya and stevenh October 30, 2024 08:37

stevenh reviewed Oct 30, 2024

View reviewed changes

modules/influxdb/influxdb.go Outdated Show resolved Hide resolved

Move Shutdown check to WithInitDb (#4)

90ad980

Move Shutdown check to WithInitDb

marcinmilewski93 requested a review from stevenh October 31, 2024 00:54

mdelapenya previously approved these changes Oct 31, 2024

View reviewed changes

stevenh requested changes Oct 31, 2024

View reviewed changes

modules/influxdb/influxdb.go Outdated Show resolved Hide resolved

modules/influxdb/influxdb.go Outdated Show resolved Hide resolved

Remove slice

df78856

Co-authored-by: Steven Hartland <[email protected]>

marcinmilewski93 dismissed mdelapenya’s stale review via df78856 October 31, 2024 12:51

Simplify comment

e0a47e9

Co-authored-by: Steven Hartland <[email protected]>

marcinmilewski93 requested a review from stevenh October 31, 2024 12:52

mdelapenya previously approved these changes Oct 31, 2024

View reviewed changes

modules/influxdb/influxdb.go Show resolved Hide resolved

More precise WitInitDb domment

fe28e6e

marcinmilewski93 dismissed mdelapenya’s stale review via fe28e6e November 1, 2024 00:25

marcinmilewski93 requested a review from mdelapenya November 1, 2024 00:28

stevenh approved these changes Nov 5, 2024

View reviewed changes

Merge branch 'main' into influxdb_strategy

dc16bfe

mdelapenya self-assigned this Nov 20, 2024

mdelapenya added the bug An issue with the library label Nov 20, 2024

mdelapenya merged commit 83b47ca into testcontainers:main Nov 20, 2024

mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Nov 25, 2024

Merge branch 'main' into context-support

dc571d3

* main: docs: better contribution guidelines (testcontainers#2893) fix(influxdb): Respect custom waitStrategy (testcontainers#2845)

Uh oh!

Conversation

marcinmilewski93 commented Oct 22, 2024

What does this PR do?

Why is it important?

Related issues

How to test this PR

Uh oh!

netlify bot commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for testcontainers-go ready!

Uh oh!

stevenh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevenh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdelapenya left a comment

Choose a reason for hiding this comment

Uh oh!

stevenh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mdelapenya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mdelapenya commented Nov 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Oct 22, 2024 •

edited

Loading