PostgreSQLContainer does not honor withCommand

[kellen]

The postgres container does not honor withCommand because it is overridden in configure():

    protected void configure() {

        addExposedPort(POSTGRESQL_PORT);
        addEnv("POSTGRES_DB", databaseName);
        addEnv("POSTGRES_USER", username);
        addEnv("POSTGRES_PASSWORD", password);
        setCommand("postgres");
    }

This seems like it would also be a problem for several other containers using setCommand in configure, including NginxContainer, BrowserWebDriverContainer, and VncRecordingContainer.

In the case of PostgreSQLContainer, the command can just be removed, but in these other cases I'm not sure what the best behavior would be; possibly moving the default setCommand to the constructor or calling it whenever the container's internal configuration changes (e.g. when setting the vncPassword).

[kiview]

Hi @kellen,
yes you are correct regarding the container implementations you've listed.

However I don't think removing the setCommand is the solution.

What about doing setCommand conditionally, if commandParts is empty? This would be kind of more similar to the Docker behaviour. The problem with this approach is, that a user can't override the default command with an "empty" command. I'm not sure if this is real in practice, but can also think about this use case if we want a clean implementation now.

[kellen]

Ok, I was following the model set by #767.

The three cases I can see are:

1. User wants container default command
2. User wants testcontainers default command
3. User wants their own custom command

If the user wants the default container command is the intention that they do not ever call setCommand? Then in the cases of testcontainers which define a default command, they'd need to call setCommand("") or something?

The problem with this approach is, that a user can't override the default command with an "empty" command.

Do you mean the testcontainers default or the underlying docker container default?

[kiview]

Oh you are right, for some containers it's the solution since the underlying image already has the correct command, and I think your PR in #938 is fine, thanks for this.

Your 3 use case also seem to fit. But also we can merge 1 and 3, since 3 is powerful enough to allow 1.

Do you mean the testcontainers default or the underlying docker container default?

In this case I meant the Testcontainers default.

[kellen]

Ok, great.

For the other container types, since configure() is called somewhere down in the start method, they would instead need to be reconfigured when their respective parameters are set (so that the user-set commands don't get overridden). Unless configure() is designed to be called by the end-user?

[kiview]

configure() is a callback that's part of the container lifecycle, so normally it should be called by Testcontainers itself.

Since using the setters should happen before the actual container creation, we can normally assume, that configure() will be called, after the setters have been used by the user.

[kellen]

we can normally assume, that configure() will be called, after the setters have been used by the user.

Ok, so the other usages of setCommand inside of configure() will need to be refactored or they'll have the same (incorrect) behavior of ignoring user configuration.

[rnorth]

Fix was released in 1.12.0. Thanks for the contribution @kellen!