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

skipPom is ignored by "push" goal #1482

Closed
edrandall opened this issue Jul 3, 2021 · 8 comments · Fixed by #1525
Closed

skipPom is ignored by "push" goal #1482

edrandall opened this issue Jul 3, 2021 · 8 comments · Fixed by #1525
Assignees
Labels

Comments

@edrandall
Copy link
Contributor

edrandall commented Jul 3, 2021

Description

Have a multi-module project with docker-maven-plugin configuration in the images-parent module, which has packaging "pom".
Each subdirectory module has a simple minimal pom.xml with packaging "jar" and Dockerfile to build an image.
The d-m-p configuration has skipPom=true configured
The "build" goal is bound to "package" amd "push" is bound to "install".

During the "build" goal, the pom with packaging=pom is correctly skipped as expected.
unfortunately the "push" goal ignores skipPom and attempts to run in the parent pom.

[INFO] --- docker-maven-plugin:0.36.0:build (docker-build) @  images-parent  ---
[INFO] DOCKER> [ images-parent :1-0-0-snapshot] : Skipped building
[INFO]

[INFO] --- docker-maven-plugin:0.36.0:push (repository-push) @ images-parent ---
[ERROR] DOCKER> Unable to add tag [our.registry.com/docker/images/images-parent:1-0-0-snapshot] to image [images-parent:1-0-0-snapshot] : {"message":"No such image: images-parent:1-0-0-snapshot"} (Not Found: 404) [{"message":"No such image: images-parent:1-0-0-snapshot"} (Not Found: 404)]

Info

  • docker-maven-plugin version :0.36.0
  • Maven version (mvn -v) :
Apache Maven 3.8.1 (05c21c65bdfed0f71a2f2ada8b84da59348c4c5d)
Maven home: ~/Apps/apache-maven-3.8.1
Java version: 11.0.10, vendor: AdoptOpenJDK, runtime: /Library/Java/JavaVirtualMachines/adoptopenjdk-11.jdk/Contents/Home
Default locale: en_GB, platform encoding: UTF-8
OS name: "mac os x", version: "10.16", arch: "x86_64", family: "mac"
  • Docker version :
Client:
 Cloud integration: 1.0.17
 Version:           20.10.7
 API version:       1.41
 Go version:        go1.16.4
 Git commit:        f0df350
 Built:             Wed Jun  2 11:56:22 2021
 OS/Arch:           darwin/amd64
 Context:           desktop-linux
 Experimental:      true

Server: Docker Engine - Community
 Engine:
  Version:          20.10.7
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       b0f5bc3
  Built:            Wed Jun  2 11:54:58 2021
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.4.6
  GitCommit:        d71fcd7d8303cbf684402823e425e9dd2e99285d
 runc:
  Version:          1.0.0-rc95
  GitCommit:        b9ee9c6314599f1b4a7f497e1f1f856fe433d3b7
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0
  • If it's a bug, how to reproduce :
    In images-parent:
	<modules>
		<module>child</module>
	</modules>

	<build>
		<plugins>
			<plugin>
				<groupId>io.fabric8</groupId>
				<artifactId>docker-maven-plugin</artifactId>
				<extensions>true</extensions>
				<executions>
					<execution>
						<id>docker-build</id>
						<phase>package</phase>
						<goals>
							<goal>build</goal>
						</goals>
						<configuration>
							<images>
								<image>
									<name>%a:${docker.release.tag}</name>
									<build>
										<contextDir>${docker.contextDir}</contextDir>
										<assembly>
											<name>dependencies</name>
											<descriptorRef>dependencies</descriptorRef>
										</assembly>
										<cleanup>try</cleanup>
										<filter>@</filter>
									</build>
								</image>
							</images>
						</configuration>
					</execution>
					<execution>
						<id>repository-push</id>
						<phase>install</phase>
						<goals>
							<goal>push</goal>
						</goals>
						<configuration>
							<images>
								<image>
									<name>%a:${docker.release.tag}</name>
									<build/>
								</image>
							</images>
						</configuration>
					</execution>
				</executions>
				<configuration>
					<dockerHost>${docker.host}</dockerHost>
					<certPath>${docker.certPath}</certPath>
					<sourceDirectory>.</sourceDirectory>
					<imagePullPolicy>IfNotPresent</imagePullPolicy>
					<pushRegistry>${docker.target.repository}</pushRegistry>
					<authConfig>
						<username>${docker.auth.user}</username>
						<password>${docker.auth.pass}</password>
					</authConfig>
					<retries>1</retries>
					<skipPush>${docker.skip.push}</skipPush>
					<skipPom>true</skipPom>
					<useColor>true</useColor>
					<verbose>true</verbose>
				</configuration>
			</plugin>
		</build>

The pom for "child" has minimal configuration, the idea is it's inheriting all build config from its parent.

The workaround for the moment is to put everything in the parent into pluginManagement and add a build section to every child:

	<build>
		<plugins>
			<plugin>
				<groupId>io.fabric8</groupId>
				<artifactId>docker-maven-plugin</artifactId>
			</plugin>
		</plugins>
	</build>

* If it's a feature request, what is your use case :
* Sample project : *[GitHub Clone URL]*
@rohanKanojia
Copy link
Member

rohanKanojia commented Jul 4, 2021

I see a skipPom check in BuildMojo:

if(buildConfig.skip() || (skipPom && packaging.equalsIgnoreCase("pom"))) {
log.info("%s : Skipped building", aImageConfig.getDescription());

However, I don't see any check like this in PushMojo. I haven't tried out your sample but maybe adding such a check in PushMojo should fix this problem. Would it be possible for you to try out locally built SNAPSHOT version of plugin with added skipPom check to see if it works for your sample?

@edrandall
Copy link
Contributor Author

edrandall commented Jul 4, 2021

Yes I could try that, though I'm OoO on holiday for a week now, so may take a while....
Probably it should be consistently handled like this on every goal.

@edrandall
Copy link
Contributor Author

Maybe skipPom should be pushed up to the AbstractDockerMojo level such that it applies uniformly to all goals.

I notice there's also a docker.skip.machine, which I've not previously come across, distinct from docker.skip; There's a difference in the way they are used but the effective difference to the end user is unclear to me -perhaps it's more related to testing the plugin without performing any docker actions?

@edrandall
Copy link
Contributor Author

edrandall commented Jul 4, 2021

This appears to work, though I am unsure of any wider implications
skippom.patch.gz

Reverse the packaging check to "pom".equalsIgnoreCase(packaging) to avoid a theoretical NPE.
The error message needs improvement.

@rohanKanojia
Copy link
Member

I think you're right. This docker machine support seems to be used for scenarios where docker host need to be accessed/created by docker-machine tool. I'm not sure whether it's related to generic skip parameters like docker.skip/docker.skip.pom

@rohanKanojia
Copy link
Member

@edrandall : polite ping, Are you back from vacation? Do you have some time to fix this? Otherwise, I can also pick this up..

@edrandall
Copy link
Contributor Author

@rohanKanojia I'm back, but I've moved to a new team and quite busy... the patch above seemed to be working fine, for the benefit of my old team.

@rohanKanojia
Copy link
Member

ohk, I'll take care of this.

@rohanKanojia rohanKanojia self-assigned this Jul 29, 2021
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Aug 15, 2021
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Feb 6, 2022
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Feb 6, 2022
rohanKanojia added a commit to rohanKanojia/docker-maven-plugin that referenced this issue Feb 6, 2022
rohanKanojia added a commit that referenced this issue Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants