-
Notifications
You must be signed in to change notification settings - Fork 645
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
fix: error already existing instance due to case sensitive Path #1847
Conversation
@Amitrei : Thanks for the PR! Is it possible to add a unit test to verify path is getting converted to lowercase as expected? |
Hi, as requested I added a test making sure the builder name is lower cased when looking up for a builder config in the Edit: fixed sonarqube issue related to the test. |
Removed unnecessary eq on verify method due to SonarQube issue
@Amitrei : Thanks, your changes look good to me. could you please add a line regarding this change to doc/changelog.md ? |
Hi Rohan, Thanks again! |
Quality Gate passedIssues Measures |
@Amitrei : Thanks a lot! |
Description
We are using a per-module builder implementation, where the module name is used as the builder name:
<builderName> ${project.artifactId} </builderName>
When specifying a buildx builder name with capital letters, for example:
<builderName> myBuilder </builderName>
,an error occurs if the builder already exists:
ERROR: existing instance for "myBuilder" but no append mode, specify --node to make changes for existing instances
Cause
The
docker buildx create
command stores the builder configuration at the following path:$DOCKER_CONFIG/buildx/instances/mybuilder (Note: the builder name is stored in lowercase).
According to the Docker buildx implementation, docker buildx always stores the builder configuration file name in lowercase.
In the Maven Docker plugin, the following code checks if a builder exists by looking for the builder name inside the instances folder. If the builder doesn't exist, it attempts to create it:
While some file system types are case-insensitive, others are not. This results in the plugin mistakenly attempting to create a builder that already exists. On my macOS, the file system ignores case sensitivity, so the code works fine. However, on our Linux-based CI runners, the "already exists" error is triggered due to case sensitivity.
The Fix
Simplify the builder name lookup by converting the builderName to lowercase when checking the instances directory. This aligns with the behavior of the docker buildx create command, which resolves the issue of case sensitivity.