Check FS mounting lazily #875
Conversation
| } catch (Exception e) { | ||
| VisibleAssertions.fail("File should be mountable but fails with " + e.getMessage()); | ||
| log.debug("Failed to check mountable file", e); |
There was a problem hiding this comment.
Probably better to rephrase the message: Failure while checking for mountable file support
There was a problem hiding this comment.
So you want to keep the message? 😆
| @@ -455,6 +456,12 @@ private void applyConfiguration(CreateContainerCmd createCommand) { | |||
| .toArray(String[]::new); | |||
| createCommand.withEnv(envArray); | |||
|
|
|||
| if (binds.size() > 0 && !TestcontainersConfiguration.getInstance().isDisableChecks()) { | |||
There was a problem hiding this comment.
Maybe extract to boolean variable shouldCheckFileMountingSupport
| List<Bind> binds = new ArrayList<>(); | ||
| binds.add(new Bind("//var/run/docker.sock", new Volume("/var/run/docker.sock"))); | ||
| if (withDummyMount) { | ||
| // Not needed for Ryuk, but we perform pre-flight checks with it (micro optimization) | ||
| binds.add(new Bind(mountableFile.getResolvedPath(), new Volume("/dummy"), AccessMode.ro)); |
There was a problem hiding this comment.
Very nice, that we can remove this!
| @@ -455,6 +456,12 @@ private void applyConfiguration(CreateContainerCmd createCommand) { | |||
| .toArray(String[]::new); | |||
| createCommand.withEnv(envArray); | |||
|
|
|||
| if (binds.size() > 0 && !TestcontainersConfiguration.getInstance().isDisableChecks()) { | |||
| if (!DockerClientFactory.instance().isSupportsFileMounting()) { | |||
| VisibleAssertions.warn("Misconfigured file mounting. Some features might not work."); | |||
There was a problem hiding this comment.
Maybe we could be very specific in the warning; something like: Unable to mount a file from test host into a running container. This may be a misconfiguration or limitation of your Docker environment. Some features might not work.
Additionally, is it right for this to be a warning, or should we throw an exception? If we're here, it's because the user has tried to do something that will not work, right? Or is it possible for this check to fail but the user's requested FS binds to work?
There was a problem hiding this comment.
If user is mounting /var/run/docker.sock, it will still work, this is why I decided to use warn :)
|
Sorry I didn't manage to review earlier today :( Same as @kiview; please treat my comments as you see fit and merge when you want - no need for a lot of back-and-forth comments on this 😄 |
No description provided.