Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: cannot replace command (or any other enumerable) #1227

Open
johnnyggalt opened this issue Aug 6, 2024 · 6 comments
Open

[Bug]: cannot replace command (or any other enumerable) #1227

johnnyggalt opened this issue Aug 6, 2024 · 6 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@johnnyggalt
Copy link

johnnyggalt commented Aug 6, 2024

Testcontainers version

3.9.0

Using the latest Testcontainers version?

Yes

Host OS

Windows

Host arch

x86

.NET version

8.0.303

Docker version

Client:
 Version:           27.0.3
 API version:       1.46
 Go version:        go1.21.11
 Git commit:        7d4bcd8
 Built:             Sat Jun 29 00:03:32 2024
 OS/Arch:           windows/amd64
 Context:           desktop-linux

Server: Docker Desktop 4.32.0 (157355)
 Engine:
  Version:          27.0.3
  API version:      1.46 (minimum version 1.24)
  Go version:       go1.21.11
  Git commit:       662f78c
  Built:            Sat Jun 29 00:02:50 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.7.18
  GitCommit:        ae71819c4f5e67bb4d5ae76a6b735f29cc25774e
 runc:
  Version:          1.7.18
  GitCommit:        v1.1.13-0-g58aa920
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Docker info

Client:
 Version:    27.0.3
 Context:    desktop-linux
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.15.1-desktop.1
    Path:     C:\Program Files\Docker\cli-plugins\docker-buildx.exe
  compose: Docker Compose (Docker Inc.)
    Version:  v2.28.1-desktop.1
    Path:     C:\Program Files\Docker\cli-plugins\docker-compose.exe
  debug: Get a shell into any image or container (Docker Inc.)
    Version:  0.0.32
    Path:     C:\Program Files\Docker\cli-plugins\docker-debug.exe
  desktop: Docker Desktop commands (Alpha) (Docker Inc.)
    Version:  v0.0.14
    Path:     C:\Program Files\Docker\cli-plugins\docker-desktop.exe
  dev: Docker Dev Environments (Docker Inc.)
    Version:  v0.1.2
    Path:     C:\Program Files\Docker\cli-plugins\docker-dev.exe
  extension: Manages Docker extensions (Docker Inc.)
    Version:  v0.2.25
    Path:     C:\Program Files\Docker\cli-plugins\docker-extension.exe
  feedback: Provide feedback, right in your terminal! (Docker Inc.)
    Version:  v1.0.5
    Path:     C:\Program Files\Docker\cli-plugins\docker-feedback.exe
  init: Creates Docker-related starter files for your project (Docker Inc.)
    Version:  v1.3.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-init.exe
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc.)
    Version:  0.6.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-sbom.exe
  scout: Docker Scout (Docker Inc.)
    Version:  v1.10.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-scout.exe

Server:
 Containers: 8
  Running: 5
  Paused: 0
  Stopped: 3
 Images: 24
 Server Version: 27.0.3
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: ae71819c4f5e67bb4d5ae76a6b735f29cc25774e
 runc version: v1.1.13-0-g58aa920
 init version: de40ad0
 Security Options:
  seccomp
   Profile: unconfined
 Kernel Version: 5.15.146.1-microsoft-standard-WSL2
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 8
 Total Memory: 31.31GiB
 Name: docker-desktop
 ID: f2376507-5297-4eca-ae87-0b375ce30fd4
 Docker Root Dir: /var/lib/docker
 Debug Mode: true
  File Descriptors: 110
  Goroutines: 161
  System Time: 2024-08-06T04:28:03.00615065Z
  EventsListeners: 13
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Labels:
  com.docker.desktop.address=npipe://\\.\pipe\docker_cli
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5555
  splinter:5000
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: No blkio throttle.read_bps_device support
WARNING: No blkio throttle.write_bps_device support
WARNING: No blkio throttle.read_iops_device support
WARNING: No blkio throttle.write_iops_device support
WARNING: daemon is not using the default seccomp profile

What happened?

When building a container - any container, but in my case Keycloak - specifying a different command results in merging. If the ContainerBuilder already defines a "default" command, there's seemingly no way to replace that command completely. For example:

let keycloakContainer =
    KeycloakBuilder()
        .WithImage("...")
        .WithCommand("start")
        ...
        .Build()

This will result in an attempt to run the command start-dev start rather than the desired start because KeycloakBuilder.Init already invokes .WithCommand("start-dev"). Moreover, I cannot find a way to supplant the default like you can with non-enumerable values. The easiest thing to do for a quick workaround would be to inherit KeycloakBuilder and override Init so that it doesn't invoke WithCommand, but KeycloakBuilder is sealed.

Relevant log output

No response

Additional information

No response

@johnnyggalt johnnyggalt added the bug Something isn't working label Aug 6, 2024
@HofmeisterAn
Copy link
Collaborator

Hi, at this point, this is intentional. Modules are pre-configured, opinionated, and follow best practices (configurations) driven by the community. The builder API is "immutable" to support A/B testing, it is currently not possible to override (remove) configurations for most elements that rely on lists or dictionaries (due to the way the BuildConfiguration works).

Supporting a feature that allows developers to entirely override the configuration would likely be a good enhancement, but it should be implemented explicitly. There are many variations that could potentially break the pre-configured module configuration, and developers should be aware that this could easily cause issues.

However, if a module's configuration does not suit your use case, you can always fall back to the generic builder and configure it yourself.

@HofmeisterAn HofmeisterAn added the enhancement New feature or request label Aug 10, 2024
@johnnyggalt
Copy link
Author

Thanks @HofmeisterAn ...

you can always fall back to the generic builder and configure it yourself

Can you please clarify this? Do you mean to extend ContainerBuilder<TBuilderEntity, TContainerEntity, TConfigurationEntity> myself? If so, that's what I've done as a workaround, but it turned into quite a thing as compared to simply overriding a deafult value or, absent that ability, inheriting from KeycloakBuilder and overriding its behavior.

@HofmeisterAn
Copy link
Collaborator

Can you please clarify this? Do you mean to extend ContainerBuilder<TBuilderEntity, TContainerEntity, TConfigurationEntity> myself?

Yes, this is what I meant.

I've done as a workaround, but it turned into quite a thing as compared to simply overriding a deafult value or, absent that ability, inheriting from KeycloakBuilder and overriding its behavior.

I understand, but it is difficult to provide the desired flexibility, supporting all kinds of configurations and maintaining reliability at the same time.

@johnnyggalt
Copy link
Author

johnnyggalt commented Aug 12, 2024

Got it. Just for the record, I decided to use reflection to reduce the amount of code. It's in my test fixture anyway, so not a huge deal.

type KeycloakBuilder with
    member this.WithOptimizedStartup() =
        let keycloakConfiguration =
            this
                .GetType()
                .GetProperty("DockerResourceConfiguration", BindingFlags.Instance ||| BindingFlags.NonPublic ||| BindingFlags.DeclaredOnly)
                .GetValue(this)
                :?> KeycloakConfiguration

        if keycloakConfiguration = null then
            failwith "Failed to resolve DockerResourceConfiguration property on KeycloakConfiguration"

        let commandField =
            keycloakConfiguration
                .GetType()
                .BaseType
                .GetFields(BindingFlags.Instance ||| BindingFlags.NonPublic)
            |> Array.tryFind _.Name.StartsWith("<Command>")
            |> Option.toObj

        if commandField = null then
            failwith "Failed to resolve backing field for ContainerConfiguration.Command property"

        commandField.SetValue(keycloakConfiguration, [ "start"; "--optimized" ] |> box)
        this

@HofmeisterAn
Copy link
Collaborator

If you like, we can keep the issue open. IMO, it is a valid feature request to be able to override module configurations. With the current design, overriding some parts is very tricky, especially when trying to prevent developers from making misconfigurations. However, we might find a better design that allows developers to properly override module configurations while also creating awareness that with the overridden values, wait strategies, connection strings, etc., may not work as expected anymore.

@johnnyggalt
Copy link
Author

Sure thing, thanks

@johnnyggalt johnnyggalt reopened this Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants