-
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
add docker machine support #481
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,7 @@ | |
import org.codehaus.plexus.context.ContextException; | ||
import org.codehaus.plexus.personality.plexus.lifecycle.phase.Contextualizable; | ||
import io.fabric8.maven.docker.config.ImageConfiguration; | ||
import io.fabric8.maven.docker.config.MachineConfiguration; | ||
import io.fabric8.maven.docker.config.handler.ImageConfigResolver; | ||
import io.fabric8.maven.docker.log.LogDispatcher; | ||
import io.fabric8.maven.docker.log.LogOutputSpecFactory; | ||
|
@@ -67,9 +68,13 @@ public abstract class AbstractDockerMojo extends AbstractMojo implements Context | |
protected Settings settings; | ||
|
||
// Current maven project | ||
@Parameter(defaultValue= "${session}", readonly = true) | ||
@Parameter(property= "session") | ||
protected MavenSession session; | ||
|
||
// Current mojo execution | ||
@Parameter(property= "mojoExecution") | ||
protected MojoExecution execution; | ||
|
||
// Handler for external configurations | ||
@Component | ||
protected ImageConfigResolver imageConfigResolver; | ||
|
@@ -150,6 +155,10 @@ public abstract class AbstractDockerMojo extends AbstractMojo implements Context | |
@Parameter | ||
private List<ImageConfiguration> images; | ||
|
||
// Docker-machine configuration | ||
@Parameter | ||
private MachineConfiguration machine; | ||
|
||
// Images resolved with external image resolvers and hooks for subclass to | ||
// mangle the image configurations. | ||
private List<ImageConfiguration> resolvedImages; | ||
|
@@ -240,11 +249,26 @@ private void storeInPluginContext(List<ImageConfiguration> resolvedImages, Strin | |
private DockerAccess createDockerAccess(String minimalVersion) throws MojoExecutionException, MojoFailureException { | ||
DockerAccess access = null; | ||
if (isDockerAccessRequired()) { | ||
String dockerUrl = EnvUtil.extractUrl(dockerHost); | ||
boolean isDefaults = machine == null; | ||
if (isDefaults) { | ||
// create instance to resolve system defines | ||
// docker.machine.name and docker.machine.autoCreate | ||
machine = new MachineConfiguration(); | ||
} | ||
isDefaults &= machine.resolveDefines(new PluginParameterExpressionEvaluator(session, execution), getLog()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is is needed to do an extras property resolve step here ? For all other configurations we simply rely on Maven to have the properties resolved before injecting it into the Plugin via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was unable to get injection framework to provide default values for embedded instances. I'd love to be shown how to do this correctly. Chas
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thats true, for deeper configuration objects @parameter is ignored. I used therefor plain field initializers and a constructor to feed the two properties from the outside when the configuration is created because of setting the property. Please look into AbstractDockerMojo how this is resolved. |
||
log.debug(machine.toString()); | ||
if (isDefaults) { | ||
// if no <machine> configuration and no system defines, do not | ||
// use docker-machine | ||
machine = null; | ||
} | ||
|
||
DockerMachine dockerMachine = new DockerMachine(log, machine); | ||
String dockerUrl = dockerMachine.extractUrl(dockerHost); | ||
try { | ||
String version = minimalVersion != null ? minimalVersion : API_VERSION; | ||
access = new DockerAccessWithHcClient("v" + version, dockerUrl, | ||
EnvUtil.getCertPath(certPath), maxConnections, log); | ||
dockerMachine.getCertPath(certPath), maxConnections, log); | ||
access.start(); | ||
setDockerHostAddressProperty(dockerUrl); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change important ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an important change. Aligned with latest recommended practice.
Chas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, looks cleaner. will probably align the other usages, too.