Skip to content
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

Closed
wants to merge 1 commit into from
Closed

add docker machine support #481

wants to merge 1 commit into from

Conversation

chonton
Copy link
Contributor

@chonton chonton commented Jun 11, 2016

This pull request replaces #437.

Add a configuration section <machine> which configures use of docker-machine. The precedence order of configuration is

  1. dockerHost / certPath
  2. <machine>
  3. environment variables
  4. fallbacks (unix:///var/run/docker.sock / ~/.docker/)

<machine> has three sub-elements:

  • name - the name of the docker-machine
  • autoCreate - create the docker-machine if not present
  • creatOptions - the options used when creating a docker-machine

To use the <machine> element, docker-machine must be installed on the build host.
Before querying docker-machine for daemon values, the docker-machine will be started if necessary.

@rhuss
Copy link
Collaborator

rhuss commented Jun 27, 2016

Thanks again ;) Just about to integrate it into the code base (already rebased it), but I have still some questions, though

@@ -67,9 +68,13 @@
protected Settings settings;

// Current maven project
@Parameter(defaultValue= "${session}", readonly = true)
@Parameter(property= "session")
Copy link
Collaborator

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 ?

Copy link
Contributor Author

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

On Jun 26, 2016, at 6:29 PM, Roland Huß notifications@github.com wrote:

In src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java:

@@ -67,9 +68,13 @@
protected Settings settings;

 // Current maven project
  • @parameter(defaultValue= "${session}", readonly = true)
  • @parameter(property= "session")
    Why is this change important ?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Copy link
Collaborator

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.

@rhuss
Copy link
Collaborator

rhuss commented Jun 27, 2016

I'm nearly finished with including your patch and push it soon to branch integration. There will probably be a release today.

'hope my changes are ok for you (tuned a bit documentation, logging, class names ...)

rhuss added a commit that referenced this pull request Jun 27, 2016
rhuss added a commit that referenced this pull request Jun 27, 2016
rhuss added a commit that referenced this pull request Jun 27, 2016
@chonton
Copy link
Contributor Author

chonton commented Jun 27, 2016

Any change ok by me. Thanks for your dedication in supporting this plugin.

Chas

On Jun 26, 2016, at 9:14 PM, Roland Huß notifications@github.com wrote:

I'm nearly finished with including your patch and push it soon to branch integration. There will probably be a release today.

'hope my changes are ok for you (tuned a bit documentation, logging, class names ...)


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@rhuss
Copy link
Collaborator

rhuss commented Jun 27, 2016

I thank you for the contribution, its really awesome to have docker machine support now, which makes automation even easier. ;-)

@rhuss
Copy link
Collaborator

rhuss commented Jun 27, 2016

As it is now in integration I'm closing this PR. For unit tests please open a new one.

thanks ...

@rhuss rhuss closed this Jun 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants