Support deploying with docker compose without local docker-compose#462
Support deploying with docker compose without local docker-compose#462fiftin wants to merge 14 commits intotestcontainers:mainfrom fiftin:docker-compose-api
Conversation
compose_executors.go
Outdated
| args := append(cmds, options.Args...) | ||
|
|
||
| req := ContainerRequest{ | ||
| Image: "docker/compose:1.29.2", |
There was a problem hiding this comment.
Wdyt about using a configurable version of compose?
There was a problem hiding this comment.
I replaced ref to function to interface ComposeExecutor with 2 implementations.
Codecov Report
@@ Coverage Diff @@
## main #462 +/- ##
==========================================
- Coverage 65.55% 64.29% -1.27%
==========================================
Files 19 20 +1
Lines 1199 1305 +106
==========================================
+ Hits 786 839 +53
- Misses 305 354 +49
- Partials 108 112 +4
Continue to review full report at Codecov.
|
| return NewDockerCompose(filePaths, identifier, LocalDockerComposeExecutor, nil, opts...) | ||
| } | ||
|
|
||
| func NewContainerizedDockerCompose(filePaths []string, identifier string, opts ...LocalDockerComposeOption) *LocalDockerCompose { |
There was a problem hiding this comment.
Do we still return a LocalDockerCompose? It seems weird to me to use a containerised compose method that returns a LocalDC.
I'd be open to using a dedicated DockerisedDockerCompose type, although I think could come with changes in return types and generalisation...
Of course, not a blocker for this PR, but if you are interested in following-up with more refactors to make a stable API, I'm too
There was a problem hiding this comment.
I started with creating dedicated DockerisedDockerCompose type, but got too many unreasonable duplications. Using current LocalDockerCompose with different executors is a best solution for my opinion.
IMHO: interface DockerCompose should be removed and type LocalDockerCompose should renamed to DockerCompose, new interface ComposeExecutor should be used for extensions.
Alternatively:
-
Rename type
LocalDockerComposetoDockerComposeXYZ. -
Define type
LocalDockerComposeas alias ofDockerComposeXYZfor back compatibility and mark it as deprecated:
// Deprecated: Use DockerComposeXYZ instead.
type `LocalDockerCompose` `DockerComposeXYZ`As you can see I used existing docker-compose tests for containerized docker-compose and it is works out-of-box :)
compose_test.go
Outdated
| func enumDockerComposes(filePaths []string, identifier string, executor func(compose *LocalDockerCompose), t *testing.T) { | ||
| composes := []*LocalDockerCompose{ | ||
| NewLocalDockerCompose(filePaths, identifier, WithLogger(TestLogger(t))), | ||
| NewContainerizedDockerCompose(filePaths, identifier, WithLogger(TestLogger(t))), |
There was a problem hiding this comment.
I think we are using the new Dockerised compose here. Could you update the docs with examples on how to use it? There is a dedicated section for compose in the docs/features dir.
|
I replaced ref to function to interface Also I applied all |
2 participants
Related to this issue: #425.
I added support conteinerised docker-compose. To do this I refectored
compose.goand extractedComposeExecutorwhich can be:ContainerizedDockerComposeExecutor- uses docker-compose from docker container.LocalDockerComposeExecutor- uses local docker-compose.