Skip to content

fix: do not override ImageBuildOptions.Labels when building from a Dockerfile#2775

Merged
mdelapenya merged 22 commits intotestcontainers:mainfrom
rzajac:fix-2632
Sep 18, 2024
Merged

fix: do not override ImageBuildOptions.Labels when building from a Dockerfile#2775
mdelapenya merged 22 commits intotestcontainers:mainfrom
rzajac:fix-2632

Conversation

@rzajac
Copy link
Copy Markdown
Contributor

@rzajac rzajac commented Sep 9, 2024

Closes #2632.

@rzajac rzajac requested a review from a team as a code owner September 9, 2024 17:32
@netlify
Copy link
Copy Markdown

netlify bot commented Sep 9, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit ec0d18b
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/66eabc6a4818cd00086e2b7d
😎 Deploy Preview https://deploy-preview-2775--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 configuration.

rzajac and others added 4 commits September 15, 2024 00:13
Copy link
Copy Markdown
Member

@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.

Just added a few minor comments regarding previous suggestions by @stevenh (thanks!)

I think, once resolved, we are good to go.

Thanks!

Copy link
Copy Markdown
Contributor

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

Just two little fixes, then should be good to go.

rzajac and others added 2 commits September 17, 2024 10:40
Copy link
Copy Markdown
Member

@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.

@rzajac I think this PR looks really good, thanks for addressing all our comments and sorry of they took that long 🙏

Once the last final suggestions are addressed, I think we are good to go. In the meantime, it LGTM (will approve once they CI passes too).

rzajac and others added 6 commits September 17, 2024 21:00
@rzajac
Copy link
Copy Markdown
Contributor Author

rzajac commented Sep 17, 2024

@mdelapenya Glad to be of help!

stevenh
stevenh previously approved these changes Sep 17, 2024
Copy link
Copy Markdown
Contributor

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

Nice, thanks for doing this. We need to check if the test failures are caused by this and fix if needed.

Copy link
Copy Markdown
Member

@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.

Just one last minute change, regarding how the test is cleaning up the created container.

Right after that, we are good to go, thanks!

Co-authored-by: Manuel de la Peña <[email protected]>
Copy link
Copy Markdown
Member

@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.

LGTM, thanks for your patience during the review 🙇

@mdelapenya mdelapenya self-assigned this Sep 18, 2024
@mdelapenya mdelapenya added the bug An issue with the library label Sep 18, 2024
@mdelapenya mdelapenya changed the title Fix #2632 - ImageBuildOptions.Labels are overwritten fix: do not override ImageBuildOptions.Labels when building from a Dockerfile Sep 18, 2024
@mdelapenya mdelapenya merged commit 31a033c into testcontainers:main Sep 18, 2024
mdelapenya added a commit that referenced this pull request Sep 20, 2024
* main:
  feat: support databend module (#2779)
  chore: golangci-lint 1.61.0 (#2787)
  fix(mssql): bump Docker image version (#2786)
  fix: handle 127 error code for podman compatibility (#2778)
  fix: do not override ImageBuildOptions.Labels when building from a Dockerfile (#2775)
  feat(mongodb): Wait for mongodb module with a replicaset to finish (#2777)
  fix(postgres): Apply default snapshot name if no name specified (#2783)
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Sep 23, 2024
* main:
  chore: use a much smaller image for testing (testcontainers#2795)
  fix: parallel containers clean race (testcontainers#2790)
  fix(registry): wait for (testcontainers#2793)
  fix: container timeout test (testcontainers#2792)
  docs: document redpanda options (testcontainers#2789)
  feat: support databend module (testcontainers#2779)
  chore: golangci-lint 1.61.0 (testcontainers#2787)
  fix(mssql): bump Docker image version (testcontainers#2786)
  fix: handle 127 error code for podman compatibility (testcontainers#2778)
  fix: do not override ImageBuildOptions.Labels when building from a Dockerfile (testcontainers#2775)
  feat(mongodb): Wait for mongodb module with a replicaset to finish (testcontainers#2777)
  fix(postgres): Apply default snapshot name if no name specified (testcontainers#2783)
  fix: resource clean up for tests and examples (testcontainers#2738)
  ci: add generate for mocks (testcontainers#2774)
  fix: docker config error handling when config file does not exist (testcontainers#2772)
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Sep 23, 2024
* main:
  chore: use a much smaller image for testing (testcontainers#2795)
  fix: parallel containers clean race (testcontainers#2790)
  fix(registry): wait for (testcontainers#2793)
  fix: container timeout test (testcontainers#2792)
  docs: document redpanda options (testcontainers#2789)
  feat: support databend module (testcontainers#2779)
  chore: golangci-lint 1.61.0 (testcontainers#2787)
  fix(mssql): bump Docker image version (testcontainers#2786)
  fix: handle 127 error code for podman compatibility (testcontainers#2778)
  fix: do not override ImageBuildOptions.Labels when building from a Dockerfile (testcontainers#2775)
  feat(mongodb): Wait for mongodb module with a replicaset to finish (testcontainers#2777)
  fix(postgres): Apply default snapshot name if no name specified (testcontainers#2783)
  fix: resource clean up for tests and examples (testcontainers#2738)
mdelapenya added a commit that referenced this pull request Sep 26, 2024
* main: (29 commits)
  fix: template for code generation (#2800)
  fix: update module path (#2797)
  fix: container logging deadlocks (#2791)
  chore: use a much smaller image for testing (#2795)
  fix: parallel containers clean race (#2790)
  fix(registry): wait for (#2793)
  fix: container timeout test (#2792)
  docs: document redpanda options (#2789)
  feat: support databend module (#2779)
  chore: golangci-lint 1.61.0 (#2787)
  fix(mssql): bump Docker image version (#2786)
  fix: handle 127 error code for podman compatibility (#2778)
  fix: do not override ImageBuildOptions.Labels when building from a Dockerfile (#2775)
  feat(mongodb): Wait for mongodb module with a replicaset to finish (#2777)
  fix(postgres): Apply default snapshot name if no name specified (#2783)
  fix: resource clean up for tests and examples (#2738)
  ci: add generate for mocks (#2774)
  fix: docker config error handling when config file does not exist (#2772)
  docs: refine heading badges in README (#2770)
  feat(wait): for file (#2731)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue with the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ImageBuildOptions.Labels are overwritten

3 participants