From 73878e02b5a1be2cdea24a11c4e46c83fa67144c Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Fri, 8 Nov 2019 07:54:21 +0100 Subject: [PATCH] Bugfix/#1751 openshift docker s2i (#1753) * WIP: Code implementation completed * Refactored affected tests and included new test cases * Added changelog entry * Changelog typo --- CHANGELOG.md | 1 + .../openshift/OpenshiftBuildService.java | 12 +- .../openshift/OpenshiftBuildServiceTest.java | 551 +++++++++--------- .../maven/plugin/mojo/build/BuildMojo.java | 4 +- 4 files changed, 285 insertions(+), 283 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67cfb44446..13ab0a7a76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ After this we will switch probably to real [Semantic Versioning 2.0.0](http://se * Feature #1748: Quarkus generator: Extract base image from configuration. * Fix #1695: IllegalArgumentException when Spring Boot application.yaml contains integer keys * Fix #797: spring-boot generator can not handle multi-profile configuration +* Fix #1751: Build Names are suffixed with -*s2i regardless of build strategy ### 4.3.1 (18-10-2019) * Updated Kubernetes client to 4.6.1 diff --git a/core/src/main/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildService.java b/core/src/main/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildService.java index ccf955c5e3..c1196c6da6 100644 --- a/core/src/main/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildService.java +++ b/core/src/main/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildService.java @@ -83,6 +83,8 @@ */ public class OpenshiftBuildService implements BuildService { + private static final String DEFAULT_S2I_BUILD_SUFFIX = "-s2i"; + private final OpenShiftClient client; private final Logger log; private ServiceHub dockerServiceHub; @@ -309,7 +311,7 @@ private BuildStrategy createBuildStrategy(ImageConfiguration imageConfig, OpenSh .withNoCache(checkForNocache(imageConfig)) .withEnv(checkForEnv(imageConfig)) .endDockerStrategy().build(); - + if (openshiftPullSecret != null) { buildStrategy.getDockerStrategy().setPullSecret(new LocalObjectReferenceBuilder() .withName(openshiftPullSecret) @@ -698,7 +700,13 @@ private void addImageStreamToFile(File imageStreamFile, ImageName imageName, Ope // == Utility methods ========================== private String getS2IBuildName(BuildServiceConfig config, ImageName imageName) { - return imageName.getSimpleName() + config.getS2iBuildNameSuffix(); + final StringBuilder s2IBuildName = new StringBuilder(imageName.getSimpleName()); + if (!StringUtils.isEmpty(config.getS2iBuildNameSuffix())) { + s2IBuildName.append(config.getS2iBuildNameSuffix()); + } else if (config.getOpenshiftBuildStrategy() == OpenShiftBuildStrategy.s2i) { + s2IBuildName.append(DEFAULT_S2I_BUILD_SUFFIX); + } + return s2IBuildName.toString(); } private String getImageStreamName(ImageName name) { diff --git a/core/src/test/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildServiceTest.java b/core/src/test/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildServiceTest.java index 7b2bd92814..2cbc94acba 100644 --- a/core/src/test/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildServiceTest.java +++ b/core/src/test/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildServiceTest.java @@ -22,6 +22,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.KubernetesList; @@ -36,6 +37,7 @@ import io.fabric8.maven.docker.config.BuildImageConfiguration; import io.fabric8.maven.docker.config.ImageConfiguration; import io.fabric8.maven.docker.service.ArchiveService; +import io.fabric8.maven.docker.service.BuildService.BuildContext; import io.fabric8.maven.docker.service.RegistryService; import io.fabric8.maven.docker.service.ServiceHub; import io.fabric8.maven.docker.util.MojoParameters; @@ -55,6 +57,7 @@ import mockit.Mocked; import mockit.Verifications; import org.apache.commons.io.IOUtils; +import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.project.MavenProject; import org.codehaus.plexus.archiver.tar.TarArchiver; import org.junit.Before; @@ -147,183 +150,183 @@ public void init() throws Exception { .dockerMojoParameters(dockerMojoParameters); defaultConfigSecret = new BuildService.BuildServiceConfig.Builder() - .buildDirectory(baseDir) - .buildRecreateMode(BuildRecreateMode.none) - .s2iBuildNameSuffix("-s2i-suffix2") - .openshiftPullSecret("pullsecret-fabric8") - .openshiftBuildStrategy(OpenShiftBuildStrategy.s2i) - .dockerMojoParameters(dockerMojoParameters); + .buildDirectory(baseDir) + .buildRecreateMode(BuildRecreateMode.none) + .dockerBuildContext(context) + .s2iBuildNameSuffix("-s2i-suffix2") + .openshiftPullSecret("pullsecret-fabric8") + .openshiftBuildStrategy(OpenShiftBuildStrategy.s2i) + .dockerMojoParameters(dockerMojoParameters); } @Test public void testSuccessfulBuild() throws Exception { - int nTries = 0; - boolean bTestComplete = false; - do { - try { - nTries++; - BuildService.BuildServiceConfig config = defaultConfig.build(); - WebServerEventCollector collector = createMockServer(config, true, 50, false, false); - OpenShiftMockServer mockServer = collector.getMockServer(); - - DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); - LOG.info("Current write timeout is : {}", client.getHttpClient().writeTimeoutMillis()); - LOG.info("Current read timeout is : {}", client.getHttpClient().readTimeoutMillis()); - LOG.info("Retry on failure : {}", client.getHttpClient().retryOnConnectionFailure()); - OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); - service.build(image); - - // we should Foadd a better way to assert that a certain call has been made - assertTrue(mockServer.getRequestCount() > 8); - collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); - collector.assertEventsNotRecorded("patch-build-config"); - bTestComplete = true; - } catch (Fabric8ServiceException exception) { - Throwable rootCause = getRootCause(exception); - logger.warn("A problem encountered while running test {}, retrying..", exception.getMessage()); - // Let's wait for a while, and then retry again - if (rootCause != null && rootCause instanceof IOException) { - continue; - } - } - } while (nTries < MAX_TIMEOUT_RETRIES && !bTestComplete); + retryInMockServer(() -> { + BuildService.BuildServiceConfig config = defaultConfig.build(); + WebServerEventCollector collector = createMockServer(config, true, 50, false, false); + OpenShiftMockServer mockServer = collector.getMockServer(); + + DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); + LOG.info("Current write timeout is : {}", client.getHttpClient().writeTimeoutMillis()); + LOG.info("Current read timeout is : {}", client.getHttpClient().readTimeoutMillis()); + LOG.info("Retry on failure : {}", client.getHttpClient().retryOnConnectionFailure()); + OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + service.build(image); + + // we should Foadd a better way to assert that a certain call has been made + assertTrue(mockServer.getRequestCount() > 8); + collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); + assertEquals("{\"apiVersion\":\"build.openshift.io/v1\",\"kind\":\"BuildConfig\",\"metadata\":{\"annotations\":{},\"labels\":{},\"name\":\"myapp-s2i-suffix2\"},\"spec\":{\"nodeSelector\":{},\"output\":{\"to\":{\"kind\":\"ImageStreamTag\",\"name\":\"myapp:latest\"}},\"source\":{\"type\":\"Binary\"},\"strategy\":{\"sourceStrategy\":{\"forcePull\":false,\"from\":{\"kind\":\"DockerImage\",\"name\":\"myapp\"}},\"type\":\"Source\"},\"triggers\":[]}}", collector.getBodies().get(1)); + collector.assertEventsNotRecorded("patch-build-config"); + }); + } + + @Test + public void testSuccessfulBuildNoS2iSuffix() throws Exception { + retryInMockServer(() -> { + BuildService.BuildServiceConfig config = defaultConfig + .s2iBuildNameSuffix(null) + .build(); + WebServerEventCollector collector = createMockServer(config, true, 50, false, false); + OpenShiftMockServer mockServer = collector.getMockServer(); + + DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); + LOG.info("Current write timeout is : {}", client.getHttpClient().writeTimeoutMillis()); + LOG.info("Current read timeout is : {}", client.getHttpClient().readTimeoutMillis()); + LOG.info("Retry on failure : {}", client.getHttpClient().retryOnConnectionFailure()); + OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + service.build(image); + + // we should Foadd a better way to assert that a certain call has been made + assertTrue(mockServer.getRequestCount() > 8); + collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); + assertEquals("{\"apiVersion\":\"build.openshift.io/v1\",\"kind\":\"BuildConfig\",\"metadata\":{\"annotations\":{},\"labels\":{},\"name\":\"myapp-s2i\"},\"spec\":{\"nodeSelector\":{},\"output\":{\"to\":{\"kind\":\"ImageStreamTag\",\"name\":\"myapp:latest\"}},\"source\":{\"type\":\"Binary\"},\"strategy\":{\"sourceStrategy\":{\"forcePull\":false,\"from\":{\"kind\":\"DockerImage\",\"name\":\"myapp\"}},\"type\":\"Source\"},\"triggers\":[]}}", collector.getBodies().get(1)); + collector.assertEventsNotRecorded("patch-build-config"); + }); } @Test public void testDockerBuild() throws Exception { - int nTries = 0; - boolean bTestComplete = false; - do { - try { - nTries++; + retryInMockServer(() -> { + final io.fabric8.maven.docker.service.BuildService.BuildContext context + = new io.fabric8.maven.docker.service.BuildService.BuildContext.Builder() + .registryConfig(new RegistryService.RegistryConfig.Builder().build()) + .build(); - final io.fabric8.maven.docker.service.BuildService.BuildContext context - = new io.fabric8.maven.docker.service.BuildService.BuildContext.Builder() - .registryConfig(new RegistryService.RegistryConfig.Builder().build()) - .build(); - - BuildService.BuildServiceConfig.Builder dockerConfig = new BuildService.BuildServiceConfig.Builder() - .buildDirectory(baseDir) - .dockerBuildContext(context) - .buildRecreateMode(BuildRecreateMode.none) - .s2iBuildNameSuffix("-docker") - .openshiftBuildStrategy(OpenShiftBuildStrategy.docker) - .dockerMojoParameters(dockerMojoParameters); - - BuildService.BuildServiceConfig config = dockerConfig.build(); - WebServerEventCollector collector = createMockServer(config, true, 50, - false, false); - OpenShiftMockServer mockServer = collector.getMockServer(); - - DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); - OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); - service.build(image); - - assertTrue(mockServer.getRequestCount() > 8); - collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); - assertEquals("{\"apiVersion\":\"build.openshift.io/v1\",\"kind\":\"BuildConfig\",\"metadata\":{\"annotations\":{},\"labels\":{},\"name\":\"myapp-docker\"},\"spec\":{\"nodeSelector\":{},\"output\":{\"to\":{\"kind\":\"ImageStreamTag\",\"name\":\"myapp:latest\"}},\"source\":{\"type\":\"Binary\"},\"strategy\":{\"dockerStrategy\":{\"from\":{\"kind\":\"DockerImage\",\"name\":\"myapp\"}},\"type\":\"Docker\"},\"triggers\":[]}}", collector.getBodies().get(1)); - collector.assertEventsNotRecorded("patch-build-config"); - bTestComplete = true; - } catch (Fabric8ServiceException exception) { - Throwable rootCause = getRootCause(exception); - logger.warn("A problem encountered while running test {}, retrying..", exception.getMessage()); - // Let's wait for a while, and then retry again - if (rootCause != null && rootCause instanceof IOException) { - continue; - } - } - } while (nTries < MAX_TIMEOUT_RETRIES && !bTestComplete); + BuildService.BuildServiceConfig.Builder dockerConfig = new BuildService.BuildServiceConfig.Builder() + .buildDirectory(baseDir) + .dockerBuildContext(context) + .buildRecreateMode(BuildRecreateMode.none) + .s2iBuildNameSuffix("-docker") + .openshiftBuildStrategy(OpenShiftBuildStrategy.docker) + .dockerMojoParameters(dockerMojoParameters); + + BuildService.BuildServiceConfig config = dockerConfig.build(); + WebServerEventCollector collector = createMockServer(config, true, 50, + false, false); + OpenShiftMockServer mockServer = collector.getMockServer(); + + DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); + OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + service.build(image); + + assertTrue(mockServer.getRequestCount() > 8); + collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); + assertEquals("{\"apiVersion\":\"build.openshift.io/v1\",\"kind\":\"BuildConfig\",\"metadata\":{\"annotations\":{},\"labels\":{},\"name\":\"myapp-docker\"},\"spec\":{\"nodeSelector\":{},\"output\":{\"to\":{\"kind\":\"ImageStreamTag\",\"name\":\"myapp:latest\"}},\"source\":{\"type\":\"Binary\"},\"strategy\":{\"dockerStrategy\":{\"from\":{\"kind\":\"DockerImage\",\"name\":\"myapp\"}},\"type\":\"Docker\"},\"triggers\":[]}}", collector.getBodies().get(1)); + collector.assertEventsNotRecorded("patch-build-config"); + }); + } + + @Test + public void testDockerBuildNoS2iSuffix() throws Exception { + retryInMockServer(() -> { + final BuildContext context = new BuildContext.Builder() + .registryConfig(new RegistryService.RegistryConfig.Builder().build()) + .build(); + final BuildService.BuildServiceConfig.Builder dockerConfig = new BuildService.BuildServiceConfig.Builder() + .buildDirectory(baseDir) + .dockerBuildContext(context) + .buildRecreateMode(BuildRecreateMode.none) + .openshiftBuildStrategy(OpenShiftBuildStrategy.docker) + .dockerMojoParameters(dockerMojoParameters); + final BuildService.BuildServiceConfig config = dockerConfig.build(); + WebServerEventCollector collector = createMockServer(config, true, 50, + false, false); + OpenShiftMockServer mockServer = collector.getMockServer(); + + DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); + OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + service.build(image); + + assertTrue(mockServer.getRequestCount() > 8); + collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); + assertEquals("{\"apiVersion\":\"build.openshift.io/v1\",\"kind\":\"BuildConfig\",\"metadata\":{\"annotations\":{},\"labels\":{},\"name\":\"myapp\"},\"spec\":{\"nodeSelector\":{},\"output\":{\"to\":{\"kind\":\"ImageStreamTag\",\"name\":\"myapp:latest\"}},\"source\":{\"type\":\"Binary\"},\"strategy\":{\"dockerStrategy\":{\"from\":{\"kind\":\"DockerImage\",\"name\":\"myapp\"}},\"type\":\"Docker\"},\"triggers\":[]}}", collector.getBodies().get(1)); + collector.assertEventsNotRecorded("patch-build-config"); + }); } @Test public void testDockerBuildFromExt() throws Exception { - int nTries = 0; - boolean bTestComplete = false; - do { - try { - nTries++; + retryInMockServer(() -> { + final io.fabric8.maven.docker.service.BuildService.BuildContext context + = new io.fabric8.maven.docker.service.BuildService.BuildContext.Builder() + .registryConfig(new RegistryService.RegistryConfig.Builder().build()) + .build(); - final io.fabric8.maven.docker.service.BuildService.BuildContext context - = new io.fabric8.maven.docker.service.BuildService.BuildContext.Builder() - .registryConfig(new RegistryService.RegistryConfig.Builder().build()) - .build(); - - BuildService.BuildServiceConfig.Builder dockerConfig = new BuildService.BuildServiceConfig.Builder() - .buildDirectory(baseDir) - .dockerBuildContext(context) - .buildRecreateMode(BuildRecreateMode.none) - .s2iBuildNameSuffix("-docker") - .openshiftBuildStrategy(OpenShiftBuildStrategy.docker) - .dockerMojoParameters(dockerMojoParameters); - - BuildService.BuildServiceConfig config = dockerConfig.build(); - WebServerEventCollector collector = createMockServer(config, true, 50, - false, false); - OpenShiftMockServer mockServer = collector.getMockServer(); - - DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); - OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); - Map fromExt = ImmutableMap.of("name", "app:1.2-1", - "kind", "ImageStreamTag", - "namespace", "my-project"); - ImageConfiguration fromExtImage = new ImageConfiguration.Builder() - .name(projectName) - .buildConfig(new BuildImageConfiguration.Builder() - .fromExt(fromExt) - .noCache(Boolean.TRUE) - .build() - ).build(); - - service.build(fromExtImage); - - assertTrue(mockServer.getRequestCount() > 8); - collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); - assertEquals("{\"apiVersion\":\"build.openshift.io/v1\",\"kind\":\"BuildConfig\",\"metadata\":{\"annotations\":{},\"labels\":{},\"name\":\"myapp-docker\"},\"spec\":{\"nodeSelector\":{},\"output\":{\"to\":{\"kind\":\"ImageStreamTag\",\"name\":\"myapp:latest\"}},\"source\":{\"type\":\"Binary\"},\"strategy\":{\"dockerStrategy\":{\"from\":{\"kind\":\"ImageStreamTag\",\"name\":\"app:1.2-1\",\"namespace\":\"my-project\"},\"noCache\":true},\"type\":\"Docker\"},\"triggers\":[]}}", collector.getBodies().get(1)); - collector.assertEventsNotRecorded("patch-build-config"); - bTestComplete = true; - } catch (Fabric8ServiceException exception) { - Throwable rootCause = getRootCause(exception); - logger.warn("A problem encountered while running test {}, retrying..", exception.getMessage()); - // Let's wait for a while, and then retry again - if (rootCause != null && rootCause instanceof IOException) { - continue; - } - } - } while (nTries < MAX_TIMEOUT_RETRIES && !bTestComplete); + BuildService.BuildServiceConfig.Builder dockerConfig = new BuildService.BuildServiceConfig.Builder() + .buildDirectory(baseDir) + .dockerBuildContext(context) + .buildRecreateMode(BuildRecreateMode.none) + .s2iBuildNameSuffix("-docker") + .openshiftBuildStrategy(OpenShiftBuildStrategy.docker) + .dockerMojoParameters(dockerMojoParameters); + + BuildService.BuildServiceConfig config = dockerConfig.build(); + WebServerEventCollector collector = createMockServer(config, true, 50, + false, false); + OpenShiftMockServer mockServer = collector.getMockServer(); + + DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); + OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + Map fromExt = ImmutableMap.of("name", "app:1.2-1", + "kind", "ImageStreamTag", + "namespace", "my-project"); + ImageConfiguration fromExtImage = new ImageConfiguration.Builder() + .name(projectName) + .buildConfig(new BuildImageConfiguration.Builder() + .fromExt(fromExt) + .noCache(Boolean.TRUE) + .build() + ).build(); + + service.build(fromExtImage); + + assertTrue(mockServer.getRequestCount() > 8); + collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); + assertEquals("{\"apiVersion\":\"build.openshift.io/v1\",\"kind\":\"BuildConfig\",\"metadata\":{\"annotations\":{},\"labels\":{},\"name\":\"myapp-docker\"},\"spec\":{\"nodeSelector\":{},\"output\":{\"to\":{\"kind\":\"ImageStreamTag\",\"name\":\"myapp:latest\"}},\"source\":{\"type\":\"Binary\"},\"strategy\":{\"dockerStrategy\":{\"from\":{\"kind\":\"ImageStreamTag\",\"name\":\"app:1.2-1\",\"namespace\":\"my-project\"},\"noCache\":true},\"type\":\"Docker\"},\"triggers\":[]}}", collector.getBodies().get(1)); + collector.assertEventsNotRecorded("patch-build-config"); + }); } @Test public void testSuccessfulBuildSecret() throws Exception { - int nTries = 0; - boolean bTestComplete = false; - do { - try { - nTries++; - BuildService.BuildServiceConfig config = defaultConfigSecret.build(); - WebServerEventCollector collector = createMockServer(config, true, 50, false, false); - OpenShiftMockServer mockServer = collector.getMockServer(); - - DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); - LOG.info("Current write timeout is : {}", client.getHttpClient().writeTimeoutMillis()); - LOG.info("Current read timeout is : {}", client.getHttpClient().readTimeoutMillis()); - LOG.info("Retry on failure : {}", client.getHttpClient().retryOnConnectionFailure()); - OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); - service.build(image); - - // we should Foadd a better way to assert that a certain call has been made - assertTrue(mockServer.getRequestCount() > 8); - collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); - collector.assertEventsNotRecorded("patch-build-config"); - bTestComplete = true; - } catch (Fabric8ServiceException exception) { - Throwable rootCause = getRootCause(exception); - logger.warn("A problem encountered while running test {}, retrying..", exception.getMessage()); - // Let's wait for a while, and then retry again - if (rootCause != null && rootCause instanceof IOException) { - continue; - } - } - } while (nTries < MAX_TIMEOUT_RETRIES && !bTestComplete); + retryInMockServer(() -> { + BuildService.BuildServiceConfig config = defaultConfigSecret.build(); + WebServerEventCollector collector = createMockServer(config, true, 50, false, false); + OpenShiftMockServer mockServer = collector.getMockServer(); + + DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.createOpenShiftClient(); + LOG.info("Current write timeout is : {}", client.getHttpClient().writeTimeoutMillis()); + LOG.info("Current read timeout is : {}", client.getHttpClient().readTimeoutMillis()); + LOG.info("Retry on failure : {}", client.getHttpClient().retryOnConnectionFailure()); + OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + service.build(image); + + // we should Foadd a better way to assert that a certain call has been made + assertTrue(mockServer.getRequestCount() > 8); + collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed"); + collector.assertEventsNotRecorded("patch-build-config"); + }); } @Test(expected = Fabric8ServiceException.class) @@ -350,146 +353,131 @@ public void testFailedBuildSecret() throws Exception { @Test public void testSuccessfulSecondBuild() throws Exception { - int nTries = 0; - boolean bTestComplete = false; - do { - try { - nTries++; - BuildService.BuildServiceConfig config = defaultConfig.build(); - WebServerEventCollector collector = createMockServer(config, true, 50, true, true); - OpenShiftMockServer mockServer = collector.getMockServer(); - - OpenShiftClient client = mockServer.createOpenShiftClient(); - OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); - service.build(image); - - assertTrue(mockServer.getRequestCount() > 8); - collector.assertEventsRecordedInOrder("build-config-check", "patch-build-config", "pushed"); - collector.assertEventsNotRecorded("new-build-config"); - bTestComplete = true; - } catch (Fabric8ServiceException exception) { - Throwable rootCause = getRootCause(exception); - logger.warn("A problem encountered while running test {}, retrying..", exception.getMessage()); - // Let's wait for a while, and then retry again - if (rootCause != null && rootCause instanceof IOException) { - continue; - } - } - } while (nTries < MAX_TIMEOUT_RETRIES && !bTestComplete); + retryInMockServer(() -> { + BuildService.BuildServiceConfig config = defaultConfig.build(); + WebServerEventCollector collector = createMockServer(config, true, 50, true, true); + OpenShiftMockServer mockServer = collector.getMockServer(); + + OpenShiftClient client = mockServer.createOpenShiftClient(); + OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + service.build(image); + + assertTrue(mockServer.getRequestCount() > 8); + collector.assertEventsRecordedInOrder("build-config-check", "patch-build-config", "pushed"); + collector.assertEventsNotRecorded("new-build-config"); + }); } @Test public void checkTarPackage() throws Exception { - int nTries = 0; - boolean bTestComplete = false; - do { - try { - nTries++; - BuildService.BuildServiceConfig config = defaultConfig.build(); - WebServerEventCollector collector = createMockServer(config, true, 50, true, true); - OpenShiftMockServer mockServer = collector.getMockServer(); - - OpenShiftClient client = mockServer.createOpenShiftClient(); - final OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); - - ImageConfiguration imageWithEnv = new ImageConfiguration.Builder(image) - .buildConfig(new BuildImageConfiguration.Builder(image.getBuildConfiguration()) - .env(Collections.singletonMap("FOO", "BAR")) - .build() - ).build(); + retryInMockServer(() -> { + BuildService.BuildServiceConfig config = defaultConfig.build(); + WebServerEventCollector collector = createMockServer(config, true, 50, true, true); + OpenShiftMockServer mockServer = collector.getMockServer(); + + OpenShiftClient client = mockServer.createOpenShiftClient(); + final OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + + ImageConfiguration imageWithEnv = new ImageConfiguration.Builder(image) + .buildConfig(new BuildImageConfiguration.Builder(image.getBuildConfiguration()) + .env(Collections.singletonMap("FOO", "BAR")) + .build() + ).build(); - service.createBuildArchive(imageWithEnv); + service.createBuildArchive(imageWithEnv); - final List customizer = new LinkedList<>(); - new Verifications() {{ - archiveService.createDockerBuildArchive(withInstanceOf(ImageConfiguration.class), withInstanceOf(MojoParameters.class), withCapture(customizer)); + final List customizer = new LinkedList<>(); + new Verifications() {{ + archiveService.createDockerBuildArchive(withInstanceOf(ImageConfiguration.class), withInstanceOf(MojoParameters.class), withCapture(customizer)); - assertTrue(customizer.size() == 1); - }}; + assertTrue(customizer.size() == 1); + }}; - customizer.get(0).customize(tarArchiver); + customizer.get(0).customize(tarArchiver); - final List file = new LinkedList<>(); - new Verifications() {{ - String path; - tarArchiver.addFile(withCapture(file), path = withCapture()); + final List file = new LinkedList<>(); + new Verifications() {{ + String path; + tarArchiver.addFile(withCapture(file), path = withCapture()); - assertEquals(".s2i/environment", path); - }}; + assertEquals(".s2i/environment", path); + }}; - assertEquals(1, file.size()); - List lines; - try (FileReader reader = new FileReader(file.get(0))) { - lines = IOUtils.readLines(reader); - } - assertTrue(lines.contains("FOO=BAR")); - bTestComplete = true; - } catch (Fabric8ServiceException exception) { - Throwable rootCause = getRootCause(exception); - logger.warn("A problem encountered while running test {}, retrying..", exception.getMessage()); - // Let's wait for a while, and then retry again - if (rootCause != null && rootCause instanceof IOException) { - continue; - } + assertEquals(1, file.size()); + List lines; + try (FileReader reader = new FileReader(file.get(0))) { + lines = IOUtils.readLines(reader); } - } while (nTries < MAX_TIMEOUT_RETRIES && !bTestComplete); + assertTrue(lines.contains("FOO=BAR")); + }); } @Test public void checkTarPackageSecret() throws Exception { - int nTries = 0; - boolean bTestComplete = false; - do { - try { - nTries++; - BuildService.BuildServiceConfig config = defaultConfigSecret.build(); - WebServerEventCollector collector = createMockServer(config, true, 50, true, true); - OpenShiftMockServer mockServer = collector.getMockServer(); + retryInMockServer(() -> { + BuildService.BuildServiceConfig config = defaultConfigSecret.build(); + WebServerEventCollector collector = createMockServer(config, true, 50, true, true); + OpenShiftMockServer mockServer = collector.getMockServer(); + + OpenShiftClient client = mockServer.createOpenShiftClient(); + final OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + + ImageConfiguration imageWithEnv = new ImageConfiguration.Builder(image) + .buildConfig(new BuildImageConfiguration.Builder(image.getBuildConfiguration()) + .env(Collections.singletonMap("FOO", "BAR")) + .build() + ).build(); - OpenShiftClient client = mockServer.createOpenShiftClient(); - final OpenshiftBuildService service = new OpenshiftBuildService(client, logger, dockerServiceHub, config); + service.createBuildArchive(imageWithEnv); - ImageConfiguration imageWithEnv = new ImageConfiguration.Builder(image) - .buildConfig(new BuildImageConfiguration.Builder(image.getBuildConfiguration()) - .env(Collections.singletonMap("FOO", "BAR")) - .build() - ).build(); + final List customizer = new LinkedList<>(); + new Verifications() {{ + archiveService.createDockerBuildArchive(withInstanceOf(ImageConfiguration.class), withInstanceOf(MojoParameters.class), withCapture(customizer)); - service.createBuildArchive(imageWithEnv); + assertTrue(customizer.size() == 1); + }}; - final List customizer = new LinkedList<>(); - new Verifications() {{ - archiveService.createDockerBuildArchive(withInstanceOf(ImageConfiguration.class), withInstanceOf(MojoParameters.class), withCapture(customizer)); + customizer.get(0).customize(tarArchiver); - assertTrue(customizer.size() == 1); - }}; + final List file = new LinkedList<>(); + new Verifications() {{ + String path; + tarArchiver.addFile(withCapture(file), path = withCapture()); - customizer.get(0).customize(tarArchiver); + assertEquals(".s2i/environment", path); + }}; - final List file = new LinkedList<>(); - new Verifications() {{ - String path; - tarArchiver.addFile(withCapture(file), path = withCapture()); + assertEquals(1, file.size()); + List lines; + try (FileReader reader = new FileReader(file.get(0))) { + lines = IOUtils.readLines(reader); + } + assertTrue(lines.contains("FOO=BAR")); + }); + } - assertEquals(".s2i/environment", path); - }}; + @FunctionalInterface + private interface MockServerRetryable { + void run() throws Fabric8ServiceException, MojoExecutionException, IOException; + } - assertEquals(1, file.size()); - List lines; - try (FileReader reader = new FileReader(file.get(0))) { - lines = IOUtils.readLines(reader); - } - assertTrue(lines.contains("FOO=BAR")); + private void retryInMockServer(MockServerRetryable retryable) throws Exception { + Throwable rootCause = null; + int nTries = 0; + boolean bTestComplete = false; + do { + try { + nTries++; + retryable.run(); bTestComplete = true; } catch (Fabric8ServiceException exception) { - Throwable rootCause = getRootCause(exception); + rootCause = getRootCause(exception); logger.warn("A problem encountered while running test {}, retrying..", exception.getMessage()); - // Let's wait for a while, and then retry again - if (rootCause != null && rootCause instanceof IOException) { - continue; - } } } while (nTries < MAX_TIMEOUT_RETRIES && !bTestComplete); + if (!bTestComplete && rootCause != null) { + throw new Exception("Test did not complete", rootCause); + } } protected WebServerEventCollector createMockServer(BuildService.BuildServiceConfig config, boolean success, long buildDelay, boolean buildConfigExists, boolean @@ -497,9 +485,14 @@ protected WebServerEventCollector createMockServer(BuildSer OpenShiftMockServer mockServer = new OpenShiftMockServer(false); WebServerEventCollector collector = new WebServerEventCollector<>(mockServer); + final String s2iBuildNameSuffix = Optional + .ofNullable(config.getS2iBuildNameSuffix()) + .orElseGet(() -> config.getOpenshiftBuildStrategy() == OpenShiftBuildStrategy.s2i ? + "-s2i" : ""); + BuildConfig bc = new BuildConfigBuilder() .withNewMetadata() - .withName(projectName + config.getS2iBuildNameSuffix()) + .withName(projectName + s2iBuildNameSuffix) .endMetadata() .withNewSpec() .endSpec() @@ -509,7 +502,7 @@ protected WebServerEventCollector createMockServer(BuildSer if (config.getOpenshiftPullSecret() != null) { bcSecret = new BuildConfigBuilder() .withNewMetadata() - .withName(projectName + config.getS2iBuildNameSuffix() + "pullSecret") + .withName(projectName + s2iBuildNameSuffix + "pullSecret") .endMetadata() .withNewSpec() .withStrategy(new BuildStrategyBuilder().withType("Docker") @@ -550,26 +543,26 @@ protected WebServerEventCollector createMockServer(BuildSer .build(); if (!buildConfigExists) { - mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + config.getS2iBuildNameSuffix()).andReply(collector.record("build-config-check").andReturn + mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix).andReply(collector.record("build-config-check").andReturn (404, "")).once(); - mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + config.getS2iBuildNameSuffix() + "pullSecret").andReply(collector.record("build-config-check").andReturn + mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix + "pullSecret").andReply(collector.record("build-config-check").andReturn (404, "")).once(); mockServer.expect().post().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs").andReply(collector.record("new-build-config").andReturn(201, bc)).once(); if (bcSecret != null) { mockServer.expect().post().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs").andReply(collector.record("new-build-config").andReturn(201, bcSecret)).once(); } } else { - mockServer.expect().patch().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + config.getS2iBuildNameSuffix()).andReply(collector.record("patch-build-config").andReturn + mockServer.expect().patch().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix).andReply(collector.record("patch-build-config").andReturn (200, bc)).once(); if (bcSecret != null) { - mockServer.expect().patch().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + config.getS2iBuildNameSuffix() + "pullSecret").andReply(collector.record("patch-build-config").andReturn + mockServer.expect().patch().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix + "pullSecret").andReply(collector.record("patch-build-config").andReturn (200, bcSecret)).once(); } } - mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + config.getS2iBuildNameSuffix()).andReply(collector.record("build-config-check").andReturn(200, + mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix).andReply(collector.record("build-config-check").andReturn(200, bc)).always(); if (bcSecret != null) { - mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + config.getS2iBuildNameSuffix() + "pullSecret").andReply(collector.record("build-config-check").andReturn(200, + mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix + "pullSecret").andReply(collector.record("build-config-check").andReturn(200, bcSecret)).always(); } @@ -581,14 +574,14 @@ protected WebServerEventCollector createMockServer(BuildSer mockServer.expect().post().withPath("/apis/image.openshift.io/v1/namespaces/test/imagestreams").andReturn(201, imageStream).once(); - mockServer.expect().post().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + config.getS2iBuildNameSuffix() + "/instantiatebinary?commit=") + mockServer.expect().post().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix + "/instantiatebinary?commit=") .andReply( collector.record("pushed") .andReturn(201, imageStream)) .once(); mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/builds").andReply(collector.record("check-build").andReturn(200, builds)).always(); - mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/builds?labelSelector=openshift.io/build-config.name%3D" + projectName + config.getS2iBuildNameSuffix()).andReturn(200, builds) + mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/builds?labelSelector=openshift.io/build-config.name%3D" + projectName + s2iBuildNameSuffix).andReturn(200, builds) .always(); mockServer.expect().withPath("/apis/build.openshift.io/v1/namespaces/test/builds/" + projectName).andReturn(200, build).always(); diff --git a/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/BuildMojo.java b/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/BuildMojo.java index ff815bcd6e..1e7f27b68c 100644 --- a/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/BuildMojo.java +++ b/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/BuildMojo.java @@ -127,7 +127,7 @@ public class BuildMojo extends io.fabric8.maven.docker.BuildMojo { * The S2I binary builder BuildConfig name suffix appended to the image name to avoid * clashing with the underlying BuildConfig for the Jenkins pipeline */ - @Parameter(property = "fabric8.s2i.buildNameSuffix", defaultValue = "-s2i") + @Parameter(property = "fabric8.s2i.buildNameSuffix", defaultValue = "") private String s2iBuildNameSuffix; /** @@ -403,4 +403,4 @@ private ProcessorConfig extractGeneratorConfig() { throw new IllegalArgumentException("Cannot extract generator config: " + e,e); } } -} \ No newline at end of file +}