Skip to content

chore: deprecate BindMount APIs#1907

Merged
mdelapenya merged 4 commits intotestcontainers:mainfrom
mdelapenya:deprecate-mount-apis
Nov 8, 2023
Merged

chore: deprecate BindMount APIs#1907
mdelapenya merged 4 commits intotestcontainers:mainfrom
mdelapenya:deprecate-mount-apis

Conversation

@mdelapenya
Copy link
Copy Markdown
Member

@mdelapenya mdelapenya commented Nov 7, 2023

What does this PR do?

This PR deprecates all the BindMount types and functions/methods, removing their internal usage from the library (test code, basically).

The internal changes will modify the usage of BindMounts in this manner:

  • using ContainerRequest's Files field.
  • using ContainerRequest's HostCOnfigModifier field, using the closure to update the Binds of the Docker container's HostConfig type.
  • using the CopyFileToContainer/CopyDirToContainer methods of a running container.

It's also merging the docs regarding copy files and volumes, as both topics are closely related.

Why is it important?

BindMounts will make code not be portable cross docker environments, e.g. in remote Docker hosts, as the file binding won't exists in the remote.

For that reason we advocate for using the Files attribute of the container request in order to copy files/dirs to a container after it's created but before it's started, or the HostCOnfigModifier to define container binds, or CopyFileToContainer/CopyDirToContainer container methods.

Related issues

@mdelapenya mdelapenya requested a review from a team as a code owner November 7, 2023 20:21
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Nov 7, 2023
@mdelapenya mdelapenya self-assigned this Nov 7, 2023
@netlify
Copy link
Copy Markdown

netlify bot commented Nov 7, 2023

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 7dd4b4e
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/654b4c5db1f0f90008e90dc7
😎 Deploy Preview https://deploy-preview-1907--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.


// validateMounts ensures that the mounts do not have duplicate targets.
// It will check the Mounts and HostConfigModifier.Binds fields.
func (c *ContainerRequest) validateMounts() error {
Copy link
Copy Markdown
Member Author

@mdelapenya mdelapenya Nov 8, 2023

Choose a reason for hiding this comment

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

TBH I think the validation functions are very little useful, as they actually do too few checks.

We probably need a follow up discussion to remove it or rethink what to check here

}
ctx, cnl := context.WithTimeout(context.Background(), 30*time.Second)
defer cnl()
// Create a Docker client.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No need to instantiate a client for the volume, as Docker will handle the mounts correctly.

@mdelapenya mdelapenya merged commit 555cb76 into testcontainers:main Nov 8, 2023
@mdelapenya mdelapenya deleted the deprecate-mount-apis branch November 8, 2023 12:23
mdelapenya added a commit to kuisathaverat/testcontainers-go that referenced this pull request Nov 20, 2023
* main: (31 commits)
  feat: support for executing commands in a container with user, workDir and env (testcontainers#1914)
  fix(modules.kafka): Switch to MaxInt for 32-bit support (testcontainers#1923)
  docs: fix code snippet for image substitution (testcontainers#1918)
  Add database driver note to SQL Wait strategy docs (testcontainers#1916)
  Reduce flakiness in ClickHouse tests (testcontainers#1902)
  lint: enable nonamedreturns (testcontainers#1909)
  chore: deprecate BindMount APIs (testcontainers#1907)
  fix(reaper): fix race condition when reusing reapers (testcontainers#1904)
  feat: Allow the container working directory to be specified (testcontainers#1899)
  chore: make rabbitmq examples more readable (testcontainers#1905)
  chore(deps): bump github.com/twmb/franz-go and github.com/twmb/franz-go/pkg/kadm in /modules/redpanda (testcontainers#1896)
  Fix - respect ContainerCustomizer in neo4j module (testcontainers#1903)
  chore(deps): bump github.com/nats-io/nkeys and github.com/nats-io/nats.go in /modules/nats (testcontainers#1897)
  chore: add tests for withNetwork option (testcontainers#1894)
  chore(deps): bump google.golang.org/grpc and cloud.google.com/go/firestore in /modules/gcloud (testcontainers#1891)
  chore(deps): bump github.com/aws/aws-sdk-go and github.com/aws/aws-sdk-go-v2/config in /modules/localstack (testcontainers#1892)
  chore(deps): bump Github actions (testcontainers#1890)
  chore(deps): bump github.com/shirou/gopsutil/v3 from 3.23.9 to 3.23.10 (testcontainers#1858)
  chore(deps): bump github.com/hashicorp/consul/api in /examples/consul (testcontainers#1863)
  chore(deps): bump github.com/IBM/sarama in /modules/kafka (testcontainers#1874)
  ...
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Nov 30, 2023
* main: (100 commits)
  fix: fallback matching of registry authentication config (testcontainers#1927)
  feat: support customizing the Docker build command (testcontainers#1931)
  docs: include MongoDB's username and password options into the docs (testcontainers#1930)
  feat: support for custom registry prefixes at the configuration level (testcontainers#1928)
  Add username and password functions to mongodb (testcontainers#1910)
  chore: skip TestContainerLogWithErrClosed as flaky on rootless docker (testcontainers#1925)
  docs: add some Vault module examples (testcontainers#1825)
  feat: support for executing commands in a container with user, workDir and env (testcontainers#1914)
  fix(modules.kafka): Switch to MaxInt for 32-bit support (testcontainers#1923)
  docs: fix code snippet for image substitution (testcontainers#1918)
  Add database driver note to SQL Wait strategy docs (testcontainers#1916)
  Reduce flakiness in ClickHouse tests (testcontainers#1902)
  lint: enable nonamedreturns (testcontainers#1909)
  chore: deprecate BindMount APIs (testcontainers#1907)
  fix(reaper): fix race condition when reusing reapers (testcontainers#1904)
  feat: Allow the container working directory to be specified (testcontainers#1899)
  chore: make rabbitmq examples more readable (testcontainers#1905)
  chore(deps): bump github.com/twmb/franz-go and github.com/twmb/franz-go/pkg/kadm in /modules/redpanda (testcontainers#1896)
  Fix - respect ContainerCustomizer in neo4j module (testcontainers#1903)
  chore(deps): bump github.com/nats-io/nkeys and github.com/nats-io/nats.go in /modules/nats (testcontainers#1897)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Changes that do not impact the existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant