Skip to content

Commit

Permalink
fix: Image names with multiple components can be used with OpenShift
Browse files Browse the repository at this point in the history 
(e.g. docker.io/registry/component1/component2/component3:tag

- https://github.com/distribution/distribution/blob/2461543d988979529609e8cb6fca9ca190dc48da/reference/reference.go#L7
- https://github.com/moby/moby/blob/master/image/spec/v1.1.md

Signed-off-by: Marc Nuri <marc@marcnuri.com>
  • Loading branch information
@manusa
manusa committed Jun 4, 2021
1 parent e1efb76 commit 52b15fb
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 44 deletions.
12 changes: 7 additions & 5 deletions ...vice/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
*/
public class ImageStreamService {


public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss'Z'";
private static final int IMAGE_STREAM_TAG_RETRIES = 15;
private static final long IMAGE_STREAM_TAG_RETRY_TIMEOUT_IN_MILLIS = 1000;
Expand All @@ -74,7 +73,7 @@ public void appendImageStreamResource(ImageName imageName, File target) throws I
try {
ImageStream is = new ImageStreamBuilder()
.withNewMetadata()
.withName(imageName.getSimpleName())
.withName(resolveImageStreamName(imageName))
.endMetadata()

.withNewSpec()
Expand All @@ -87,7 +86,7 @@ public void appendImageStreamResource(ImageName imageName, File target) throws I
.build();
createOrUpdateImageStreamTag(client, imageName, is);
appendImageStreamToFile(is, target);
log.info("ImageStream %s written to %s", imageName.getSimpleName(), target);
log.info("ImageStream %s written to %s", resolveImageStreamName(imageName), target);
} catch (KubernetesClientException e) {
KubernetesHelper.handleKubernetesClientException(e, this.log);
} catch (IOException e) {
Expand Down Expand Up @@ -122,8 +121,8 @@ private Map<String, ImageStream> readAlreadyExtractedImageStreams(File target) t

private void createOrUpdateImageStreamTag(OpenShiftClient client, ImageName image, ImageStream is) {
String namespace = client.getNamespace();
String tagSha = findTagSha(client, image.getSimpleName(), client.getNamespace());
String name = image.getSimpleName() + "@" + tagSha;
String tagSha = findTagSha(client, resolveImageStreamName(image), client.getNamespace());
String name = resolveImageStreamName(image) + "@" + tagSha;

TagReference tag = extractTag(is);
ObjectReference from = extractFrom(tag);
Expand Down Expand Up @@ -255,4 +254,7 @@ private Date extractDate(TagEvent tag) {
}

}
protected static String resolveImageStreamName(ImageName name) {
return name.getSimpleName().replace("/", "-");
}
}
20 changes: 9 additions & 11 deletions ...e/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;

import static org.eclipse.jkube.kit.config.service.openshift.ImageStreamService.resolveImageStreamName;

/**
* @author nicola
*/
Expand Down Expand Up @@ -135,7 +137,7 @@ public void build(ImageConfiguration imageConfig) throws JKubeServiceException {
buildName = updateOrCreateBuildConfig(config, client, builder, imageConfig, null);
}

checkOrCreateImageStream(config, client, builder, getImageStreamName(imageName));
checkOrCreateImageStream(config, client, builder, resolveImageStreamName(imageName));
applyResourceObjects(config, client, builder);

// Start the actual build
Expand Down Expand Up @@ -333,8 +335,8 @@ private BuildStrategy createBuildStrategy(ImageConfiguration imageConfig, JKubeB
} else {
fromName = getMapValueWithDefault(fromExt, JKubeBuildStrategy.SourceStrategy.name, buildConfig.getFrom());
}
final String fromKind = getMapValueWithDefault(fromExt, JKubeBuildStrategy.SourceStrategy.kind, "DockerImage");
final String fromNamespace = getMapValueWithDefault(fromExt, JKubeBuildStrategy.SourceStrategy.namespace, "ImageStreamTag".equals(fromKind) ? "openshift" : null);
final String fromKind = getMapValueWithDefault(fromExt, JKubeBuildStrategy.SourceStrategy.kind, DOCKER_IMAGE);
final String fromNamespace = getMapValueWithDefault(fromExt, JKubeBuildStrategy.SourceStrategy.namespace, IMAGE_STREAM_TAG.equals(fromKind) ? "openshift" : null);
if (osBuildStrategy == JKubeBuildStrategy.docker) {
BuildStrategy buildStrategy = new BuildStrategyBuilder()
.withType("Docker")
Expand Down Expand Up @@ -380,7 +382,7 @@ private BuildStrategy createBuildStrategy(ImageConfiguration imageConfig, JKubeB

private BuildOutput createBuildOutput(BuildServiceConfig config, ImageName imageName) {
final String buildOutputKind = Optional.ofNullable(config.getBuildOutputKind()).orElse(DEFAULT_BUILD_OUTPUT_KIND);
final String outputImageStreamTag = getImageStreamName(imageName) + ":" + (imageName.getTag() != null ? imageName.getTag() : "latest");
final String outputImageStreamTag = resolveImageStreamName(imageName) + ":" + (imageName.getTag() != null ? imageName.getTag() : "latest");
final BuildOutputBuilder buildOutputBuilder = new BuildOutputBuilder();
buildOutputBuilder.withNewTo().withKind(buildOutputKind).withName(outputImageStreamTag).endTo();
if (DOCKER_IMAGE.equals(buildOutputKind)) {
Expand Down Expand Up @@ -688,9 +690,9 @@ private void logBuildFailedDetails(OpenShiftClient client, String buildName) {
String kind = ref.getKind();
String name = ref.getName();

if ("DockerImage".equals(kind)) {
if (DOCKER_IMAGE.equals(kind)) {
log.error("Please, ensure that the Docker image '%s' exists and is accessible by OpenShift", name);
} else if ("ImageStreamTag".equals(kind)) {
} else if (IMAGE_STREAM_TAG.equals(kind)) {
String namespace = ref.getNamespace();
String namespaceInfo = "current";
String namespaceParams = "";
Expand Down Expand Up @@ -732,7 +734,7 @@ private void addImageStreamToFile(File imageStreamFile, ImageName imageName, Ope
// == Utility methods ==========================

private String getS2IBuildName(BuildServiceConfig config, ImageName imageName) {
final StringBuilder s2IBuildName = new StringBuilder(imageName.getSimpleName());
final StringBuilder s2IBuildName = new StringBuilder(resolveImageStreamName(imageName));
if (!StringUtils.isEmpty(config.getS2iBuildNameSuffix())) {
s2IBuildName.append(config.getS2iBuildNameSuffix());
} else if (config.getJKubeBuildStrategy() == JKubeBuildStrategy.s2i) {
Expand All @@ -741,10 +743,6 @@ private String getS2IBuildName(BuildServiceConfig config, ImageName imageName) {
return s2IBuildName.toString();
}

private String getImageStreamName(ImageName name) {
return name.getSimpleName();
}

private String getMapValueWithDefault(Map<String, String> map, JKubeBuildStrategy.SourceStrategy strategy, String defaultValue) {
return getMapValueWithDefault(map, strategy.key(), defaultValue);
}
Expand Down
69 changes: 51 additions & 18 deletions ...c/test/java/org/eclipse/jkube/kit/config/service/openshift/OpenshiftBuildServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,34 @@ public void testDockerBuild() throws Exception {
});
}

@Test
public void testDockerBuildWithMultiComponentImageName() throws Exception {
retryInMockServer(() -> {
BuildServiceConfig dockerConfig = BuildServiceConfig.builder()
.buildDirectory(baseDir)
.buildRecreateMode(BuildRecreateMode.none)
.s2iBuildNameSuffix("-docker")
.jKubeBuildStrategy(JKubeBuildStrategy.docker).build();
image.setName("docker.io/registry/component1/component2/name:tag");
// @formatter:on
new Expectations() {{
jKubeServiceHub.getBuildServiceConfig(); result = dockerConfig;
}};
// @formatter:off
WebServerEventCollector collector = createMockServer("component1-component2-name",
dockerConfig, true, 50, false, false);

DefaultOpenShiftClient client = (DefaultOpenShiftClient) mockServer.getOpenshiftClient();
OpenshiftBuildService service = new OpenshiftBuildService(client, logger, jKubeServiceHub);
service.build(image);

assertTrue(mockServer.getMockServer().getRequestCount() > 8);
collector.assertEventsRecordedInOrder("build-config-check", "new-build-config", "pushed");
assertEquals("{\"apiVersion\":\"build.openshift.io/v1\",\"kind\":\"BuildConfig\",\"metadata\":{\"name\":\"component1-component2-name-docker\"},\"spec\":{\"output\":{\"to\":{\"kind\":\"ImageStreamTag\",\"name\":\"component1-component2-name:tag\"}},\"source\":{\"type\":\"Binary\"},\"strategy\":{\"dockerStrategy\":{\"from\":{\"kind\":\"DockerImage\",\"name\":\"myapp\"},\"noCache\":false},\"type\":\"Docker\"}}}", collector.getBodies().get(1));
collector.assertEventsNotRecorded("patch-build-config");
});
}

@Test
public void testDockerBuildNoS2iSuffix() throws Exception {
retryInMockServer(() -> {
Expand Down Expand Up @@ -610,20 +638,25 @@ private void retryInMockServer(MockServerRetryable retryable) throws Exception {
}

protected WebServerEventCollector createMockServer(
BuildServiceConfig config, boolean success, long buildDelay, boolean buildConfigExists, boolean imageStreamExists) {
return createMockServer(projectName, config, success, buildDelay, buildConfigExists, imageStreamExists);
}

protected WebServerEventCollector createMockServer(String resourceName,
BuildServiceConfig config, boolean success, long buildDelay, boolean buildConfigExists, boolean imageStreamExists) {
WebServerEventCollector collector = new WebServerEventCollector();


final String s2iBuildNameSuffix = Optional
.ofNullable(config.getS2iBuildNameSuffix())
.orElseGet(() -> config.getJKubeBuildStrategy() == JKubeBuildStrategy.s2i ?
"-s2i" : "");

final String buildOutputKind = Optional.ofNullable(config.getBuildOutputKind()).orElse("ImageStreamTag");
final String buildOutputKind = Optional.ofNullable(config.getBuildOutputKind()).orElse("ImageStreamTag");

BuildConfig bc = new BuildConfigBuilder()
.withNewMetadata()
.withName(projectName + s2iBuildNameSuffix)
.withName(resourceName + s2iBuildNameSuffix)
.endMetadata()
.withNewSpec()
.withNewOutput()
Expand All @@ -638,7 +671,7 @@ protected WebServerEventCollector createMockServer(
if (config.getOpenshiftPullSecret() != null) {
bcSecret = new BuildConfigBuilder()
.withNewMetadata()
.withName(projectName + s2iBuildNameSuffix + "pullSecret")
.withName(resourceName + s2iBuildNameSuffix + "pullSecret")
.endMetadata()
.withNewSpec()
.withStrategy(new BuildStrategyBuilder().withType("Docker")
Expand All @@ -655,7 +688,7 @@ protected WebServerEventCollector createMockServer(

ImageStream imageStream = new ImageStreamBuilder()
.withNewMetadata()
.withName(projectName)
.withName(resourceName)
.endMetadata()
.withStatus(new ImageStreamStatusBuilder()
.addNewTagLike(new NamedTagEventListBuilder()
Expand All @@ -670,7 +703,7 @@ protected WebServerEventCollector createMockServer(
KubernetesList builds = new KubernetesListBuilder().withItems(
new BuildBuilder()
.withNewMetadata()
.withName(projectName)
.withName(resourceName)
.endMetadata()
.build())
.withNewMetadata().withResourceVersion("1").endMetadata()
Expand All @@ -683,49 +716,49 @@ protected WebServerEventCollector createMockServer(
.build();

if (!buildConfigExists) {
mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix).andReply(collector.record("build-config-check").andReturn
mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + resourceName + s2iBuildNameSuffix).andReply(collector.record("build-config-check").andReturn
(404, "")).once();
mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + projectName + s2iBuildNameSuffix + "pullSecret").andReply(collector.record("build-config-check").andReturn
mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + resourceName + 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 + s2iBuildNameSuffix).andReply(collector.record("patch-build-config").andReturn
mockServer.expect().patch().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + resourceName + 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 + s2iBuildNameSuffix + "pullSecret").andReply(collector.record("patch-build-config").andReturn
mockServer.expect().patch().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + resourceName + 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 + s2iBuildNameSuffix).andReply(collector.record("build-config-check").andReturn(200,
mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + resourceName + 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 + s2iBuildNameSuffix + "pullSecret").andReply(collector.record("build-config-check").andReturn(200,
mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + resourceName + s2iBuildNameSuffix + "pullSecret").andReply(collector.record("build-config-check").andReturn(200,
bcSecret)).always();
}


if (!imageStreamExists) {
mockServer.expect().get().withPath("/apis/image.openshift.io/v1/namespaces/test/imagestreams/" + projectName).andReturn(404, "").once();
mockServer.expect().get().withPath("/apis/image.openshift.io/v1/namespaces/test/imagestreams/" + resourceName).andReturn(404, "").once();
}
mockServer.expect().get().withPath("/apis/image.openshift.io/v1/namespaces/test/imagestreams/" + projectName).andReturn(200, imageStream).always();
mockServer.expect().get().withPath("/apis/image.openshift.io/v1/namespaces/test/imagestreams/" + resourceName).andReturn(200, imageStream).always();

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 + s2iBuildNameSuffix + "/instantiatebinary?name="+ projectName + s2iBuildNameSuffix +"&namespace=test")
mockServer.expect().post().withPath("/apis/build.openshift.io/v1/namespaces/test/buildconfigs/" + resourceName + s2iBuildNameSuffix + "/instantiatebinary?name="+ resourceName + s2iBuildNameSuffix +"&namespace=test")
.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 + s2iBuildNameSuffix).andReturn(200, builds)
mockServer.expect().get().withPath("/apis/build.openshift.io/v1/namespaces/test/builds?labelSelector=openshift.io/build-config.name%3D" + resourceName + s2iBuildNameSuffix).andReturn(200, builds)
.always();

mockServer.expect().withPath("/apis/build.openshift.io/v1/namespaces/test/builds/" + projectName).andReturn(200, build).always();
mockServer.expect().withPath("/apis/build.openshift.io/v1/namespaces/test/builds?fieldSelector=metadata.name%3D" + projectName + "&watch=true")
mockServer.expect().withPath("/apis/build.openshift.io/v1/namespaces/test/builds/" + resourceName).andReturn(200, build).always();
mockServer.expect().withPath("/apis/build.openshift.io/v1/namespaces/test/builds?fieldSelector=metadata.name%3D" + resourceName + "&watch=true")
.andUpgradeToWebSocket().open()
.waitFor(buildDelay)
.andEmit(new WatchEvent(build, "MODIFIED"))
Expand Down

0 comments on commit 52b15fb

Please sign in to comment.