chore: move host-config and endpoint settings to a specific modifiers #633

mdelapenya · 2022-11-22T11:38:19Z

What does this PR do?

This PR addresses the need of separating concerns when creating a container, as we want to distinguish between basic and advanced settings.

With basic, we mean settings that are used by the vast majority of our users, exposed as part of the public API of Testcontainers for Go. And with advanced, we mean settings that should still be still available in the public API, but in a hidden manner.

Therefore, we are adding the concept of a Modifier to the ContainerRequest: a user will be able to define what advanced configurations should be processed before a container is created.

The advanced settings that we have identified at this moment are those that are related to 3 elements:

the container.Config Docker type
the container.HostConfig Docker type
the network.EndpointSettings

Therefore, we have defined three modifiers: ConfigModifier, HostConfigModifier and EndpointSettingsModifier.

From Docker docs:

// HostConfig the non-portable Config structure of a container.
// Here, "non-portable" means "dependent of the host we are running on".
// Portable information *should* appear in Config.

On the contrary, we chose basic settings to be related to the portable information of a container, under the container.Config Docker type:

// Config contains the configuration data about a container.
// It should hold only portable information about the container.
// Here, "portable" means "independent from the host we are running on".
// Non-portable information *should* appear in HostConfig.
// All fields added to this struct must be marked `omitempty` to keep getting
// predictable hashes from the old `v1Compatibility` configuration.

The technical implementation of this PR comes with:

we have created three new fields in the ContainerRequest that holds a function to pre-configure advanced settings for the container (hostConfig and endpointSettings): ConfigModifier, HostConfigModifier and EndpointSettingsModifier. These fields in the request are part of the public API of the container request.
the Modifiers functions are executed before the container is created using the Docker client.
the Modifiers function allow users modify the config, hostConfig and endpointSettings Docker types when they request a container, handy for advances use cases.
the fields that were present in the ContainerRequest related to the HostConfig, and the EndpointSettings have been Deprecated in favor of the different Modifiers.
the fields that relate to the "basic" settings, covered by the Config Docker type, are still present in the container request. The modifier will take precedence, as we are going to merge the struct in the modifier with the already created one (from the container request).
the library will internally check if a request includes each Modifier, and provide a default hook including the deprecated fields if it's not present in the request. This way we are keeping backwards compatibility until we remove those fields.
we have updated the existing tests, using the Modifiers when needed, removing the deprecated fields from the tests.
we have added unit tests for the preCreateModifierHook, to verify that all the fields in play are properly set.

Why is it important?

We want to allow users configure their containers, but helping them not to commit mistakes caused by a wrong configuration. If the public API directly exposes Docker types in a way that they are easy to consume, a user trying to configure a container with basic settings could be tempted to use advanced setting.

More info about this can be found in the #628 discussion.

Another benefit that comes here is reducing the size of the container request: we do not need to add fields to the public API to map a Docker type: anything can be modified with the Modifiers. We should provide a simple public API, but for advanced use cases, the modifiers assist.

Related issues

Relates Creating container requests: separation of concerns #628

How to test this PR

Running the tests would not represent a change in the previous behavior.

Another test could be using a custom project that replaces the testcontainers-go dependency for the one containing this PR in the local workspace. The dependent project must compile and its tests must pass.

…gs before container creation

Callback suggests execution after an operation. Using hook following what git does with commit hooks

kiview

I tried to match the levels against the current Java implementations. Some are for sure up to discussion, for some we know about their usage, especially in the context of container modules.

Every container module is a direct use case.

container.go

kiview · 2022-11-22T13:49:58Z

container.go

@@ -101,28 +102,29 @@ type ContainerRequest struct {
 	Cmd             []string
 	Labels          map[string]string
 	Mounts          ContainerMounts
-	Tmpfs           map[string]string
+	Tmpfs           map[string]string // Deprecated: Use PreCreationHook instead
 	RegistryCred    string
 	WaitingFor      wait.Strategy
 	Name            string // for specifying container name


I would deprecate this, risk for collisions and generally not needed in the context of TC usage.

Indeed. But I'd do it as part of a new PR, to make it even clearer that passing a name could be seen as an antipattern from testing standpoint

kiview · 2022-11-22T13:50:29Z

container.go

@@ -101,28 +102,29 @@ type ContainerRequest struct {
 	Cmd             []string
 	Labels          map[string]string
 	Mounts          ContainerMounts
-	Tmpfs           map[string]string
+	Tmpfs           map[string]string // Deprecated: Use PreCreationHook instead
 	RegistryCred    string
 	WaitingFor      wait.Strategy
 	Name            string // for specifying container name
 	Hostname        string


We don't have this in 1st level in Java. What are use cases?

Ditto: used in one single test verifying that the hostname was changed.

kiview · 2022-11-22T13:51:16Z

container.go

+	Privileged      bool                                                              // Deprecated: Use PreCreationHook instead. For starting privileged container
+	Networks        []string                                                          // for specifying network names
+	NetworkAliases  map[string][]string                                               // for specifying network aliases
+	NetworkMode     container.NetworkMode                                             // Deprecated: Use PreCreationHook instead


We have this in 1st level, but I think it is not appropriate there.

kiview · 2022-11-22T13:51:47Z

container.go

+	NetworkMode     container.NetworkMode                                             // Deprecated: Use PreCreationHook instead
+	Resources       container.Resources                                               // Deprecated: Use PreCreationHook instead
+	Files           []ContainerFile                                                   // files which will be copied when container starts
+	User            string                                                            // for specifying uid:gid


Not in 1st level in Java, are there good use cases for this?

This User is not used at all by the library, only in a test that verifies that running id -u returns the defined user. I'd use a follow-up PR to discuss whether a field is actually in use or not.

container.go

Preparation for the future

* main: Add toxiproxy example (testcontainers#643) Add spanner example (testcontainers#642) chore: sync governance files (testcontainers#641) Add pubsub example (testcontainers#640) chore: adjust generator for the docs site (testcontainers#639) Add datastore example (testcontainers#638) Add firestore example (testcontainers#637) fix: avoid panics when checking container state and container.raw is nil (#635) feat: provide a tool to generate examples from code (#618) chore: bump version in mkdocs (#630) docs: remove code snippets from main README (#631) docs: document replace directive for Docker Compose (#632)

* main: (44 commits) feat: support passing registry credentials to the reaper (testcontainers#647) fix: close response body in http strategy (testcontainers#718) chore: move e2e module to postgres example module (testcontainers#717) chore: bump containerd transitive dep in examples (testcontainers#715) chore(deps): bump github.com/containerd/containerd from 1.6.12 to 1.6.14 (testcontainers#703) chore(deps): bump github.com/compose-spec/compose-go in /modules/compose (testcontainers#710) chore: bump testcontainers-go to 0.17.0 in examples (testcontainers#714) chore(deps): bump github.com/docker/compose/v2 in /modules/compose (testcontainers#711) chore: support running MySQL compose in ARM (testcontainers#712) chore: simplify compose replace directives (testcontainers#713) chore: add compose module to dependabot (testcontainers#709) chore: move compose code to a separate module (testcontainers#650) docs: refine onboarding process with quickstart guide (testcontainers#706) chore: move redis-specific tests to the example module (testcontainers#701) chore: bump transitive dependencies (#527) chore: reduce concurrent builds (testcontainers#702) chore: add mysql example (testcontainers#700) chore(deps): bump google.golang.org/api from 0.104.0 to 0.105.0 (testcontainers#699) chore(deps): bump google.golang.org/api in /examples/firestore (testcontainers#683) chore(deps): bump cloud.google.com/go/spanner in /examples/spanner (testcontainers#688) ...

netlify · 2023-01-04T17:26:12Z

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	`43998c8`
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/63eccc214cc29700070585c0
😎 Deploy Preview	https://deploy-preview-633--testcontainers-go.netlify.app/features/creating_container
📱 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 settings.

mdelapenya · 2023-01-30T15:54:50Z

As discussed offline with @HofmeisterAn and @eddumelendez having another modifier for the container.Config docker type would be of interest, as then anybody will be able anything they need with advanced scenarios. At the same time we should document it for advanced settings.

In the case of advanced setups, a user would be able to modify anything in the Docker config

lifecycle.go

This reverts commit f1b6e26.

docs/features/creating_container.md

kiview · 2023-02-16T10:04:28Z

docs/features/creating_container.md

@@ -87,6 +87,17 @@ func TestIntegrationNginxLatestReturn(t *testing.T) {
 }
 ```

+### Advanced Settings
+
+The aforementioned `GenericContainer` function and the `ContainerRequest` struct represent a straightforward manner to configure the containers, but you could need to create your containers with more advance settings regarding the config, host config and endpoint settings Docker types. For those more advance settings, _Testcontainers for Go_ offers a way to fully customise the container request and those internal Docker types. These customisations, called _modifiers_, will be applied just before the internal call to the Docker client to create the container.


Suggested change

The aforementioned `GenericContainer` function and the `ContainerRequest` struct represent a straightforward manner to configure the containers, but you could need to create your containers with more advance settings regarding the config, host config and endpoint settings Docker types. For those more advance settings, _Testcontainers for Go_ offers a way to fully customise the container request and those internal Docker types. These customisations, called _modifiers_, will be applied just before the internal call to the Docker client to create the container.

The aforementioned `GenericContainer` function and the `ContainerRequest` struct represent a straightforward manner to configure the containers, but you might need to create your containers with more advance settings regarding the config, host config and endpoint settings Docker types. For those more advance settings, _Testcontainers for Go_ offers a way to fully customize the container request and access those internal Docker types. These customizations, called _modifiers_, will be applied just before the internal call to the Docker client to create the container.

kiview · 2023-02-16T10:05:19Z

docs/features/creating_container.md

+<!--/codeinclude-->
+
+!!!warning
+	The only special case where the modifiers are not applied last, is when there are no exposed ports in the container request and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will extract the ports from the underliying Docker image and export them.


Suggested change

The only special case where the modifiers are not applied last, is when there are no exposed ports in the container request and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will extract the ports from the underliying Docker image and export them.

The only special case where the modifiers are not applied last, occurs when there are no exposed ports in the container request and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will implicitly extract the ports from the underlying Docker image and export them.

kiview · 2023-02-16T10:10:57Z

lifecycle_test.go

+	provider, err := NewDockerProvider()
+	require.Nil(t, err)
+
+	t.Run("No exposed ports", func(t *testing.T) {


Suggested change

t.Run("No exposed ports", func(t *testing.T) {

t.Run("No configured exposed ports should expose default port of image", func(t *testing.T) {

kiview · 2023-02-16T10:11:50Z

lifecycle_test.go

+		)
+	})
+
+	t.Run("No exposed ports and network mode IsContainer", func(t *testing.T) {


Suggested change

t.Run("No exposed ports and network mode IsContainer", func(t *testing.T) {

t.Run("No exposed ports and network mode IsContainer should not implicitly expose ports", func(t *testing.T) {

mdelapenya added 14 commits November 22, 2022 07:48

feat: support preconfiguring Docker's host config and endpoint settin…

19d7296

…gs before container creation

chore: deprecate ExtraHosts from the container request

2d5ce5c

chore: deprecate Binds from the container request

0150814

chore: deprecate Tmpfs from the container request

894bcee

chore: deprecate AutoRemove from the container request

0748a89

chore: deprecate Privileged from the container request

d70b26a

chore: deprecate Resources from the container request

064467d

chore: deprecate ShmSize from the container request

793cd61

chore: deprecate CapAdd from the container request

feaec02

chore: deprecate CapDrop from the container request

c327e65

chore: deprecate NetworkMode from the container request

27e1ae6

chore: consistent name for endpointSettings variable

dbfb237

chore: rename callback to hook

595a0b5

Callback suggests execution after an operation. Using hook following what git does with commit hooks

chore: extract default pre-creation hook to a function

6dfd1fd

mdelapenya added the enhancement New feature or request label Nov 22, 2022

mdelapenya self-assigned this Nov 22, 2022

mdelapenya requested a review from a team November 22, 2022 11:38

mdelapenya added 2 commits November 22, 2022 12:44

fix: wording

55f98c9

chore: extract preCreation code to a function

7b2bcdf

kiview reviewed Nov 22, 2022

View reviewed changes

mdelapenya added 2 commits November 22, 2022 17:50

chore: rename methods to use modifier

8a67f21

chore: extract life cycle to a separate file

ec4ba50

Preparation for the future

mdelapenya changed the title ~~chore: move host-config settings to a pre-create hook~~ chore: move host-config settings to a pre-create modifier Nov 22, 2022

mdelapenya added 6 commits November 22, 2022 20:38

fix: update comments

8342a00

chore: push Tmpfs back to layer 1

dd9bd20

chore: push Privileged back to layer 1

0a09cf6

fix: remove outdated comments

b437b88

chore: bring ShmSize back to the first layer

dac0df0

mdelapenya mentioned this pull request Dec 2, 2022

feat: support passing registry credentials to the reaper #647

Merged

mdelapenya added 2 commits January 3, 2023 07:32

chore: separate concerns for modifiers

17b8e1b

mdelapenya changed the title ~~chore: move host-config settings to a pre-create modifier~~ chore: move host-config and endpoint settings to a specific modifiers Jan 4, 2023

mdelapenya added 2 commits January 4, 2023 18:36

fix: typo in variable

66c5f4a

Merge branch 'main' into pre-creation-hook

f2cb127

mdelapenya added 8 commits February 2, 2023 18:27

chore: add modifier for Docker config

a15e33d

In the case of advanced setups, a user would be able to modify anything in the Docker config

chore: unit tests for the preCreateHook

cc267c1

fix: check for the existence of aliases when there are multiple networks

70f89e3

chore: adjust error messages in tests

4fb482b

chore: include mounts in the unit tests

74e32d2

fix: handle error in tests

d7d38ea

fix: handle error in tests

e802e51

Merge branch 'main' into pre-creation-hook

668ad6d

mdelapenya requested review from eddumelendez and HofmeisterAn February 6, 2023 10:11

HofmeisterAn reviewed Feb 14, 2023

View reviewed changes

lifecycle.go Show resolved Hide resolved

mdelapenya added 4 commits February 14, 2023 17:41

chore: execute modifiers the last

f1b6e26

chore: rename file

586b0e2

Revert "chore: execute modifiers the last"

b511560

This reverts commit f1b6e26.

docs: document the modifiers

e1854a8

mdelapenya commented Feb 15, 2023

View reviewed changes

docs/features/creating_container.md Outdated Show resolved Hide resolved

fix: typo

43998c8

mdelapenya marked this pull request as ready for review February 15, 2023 12:12

mdelapenya requested a review from a team as a code owner February 15, 2023 12:12

kiview approved these changes Feb 16, 2023

View reviewed changes

mdelapenya merged commit 34481cf into testcontainers:main Feb 16, 2023

mdelapenya deleted the pre-creation-hook branch February 21, 2023 13:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: move host-config and endpoint settings to a specific modifiers #633

chore: move host-config and endpoint settings to a specific modifiers #633

mdelapenya commented Nov 22, 2022 •

edited

Loading

kiview left a comment

kiview Nov 22, 2022

mdelapenya Nov 22, 2022

kiview Nov 22, 2022

mdelapenya Nov 22, 2022

kiview Nov 22, 2022

kiview Nov 22, 2022

mdelapenya Nov 22, 2022

netlify bot commented Jan 4, 2023 •

edited

Loading

mdelapenya commented Jan 30, 2023

kiview Feb 16, 2023

kiview Feb 16, 2023

kiview Feb 16, 2023

kiview Feb 16, 2023

	The only special case where the modifiers are not applied last, is when there are no exposed ports in the container request and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will extract the ports from the underliying Docker image and export them.
	The only special case where the modifiers are not applied last, occurs when there are no exposed ports in the container request and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will implicitly extract the ports from the underlying Docker image and export them.

	t.Run("No exposed ports", func(t *testing.T) {
	t.Run("No configured exposed ports should expose default port of image", func(t *testing.T) {

	t.Run("No exposed ports and network mode IsContainer", func(t *testing.T) {
	t.Run("No exposed ports and network mode IsContainer should not implicitly expose ports", func(t *testing.T) {

chore: move host-config and endpoint settings to a specific modifiers #633

chore: move host-config and endpoint settings to a specific modifiers #633

Conversation

mdelapenya commented Nov 22, 2022 • edited Loading

What does this PR do?

Why is it important?

Related issues

How to test this PR

kiview left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Jan 4, 2023 • edited Loading

✅ Deploy Preview for testcontainers-go ready!

mdelapenya commented Jan 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdelapenya commented Nov 22, 2022 •

edited

Loading

netlify bot commented Jan 4, 2023 •

edited

Loading