WIP: adds support for pushing to GCR via mvn deploy. Has failing in…
#13
Conversation
|
Here's the build output FWIW, @gimaker ran the tests against this WIP on Linux (I'm using OS X) and he is seeing successful builds. Maybe the test that's failing [ |
|
Does this rely on the Google Cloud SDK being installed? Also, Travis runs inside of GCE so that might screw things up |
| authSupplier = ContainerRegistryAuthSupplier.forApplicationDefaultCredentials() | ||
| .build(); | ||
| getLog().info("Using Google application credentials"); | ||
| } catch (IOException ex) { |
There was a problem hiding this comment.
This needs to catch DockerException as well, which maybe shouldn't be an unchecked exception to begin with? (I realize that would be a breaking change)
There was a problem hiding this comment.
the two methods in the try here can only throw IOException:
it is the methods called at build-/pull-time that might throw DockerException
|
It might make sense to add a system property that can be set when running tests to disable the automatic google credential loading, like -DdisableGoogleCredentials. Seems like the automatic loading would be useful for most developers running the plugin (without them having to opt-in to the behavior), but it needs to be turned off in some cases |
|
@mattnworb I like the current strategy of attempting to load the credentials and falling back to proceeding without them, it's just that the current code has a bug when trying to catch the resulting exception. |
|
Maybe I'm misunderstanding the desired behavior for this case? |
|
@dflemstr did you have any thoughts re: debugging the failing integration test? |
|
spotify/docker-client#773 will fix the case where the loaded credentials aren't scoped to devstorage |
|
testLowercase fails for me on this branch and on the master branch for me on Mac OS X too, although perhaps it has to do with Docker for Mac rather than the test on OS X itself. |
|
This reminds me of other issues I have seen with helios-solo on Docker For Mac - when MainIT calls I believe that on DFM, the external port is connect-able immediately after the container is created, even if the container is not yet listening on the internal port on the other side of the mapping. The fix for this is to add a healthcheck to the TemporaryJob so that can helios can do a higher-level check (can it get a HTTP 2xx response from the port) rather than a tcp-level check. |
need to add a healthcheck to the TemporaryJob to avoid helios-testing from thinking that the job is immediately available after the deploy finishes. see: #13 (comment) spotify/helios#916 (comment)
|
if you bump docker-client to 8.6.2 it should address the other issue mentioned above |
need to add a healthcheck to the TemporaryJob to avoid helios-testing from thinking that the job is immediately available after the deploy finishes. see: #13 (comment) spotify/helios#916 (comment)
|
realized I can commit to the branch directly so now the tests are green ✅ 😄 |
|
@mattnworb fwiw I'm using Docker Machine, not Docker for Mac |
|
@mavenraven can you rebase / resolve the "branch is out of sync" warning? I confirmed with @dflemstr offline that this looks good now. |
…tegration test.
Hey @dflemstr, I'm trying to figure out how to debug the failing integration test. I've tried variations of
mvn -Dmaven.failsafe.debug failsafe:integration-testwithout any success. I also can't figure out how to run these tests via IntelliJ. It'd be awesome if you could help me figure out the failing test, or just fix it if you happen to know what the problem is. Thanks!