Add the ability to attach a saved image archive as a project artifact #1210

wrosenuance · 2019-04-18T02:12:14Z

This PR is to add the ability to attach an image archive produced by docker:save to the project as an artifact. Currently this is not possible and archives have to be attached using something like build-helper-maven-plugin.

To avoid conflict with docker:source, the default attach classifier is archive (or archive-<saveAlias>), instead of docker (or docker-<alias>) as used by that mojo.

codecov · 2019-04-18T02:13:46Z

Codecov Report

Merging #1210 into master will increase coverage by 0.43%.
The diff coverage is 71.42%.

@@             Coverage Diff              @@
##             master    #1210      +/-   ##
============================================
+ Coverage     53.05%   53.48%   +0.43%     
- Complexity     1532     1553      +21     
============================================
  Files           150      150              
  Lines          7990     7996       +6     
  Branches       1202     1204       +2     
============================================
+ Hits           4239     4277      +38     
+ Misses         3333     3297      -36     
- Partials        418      422       +4

Impacted Files	Coverage Δ	Complexity Δ
...rc/main/java/io/fabric8/maven/docker/SaveMojo.java	`75% <71.42%> (+50.75%)`	`33 <2> (+21)`	⬆️

codecov · 2019-04-18T02:13:46Z

Codecov Report

Merging #1210 into master will increase coverage by 0.43%.
The diff coverage is 71.42%.

@@             Coverage Diff              @@
##             master    #1210      +/-   ##
============================================
+ Coverage     53.05%   53.48%   +0.43%     
- Complexity     1532     1553      +21     
============================================
  Files           150      150              
  Lines          7990     7996       +6     
  Branches       1202     1204       +2     
============================================
+ Hits           4239     4277      +38     
+ Misses         3333     3297      -36     
- Partials        418      422       +4

Impacted Files	Coverage Δ	Complexity Δ
...rc/main/java/io/fabric8/maven/docker/SaveMojo.java	`75% <71.42%> (+50.75%)`	`33 <2> (+21)`	⬆️

codecov · 2019-04-18T02:13:46Z

Codecov Report

Merging #1210 into master will increase coverage by 0.18%.
The diff coverage is 91.66%.

@@             Coverage Diff              @@
##             master    #1210      +/-   ##
============================================
+ Coverage     53.33%   53.51%   +0.18%     
+ Complexity     1595     1554      -41     
============================================
  Files           154      150       -4     
  Lines          8166     7998     -168     
  Branches       1249     1204      -45     
============================================
- Hits           4355     4280      -75     
+ Misses         3379     3296      -83     
+ Partials        432      422      -10

Impacted Files	Coverage Δ	Complexity Δ
...rc/main/java/io/fabric8/maven/docker/SaveMojo.java	`77.02% <91.66%> (+52.78%)`	`34 <3> (+22)`	⬆️
...8/maven/docker/config/BuildImageConfiguration.java	`84.44% <0%> (-0.26%)`	`71% <0%> (-1%)`
...config/handler/property/PropertyConfigHandler.java	`74.89% <0%> (-0.11%)`	`119% <0%> (-1%)`
...aven/docker/config/handler/property/ConfigKey.java	`100% <0%> (ø)`	`10% <0%> (ø)`	⬇️
...abric8/maven/docker/config/ArchiveCompression.java	`61.9% <0%> (ø)`	`8% <0%> (ø)`	⬇️
...ven/docker/access/hc/DockerAccessWithHcClient.java	`14.23% <0%> (ø)`	`13% <0%> (ø)`	⬇️
...io/fabric8/maven/docker/util/ImageArchiveUtil.java
.../io/fabric8/maven/docker/util/NamePatternUtil.java
...docker/model/ImageArchiveManifestEntryAdapter.java
...aven/docker/model/ImageArchiveManifestAdapter.java
... and 2 more

codecov · 2019-04-18T02:13:46Z

Codecov Report

Merging #1210 into master will increase coverage by 0.43%.
The diff coverage is 71.42%.

@@             Coverage Diff              @@
##             master    #1210      +/-   ##
============================================
+ Coverage     53.05%   53.48%   +0.43%     
- Complexity     1532     1553      +21     
============================================
  Files           150      150              
  Lines          7990     7996       +6     
  Branches       1202     1204       +2     
============================================
+ Hits           4239     4277      +38     
+ Misses         3333     3297      -36     
- Partials        418      422       +4

Impacted Files	Coverage Δ	Complexity Δ
...rc/main/java/io/fabric8/maven/docker/SaveMojo.java	`75% <71.42%> (+50.75%)`	`33 <2> (+21)`	⬆️

Signed-off-by: William Rose <william.rose@nuance.com>

rhuss

Looks good, but i have a minor suggestion how to model the configuration.

rhuss · 2019-04-18T09:39:29Z

src/main/java/io/fabric8/maven/docker/SaveMojo.java

+ private boolean saveAttach;
+
+ @Parameter(property = "docker.save.classifier")
+ private String saveClassifier;


I wonder whether we could combine both properties (as I'm always trying to keep the configuration surface small). What's about having a single 'saveAttach' string property, which can be 'true/false' (then a default classifier is select for 'true') or any other value in which case it enables the featue and uses that classifier ?

We can, although I always worry about making something that looks boolean, but isn't, because of previous mockery.

I could make specifying a non-empty saveClassifier mean "attach with this classifier", and not provide the default logic, since by the time you write <saveAttach>true</saveAttach>, you probably could have written <saveClassifier>archive</saveClassifier>, and we could use %a for the alias (as in <saveClassifier>archive-%a</saveClassifier> or -Ddocker.save.classifier=archive-%a)?

We can, although I always worry about making something that looks boolean, but isn't, because of previous mockery.

I could make specifying a non-empty saveClassifier mean "attach with this classifier", and not provide the default logic, since by the time you write <saveAttach>true</saveAttach>, you probably could have written <saveClassifier>archive</saveClassifier>, and we could use %a for the alias (as in <saveClassifier>archive-%a</saveClassifier> or -Ddocker.save.classifier=archive-%a)?

Fair enough. I think having no default name is fine as adding one is trivial (also saves the user a documentation lookup for finding out what the default would be).

Signed-off-by: William Rose <william.rose@nuance.com>

wrosenuance · 2019-04-18T15:03:21Z

I added a commit to remove saveAttach in favour of just saveClassifier. I didn't squash this with the previous commit yet as I think if you merge PR #1208 I will need to resolve a conflict in the changelog, so I can squash it all then.

…hives. This PR implements the proposed pattern matching that scans the archive prior to loading and then creates a tag from the image name loaded from the archive to the image name configured in the Maven project. Signed-off-by: William Rose <william.rose@nuance.com> Add the ability to attach a saved image archive as a project artifact. Signed-off-by: William Rose <william.rose@nuance.com> PR fabric8io#1210 Remove saveAttach and just use saveClassifier. Signed-off-by: William Rose <william.rose@nuance.com> Issue fabric8io#1207 Specify image name pattern when loading from archives. This PR implements the proposed pattern matching that scans the archive prior to loading and then creates a tag from the image name loaded from the archive to the image name configured in the Maven project. Signed-off-by: William Rose <william.rose@nuance.com>

rhuss

Thanks ! lgtm

wrosenuance force-pushed the feature-save-attach branch from 52f88f0 to b64b4e8 Compare April 18, 2019 02:14

Add the ability to attach a saved image archive as a project artifact.

9e2bf69

Signed-off-by: William Rose <william.rose@nuance.com>

wrosenuance force-pushed the feature-save-attach branch from b64b4e8 to 9e2bf69 Compare April 18, 2019 02:19

rhuss reviewed Apr 18, 2019

View reviewed changes

PR fabric8io#1210 Remove saveAttach and just use saveClassifier.

bbf7773

Signed-off-by: William Rose <william.rose@nuance.com>

wrosenuance force-pushed the feature-save-attach branch from 4752423 to bbf7773 Compare April 18, 2019 14:38

wrosenuance mentioned this pull request Apr 18, 2019

Find the name of an image in an archive using a pattern #1208

Merged

Merge branch 'master' into feature-save-attach

fd337b1

rhuss approved these changes Apr 21, 2019

View reviewed changes

rhuss merged commit 885954a into fabric8io:master Apr 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the ability to attach a saved image archive as a project artifact #1210

Add the ability to attach a saved image archive as a project artifact #1210

wrosenuance commented Apr 18, 2019

codecov bot commented Apr 18, 2019

codecov bot commented Apr 18, 2019

codecov bot commented Apr 18, 2019 •

edited

Loading

codecov bot commented Apr 18, 2019

rhuss left a comment

rhuss Apr 18, 2019

wrosenuance Apr 18, 2019

rhuss Apr 21, 2019

wrosenuance commented Apr 18, 2019

rhuss left a comment

Add the ability to attach a saved image archive as a project artifact #1210

Add the ability to attach a saved image archive as a project artifact #1210

Conversation

wrosenuance commented Apr 18, 2019

codecov bot commented Apr 18, 2019

Codecov Report

codecov bot commented Apr 18, 2019

Codecov Report

codecov bot commented Apr 18, 2019 • edited Loading

Codecov Report

codecov bot commented Apr 18, 2019

Codecov Report

rhuss left a comment

Choose a reason for hiding this comment

rhuss Apr 18, 2019

Choose a reason for hiding this comment

wrosenuance Apr 18, 2019

Choose a reason for hiding this comment

rhuss Apr 21, 2019

Choose a reason for hiding this comment

wrosenuance commented Apr 18, 2019

rhuss left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 18, 2019 •

edited

Loading