Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

Fix #825 : More flexible S2I binary build data format #1355

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ After this we will switch probably to real [Semantic Versioning 2.0.0](http://se
* Feature 1363: Added a Thorntail V2 sample for checking Jolokia/Prometheus issues
* Fix 894: Keep Service parameters stable after redeployment
* Fix 1372: Filename to type mappings should be more flexible than a string array

* Fix #825 : More flexible S2I binary build data format

###3.5.40
* Feature 1264: Added `osio` profile, with enricher to apply OpenShift.io space labels to resources
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Copyright 2016 Red Hat, Inc.
*
* Red Hat licenses this file to you under the Apache License, version
* 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
* implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package io.fabric8.maven.core.service;

import io.fabric8.maven.docker.config.ImageConfiguration;
import io.fabric8.maven.docker.service.ServiceHub;

import java.io.File;

public interface BinaryInputArchiveBuilder {
void createBinaryInput(ServiceHub hub, File targetDirectory, ImageConfiguration imageConfiguration) throws Fabric8ServiceException;
File getBinaryInputTar();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* Copyright 2016 Red Hat, Inc.
*
* Red Hat licenses this file to you under the Apache License, version
* 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
* implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package io.fabric8.maven.core.service;

import io.fabric8.maven.core.util.IoUtil;
import io.fabric8.maven.docker.assembly.ArchiverCustomizer;
import io.fabric8.maven.docker.config.ImageConfiguration;
import io.fabric8.maven.docker.service.ServiceHub;
import org.apache.maven.plugin.MojoExecutionException;
import org.codehaus.plexus.archiver.tar.TarArchiver;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.util.Map;

public class DefaultBinaryInputArchiveBuilder implements BinaryInputArchiveBuilder {

private BuildService.BuildServiceConfig config;
private File binaryInputTar = null;

public DefaultBinaryInputArchiveBuilder(BuildService.BuildServiceConfig config) {
this.config = config;
}

@Override
public void createBinaryInput(ServiceHub hub, File targetDir, ImageConfiguration imageConfiguration) throws Fabric8ServiceException {
// Adding S2I artifacts such as environment variables in S2I mode
ArchiverCustomizer customizer = getS2ICustomizer(imageConfiguration);

try {
// Create tar file with Docker archive
if (customizer != null) {
this.binaryInputTar = hub.getArchiveService().createDockerBuildArchive(imageConfiguration, config.getDockerMojoParameters(), customizer);
} else {
this.binaryInputTar = hub.getArchiveService().createDockerBuildArchive(imageConfiguration, config.getDockerMojoParameters());
}
} catch (MojoExecutionException e) {
throw new Fabric8ServiceException("Unable to create the build archive", e);
}
}

private ArchiverCustomizer getS2ICustomizer(ImageConfiguration imageConfiguration) throws Fabric8ServiceException {
try {
if (imageConfiguration.getBuildConfiguration() != null && imageConfiguration.getBuildConfiguration().getEnv() != null) {
String fileName = IoUtil.sanitizeFileName("s2i-env-" + imageConfiguration.getName());
final File environmentFile = new File(config.getBuildDirectory(), fileName);

try (PrintWriter out = new PrintWriter(new FileWriter(environmentFile))) {
for (Map.Entry<String, String> e : imageConfiguration.getBuildConfiguration().getEnv().entrySet()) {
out.println(e.getKey() + "=" + e.getValue());
}
}

return new ArchiverCustomizer() {
@Override
public TarArchiver customize(TarArchiver tarArchiver) throws IOException {
tarArchiver.addFile(environmentFile, ".s2i/environment");
return tarArchiver;
}
};
} else {
return null;
}
} catch (IOException e) {
throw new Fabric8ServiceException("Unable to add environment variables to the S2I build archive", e);
}
}

public File getBinaryInputTar() {
return binaryInputTar;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public class Fabric8ServiceHub {

private MavenProject mavenProject;

private BinaryInputArchiveBuilder binaryInputArchiveBuilder;

/**
/*
* Computed resources
Expand Down Expand Up @@ -95,7 +97,7 @@ protected BuildService build() {
throw new IllegalStateException("Openshift platform has been specified but Openshift has not been detected!");
}
// Openshift services
buildService = new OpenshiftBuildService((OpenShiftClient) client, log, dockerServiceHub, buildServiceConfig);
buildService = new OpenshiftBuildService((OpenShiftClient) client, log, dockerServiceHub, buildServiceConfig, binaryInputArchiveBuilder);
} else {
// Kubernetes services
buildService = new DockerBuildService(dockerServiceHub, buildServiceConfig);
Expand All @@ -116,6 +118,10 @@ public BuildService getBuildService() {
return (BuildService) this.services.get(BuildService.class).get();
}

public ServiceHub getDockerServiceHub() {
return dockerServiceHub;
}

public ArtifactResolverService getArtifactResolverService() {
return (ArtifactResolverService) this.services.get(ArtifactResolverService.class).get();
}
Expand Down Expand Up @@ -165,6 +171,11 @@ public Builder mavenProject(MavenProject mavenProject) {
return this;
}

public Builder binaryInputArchiveBuilder(BinaryInputArchiveBuilder binaryInputArchiveBuilder) {
hub.binaryInputArchiveBuilder = binaryInputArchiveBuilder;
return this;
}

public Fabric8ServiceHub build() {
hub.init();
return hub;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* Copyright 2016 Red Hat, Inc.
*
* Red Hat licenses this file to you under the Apache License, version
* 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
* implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package io.fabric8.maven.core.service;

import io.fabric8.maven.core.util.ResourceUtil;
import io.fabric8.maven.docker.config.ImageConfiguration;
import io.fabric8.maven.docker.service.ServiceHub;
import org.apache.maven.plugin.MojoExecutionException;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;

public class WarBinaryInputArchiveBuilder implements BinaryInputArchiveBuilder {
private BuildService.BuildServiceConfig config;
private File binaryInputTar = null;

public WarBinaryInputArchiveBuilder(BuildService.BuildServiceConfig config) {
this.config = config;
}

@Override
public void createBinaryInput(ServiceHub hub, File binaryInputDirectory, ImageConfiguration imageConfiguration) throws Fabric8ServiceException {
try {
File targetDirectory = new File(config.getBuildDirectory());
String[] jarOrWars = ResourceUtil.getFileListOfExtension(targetDirectory, ".war");

if(jarOrWars == null || jarOrWars.length == 0) {
throw new MojoExecutionException("No war/jar built yet. Please ensure that the 'package' phase has run");
}

File warFileLocation = ResourceUtil.getArchiveFile(targetDirectory, jarOrWars);
File targetWarFileLocation = new File(binaryInputDirectory, warFileLocation.getName());
targetWarFileLocation.createNewFile();
Files.copy(Paths.get(warFileLocation.getAbsolutePath()), Paths.get(targetWarFileLocation.getAbsolutePath()), StandardCopyOption.REPLACE_EXISTING);
} catch(IOException exception) {
throw new Fabric8ServiceException("Cannot examine "+ binaryInputDirectory.getName() + " for the manifest");
} catch (MojoExecutionException mojoException) {
throw new Fabric8ServiceException(mojoException);
}
}

@Override
public File getBinaryInputTar() { return binaryInputTar; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
package io.fabric8.maven.core.service.openshift;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.UnsupportedEncodingException;
import java.util.HashMap;
import java.util.List;
Expand All @@ -39,16 +37,16 @@
import io.fabric8.kubernetes.client.Watcher;
import io.fabric8.kubernetes.client.dsl.LogWatch;
import io.fabric8.maven.core.config.OpenShiftBuildStrategy;
import io.fabric8.maven.core.service.BinaryInputArchiveBuilder;
import io.fabric8.maven.core.service.BuildService;
import io.fabric8.maven.core.service.Fabric8ServiceException;
import io.fabric8.maven.core.util.IoUtil;
import io.fabric8.maven.core.util.FileUtil;
import io.fabric8.maven.core.util.ResourceFileType;
import io.fabric8.maven.core.util.kubernetes.KubernetesClientUtil;
import io.fabric8.maven.core.util.kubernetes.KubernetesHelper;
import io.fabric8.maven.core.util.kubernetes.KubernetesResourceUtil;
import io.fabric8.maven.core.util.kubernetes.OpenshiftHelper;
import io.fabric8.maven.docker.access.AuthConfig;
import io.fabric8.maven.docker.assembly.ArchiverCustomizer;
import io.fabric8.maven.docker.assembly.DockerAssemblyManager;
import io.fabric8.maven.docker.config.AssemblyConfiguration;
import io.fabric8.maven.docker.config.BuildImageConfiguration;
Expand Down Expand Up @@ -87,27 +85,37 @@ public class OpenshiftBuildService implements BuildService {
private BuildServiceConfig config;
private RegistryService.RegistryConfig registryConfig;
private AuthConfigFactory authConfigFactory;
private BinaryInputArchiveBuilder binaryInputArchiveBuilder;


public OpenshiftBuildService(OpenShiftClient client, Logger log, ServiceHub dockerServiceHub, BuildServiceConfig config) {
public OpenshiftBuildService(OpenShiftClient client, Logger log, ServiceHub dockerServiceHub, BuildServiceConfig config, BinaryInputArchiveBuilder binaryInputArchiveBuilder) {
Objects.requireNonNull(client, "client");
Objects.requireNonNull(log, "log");
Objects.requireNonNull(dockerServiceHub, "dockerServiceHub");
Objects.requireNonNull(config, "config");
Objects.requireNonNull(binaryInputArchiveBuilder, "binaryInputArchiveBuilder");

this.client = client;
this.log = log;
this.dockerServiceHub = dockerServiceHub;
this.config = config;
this.binaryInputArchiveBuilder = binaryInputArchiveBuilder;
}

@Override
public void build(ImageConfiguration imageConfig) throws Fabric8ServiceException {
String buildName = null;
try {
ImageName imageName = new ImageName(imageConfig.getName());
// Create an empty directory
File targetDir = createEmptyTargetDir();

// Fill the directory with the data in the right directory structure
binaryInputArchiveBuilder.createBinaryInput(dockerServiceHub, targetDir, imageConfig);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's about adding a File return value to the BinaryInputArchiveBuilder which then holds the the TAR archive ?

Advantages:

  • Get rid of createTar() method
  • Simplify interface to one method
  • No need for the targetDir parameter. This just would be an implementation details.

If you don't mind I would change it like this and push it to your branch (just to speed up things and to clarify my point).

@rohanKanojia would this be ok for you ?


File dockerTar = createBuildArchive(imageConfig);
// Create the tar archive by tarring the directory
File dockerTar = binaryInputArchiveBuilder.getBinaryInputTar() != null ? binaryInputArchiveBuilder.getBinaryInputTar() : createTar(targetDir);

ImageName imageName = new ImageName(imageConfig.getName());

KubernetesListBuilder builder = new KubernetesListBuilder();

Expand Down Expand Up @@ -144,49 +152,26 @@ public void build(ImageConfiguration imageConfig) throws Fabric8ServiceException
}
}

protected File createBuildArchive(ImageConfiguration imageConfig) throws Fabric8ServiceException {
// Adding S2I artifacts such as environment variables in S2I mode
ArchiverCustomizer customizer = getS2ICustomizer(imageConfig);

try {
// Create tar file with Docker archive
File dockerTar;
if (customizer != null) {
dockerTar = dockerServiceHub.getArchiveService().createDockerBuildArchive(imageConfig, config.getDockerMojoParameters(), customizer);
} else {
dockerTar = dockerServiceHub.getArchiveService().createDockerBuildArchive(imageConfig, config.getDockerMojoParameters());
}
return dockerTar;
} catch (MojoExecutionException e) {
throw new Fabric8ServiceException("Unable to create the build archive", e);
private File createEmptyTargetDir() throws MojoExecutionException {
File targetDir = new File(config.getBuildDirectory(), "s2i");
if (!targetDir.exists() && !targetDir.mkdirs()) {
throw new MojoExecutionException("Can't create build working directory " + targetDir);
}
return targetDir;
}

private ArchiverCustomizer getS2ICustomizer(ImageConfiguration imageConfiguration) throws Fabric8ServiceException {
try {
if (imageConfiguration.getBuildConfiguration() != null && imageConfiguration.getBuildConfiguration().getEnv() != null) {
String fileName = IoUtil.sanitizeFileName("s2i-env-" + imageConfiguration.getName());
final File environmentFile = new File(config.getBuildDirectory(), fileName);
private File createTar(File targetDir) throws IOException, MojoExecutionException {
TarArchiver tarArchiver = new TarArchiver();
tarArchiver.addDirectory(targetDir);

try (PrintWriter out = new PrintWriter(new FileWriter(environmentFile))) {
for (Map.Entry<String, String> e : imageConfiguration.getBuildConfiguration().getEnv().entrySet()) {
out.println(e.getKey() + "=" + e.getValue());
}
}
// Delete previously existing tar files, if any
FileUtil.deleteFilesInDirectory(targetDir, (file, s) -> { return s.endsWith(".tar"); });

return new ArchiverCustomizer() {
@Override
public TarArchiver customize(TarArchiver tarArchiver) throws IOException {
tarArchiver.addFile(environmentFile, ".s2i/environment");
return tarArchiver;
}
};
} else {
return null;
}
} catch (IOException e) {
throw new Fabric8ServiceException("Unable to add environment variables to the S2I build archive", e);
}
File tarFile = File.createTempFile(config.getArtifactId().toString(), ".tar");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I understand this: tarFile is a temporary tar filename but the tar archive itself is created with the destFile as name. The return value is then a the temporary name (which doesn't point to any tar archive if I understood the code right). Could you please check this again ?

File destFile = new File(targetDir, config.getArtifactId() + ".tar");
tarArchiver.setDestFile(destFile);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not sure that this works if you put the destination file into the directory that you tar up (as it would be tar up itself, too)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naiively I would expect a

tarArchiver.setDestFile(tarFile);
tarArchiver.createArchive();
return tarFile;

and removing destFile ....

tarArchiver.createArchive();
return tarFile;
}

private File getImageStreamFile(BuildServiceConfig config) {
Expand Down
15 changes: 15 additions & 0 deletions core/src/main/java/io/fabric8/maven/core/util/FileUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package io.fabric8.maven.core.util;

import java.io.File;
import java.io.FilenameFilter;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Path;
Expand Down Expand Up @@ -70,4 +71,18 @@ public static String getAbsolutePath(URL url) {
throw new RuntimeException(e);
}
}

public static void deleteFilesInDirectory(File parentDir, FilenameFilter filterNameFilter) {
String[] files = parentDir.list(filterNameFilter);

if(files == null || files.length == 0) {
return;
}

File fileToDelete;
for(String file : files) {
fileToDelete = new File(parentDir, file);
fileToDelete.delete();
}
}
}
Loading