[WFMP-183] Check if podman exists and use that first, otherwise u…

[jamezp]

…se docker.

https://issues.redhat.com/browse/WFMP-183

[vsalbaba]

It would be even better if the actual executable name could be provided as a parameter on the command line, however, I consider this to be better than the current state.

[jamezp]

It would be even better if the actual executable name could be provided as a parameter on the command line, however, I consider this to be better than the current state.

I think we could do something like System.getProperty("wildfly.image.dockerBinary"). I'm not sure that would honor anything defined in the <properties/> section of a POM. The standard properties do not work for complex configuration types in maven.

[vsalbaba]

There should also be updates to the documentation, in many places it implies that docker is the binary that will be used

https://github.com/wildfly/wildfly-maven-plugin/search?q=docker

[jmesnil]

@vsalbaba what would be the name to use to categorise both docker and podman?

[jamezp]

We can't change the <dockerBinary/> without breaking compatibility. I do see some documentation that should be updated, but I don't think we should change the configuration parameters.

[vsalbaba]

I would go with "container engine".
Example:

The operations related to the image are executing using a docker binary container engine. By default, it uses docker but it is possible to configure it with the <image><docker-binary> element to use another binary such as podman.

[vsalbaba]

There is also a hardcoded mention at

wildfly-maven-plugin/tests/standalone-tests/src/test/java/org/wildfly/plugin/provision/ImageTest.java

Lines 45 to 50 in e11d209

	@BeforeClass
	public static void checkDockerInstallation() {
	assumeTrue("Docker is not present in the installation, skipping the tests",
	execSilentWithTimeout(Duration.ofMillis(3000),
	"docker", "-v"));
	}

[jamezp]

There is also a hardcoded mention at

wildfly-maven-plugin/tests/standalone-tests/src/test/java/org/wildfly/plugin/provision/ImageTest.java

Lines 45 to 50 in e11d209

	@BeforeClass
	public static void checkDockerInstallation() {
	assumeTrue("Docker is not present in the installation, skipping the tests",
	execSilentWithTimeout(Duration.ofMillis(3000),
	"docker", "-v"));
	}

Thank you. Just fixed that.

[vsalbaba]

I am considering that changing the default from docker to podman might not be a compatible change, as some users may rely on docker is the default option. To resolve this issue, we can make podman the fallback option and keep docker as the default.

Additionally, I am considering the option of allowing the tests to run with both engines. By using mvn test -Dwildlfy.image.binary=docker, the tests should run with docker, even if podman is installed. Although the magic guess feature is convenient, I would also like to have more control over which engine is used.

[jamezp]

I am considering that changing the default from docker to podman might not be a compatible change, as some users may rely on docker is the default option. To resolve this issue, we can make podman the fallback option and keep docker as the default.

Additionally, I am considering the option of allowing the tests to run with both engines. By using mvn test -Dwildlfy.image.binary=docker, the tests should run with docker, even if podman is installed. Although the magic guess feature is convenient, I would also like to have more control over which engine is used.

I think this makes sense. I'll update the PR once I get a chance.

[jmesnil]

LGTM outside of the name of the sysprop

[jmesnil]

If we use podman first, maybe we should add a INFO log about the actual used binary?

[jamezp]

I do think we should use docker first, then podman to keep the current behavior. I also think it does likely make sense to log the binary being used so the user knows.

[jamezp]

Oh, wait that is already being done :)

getLog().info(format("Building application image %s using %s.", image, this.image.getBinary()));

[jmesnil]

@jamezp that's a good point. And docker remains the most popular binary to build image.

[jmesnil]

the name of the sys prop is now wildfly.image.binary

[rhusar]

Let's find better name even for internals, something like containerTool, containerBinary, podBinary, imageTool etc. Ideas?

[jamezp]

+1, that is what I'm doing. I'm just using binary in the ApplicationImageInfo as it is typically referred to as "image" so "binary" just seemed to make sense.

[jamezp]

FWIW this property cannot be changed as it results in the configuration parameter.

<image>
    <docker-binary>docker</docker-binary>
</image>

[rhusar]

Gotcha, oh well...