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

Log Docker Build Context Directory while building image in Simple Dockerfile mode #1192

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,10 @@ protected List<ImageConfiguration> resolveImages(ImageConfigResolver imageConfig
kubernetesExtension.getApiVersionOrNull(),
getBuildTimestamp(null, null, kubernetesExtension.javaProject.getBuildDirectory().getAbsolutePath(),
DOCKER_BUILD_TIMESTAMP),
kubernetesExtension.javaProject, kubernetesExtension.images, imageConfigResolver, kitLogger,
kubernetesExtension.images, imageConfigResolver, kitLogger,
kubernetesExtension.getFilter().getOrNull(),
this::customizeConfig);
this::customizeConfig,
jKubeServiceHub.getConfiguration());
}

protected File getManifest(KubernetesClient kc) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package org.eclipse.jkube.kit.build.service.docker.helper;

import org.eclipse.jkube.kit.build.service.docker.config.handler.ImageConfigResolver;
import org.eclipse.jkube.kit.common.JKubeConfiguration;
import org.eclipse.jkube.kit.config.image.ImageConfiguration;
import org.eclipse.jkube.kit.config.image.build.BuildConfiguration;
import org.eclipse.jkube.kit.build.service.docker.config.handler.property.PropertyConfigHandler;
Expand Down Expand Up @@ -181,21 +182,21 @@ private static void verifyImageNames(List<ImageConfiguration> ret) {
}
}

public static List<ImageConfiguration> initImageConfiguration(String apiVersion, Date buildTimeStamp, JavaProject javaProject, List<ImageConfiguration> images, ImageConfigResolver imageConfigResolver, KitLogger log, String filter, ConfigHelper.Customizer customizer) {
public static List<ImageConfiguration> initImageConfiguration(String apiVersion, Date buildTimeStamp, List<ImageConfiguration> images, ImageConfigResolver imageConfigResolver, KitLogger log, String filter, ConfigHelper.Customizer customizer, JKubeConfiguration jKubeConfiguration) {
List<ImageConfiguration> resolvedImages;
ImageNameFormatter imageNameFormatter = new ImageNameFormatter(javaProject, buildTimeStamp);
ImageNameFormatter imageNameFormatter = new ImageNameFormatter(jKubeConfiguration.getProject(), buildTimeStamp);
// Resolve images
resolvedImages = ConfigHelper.resolveImages(
log,
images, // Unresolved images
(ImageConfiguration image) -> imageConfigResolver.resolve(image, javaProject),
(ImageConfiguration image) -> imageConfigResolver.resolve(image, jKubeConfiguration.getProject()),
filter, // A filter which image to process
customizer); // customizer (can be overwritten by a subclass)

// Check for simple Dockerfile mode
if (isSimpleDockerFileMode(javaProject.getBaseDirectory())) {
File topDockerfile = getTopLevelDockerfile(javaProject.getBaseDirectory());
String defaultImageName = imageNameFormatter.format(getValueFromProperties(javaProject.getProperties(),
if (isSimpleDockerFileMode(jKubeConfiguration.getBasedir())) {
File topDockerfile = getTopLevelDockerfile(jKubeConfiguration.getBasedir());
String defaultImageName = imageNameFormatter.format(getValueFromProperties(jKubeConfiguration.getProject().getProperties(),
"jkube.image.name", "jkube.generator.name"));
if (resolvedImages.isEmpty()) {
resolvedImages.add(createSimpleDockerfileConfig(topDockerfile, defaultImageName));
Expand All @@ -211,6 +212,7 @@ public static List<ImageConfiguration> initImageConfiguration(String apiVersion,
BuildConfiguration buildConfiguration = image.getBuildConfiguration();
if (buildConfiguration != null && buildConfiguration.isDockerFileMode()) {
log.info("Using Dockerfile: %s", buildConfiguration.getDockerFile().getAbsolutePath());
log.info("Using Docker Context Directory: %s", buildConfiguration.getAbsoluteContextDirPath(jKubeConfiguration.getSourceDirectory(), jKubeConfiguration.getBasedir().getAbsolutePath()));
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these directories in JavaProject?

I remember in the early days these fields were scattered all around and we managed to reduce them to just the JKubeConfiguration and the JavaProject instances.
What's the issue in this case?

Copy link
Member

Choose a reason for hiding this comment

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

If the homonymous fields have the same purpose, one set should be deprecated. If not, the undocumented, should be. This way we would avoid situations like the current were there's confusion about what should be used

Copy link
Member Author

Choose a reason for hiding this comment

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

I can see jKubeConfiguration.getBaseDir() returns JavaProject's base directory. But I'm not able to find any sourceDirectory equivalent in JavaProject

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
log.info("Using Docker Context Directory: %s", buildConfiguration.getAbsoluteContextDirPath(jKubeConfiguration.getSourceDirectory(), jKubeConfiguration.getBasedir().getAbsolutePath()));
log.info("Using Docker context directory: %s", buildConfiguration.getAbsoluteContextDirPath(jKubeConfiguration.getSourceDirectory(), jKubeConfiguration.getBasedir().getAbsolutePath()));

Copy link
Member

Choose a reason for hiding this comment

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

I can see jKubeConfiguration.getBaseDir() returns JavaProject's base directory. But I'm not able to find any sourceDirectory equivalent in JavaProject

OK, so this one is not duplicate, it just retrieves javaProject.baseDirectory

However, outputDirectory is still defined in both places. We'll need to investigate and deprecate, rename or proper document them.

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
*/
package org.eclipse.jkube.kit.build.service.docker.helper;

import mockit.Expectations;
import mockit.Mocked;
import org.eclipse.jkube.kit.build.service.docker.config.handler.ImageConfigResolver;
import org.eclipse.jkube.kit.common.JKubeConfiguration;
import org.eclipse.jkube.kit.common.JavaProject;
import org.eclipse.jkube.kit.common.KitLogger;
import org.eclipse.jkube.kit.config.image.ImageConfiguration;
import org.eclipse.jkube.kit.config.image.build.BuildConfiguration;
import org.junit.Before;
import org.junit.Test;

import java.io.File;
Expand All @@ -30,16 +30,24 @@
import java.util.Properties;

import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class ConfigHelperTest {
@Mocked
Copy link
Member

Choose a reason for hiding this comment

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

Why not using Mockito annotations ?

Copy link
Member

Choose a reason for hiding this comment

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

You'd need to run the test with MockitoJUnitRunner.class

These annotations were the ones provided by JMockit.

private ImageConfigResolver imageConfigResolver;

@Mocked
private KitLogger logger;

@Mocked
private JavaProject javaProject;
private JKubeConfiguration jKubeConfiguration;

@Before
public void setUp() {
imageConfigResolver = mock(ImageConfigResolver.class, RETURNS_DEEP_STUBS);
logger = new KitLogger.SilentLogger();
javaProject = mock(JavaProject.class, RETURNS_DEEP_STUBS);
jKubeConfiguration = mock(JKubeConfiguration.class, RETURNS_DEEP_STUBS);
when(jKubeConfiguration.getProject()).thenReturn(javaProject);
}

@Test
public void initImageConfiguration_withSimpleImageConfiguration_shouldReturnImageConfiguration() {
Expand All @@ -52,16 +60,11 @@ public void initImageConfiguration_withSimpleImageConfiguration_shouldReturnImag
.build();
List<ImageConfiguration> images = new ArrayList<>();
images.add(dummyImageConfiguration);
new Expectations() {{
imageConfigResolver.resolve(dummyImageConfiguration, javaProject);
result = dummyImageConfiguration;

javaProject.getBaseDirectory();
result = new File("dummydir");
}};
when(jKubeConfiguration.getBasedir()).thenReturn(new File("dummydir"));
when(imageConfigResolver.resolve(dummyImageConfiguration, javaProject)).thenReturn(images);

// When
List<ImageConfiguration> resolvedImages = ConfigHelper.initImageConfiguration("1.12", new Date(), javaProject, images, imageConfigResolver, logger, null, configs -> configs);
List<ImageConfiguration> resolvedImages = ConfigHelper.initImageConfiguration("1.12", new Date(), images, imageConfigResolver, logger, null, configs -> configs, jKubeConfiguration);

// Then
assertThat(resolvedImages)
Expand All @@ -74,25 +77,14 @@ public void initImageConfiguration_withSimpleImageConfiguration_shouldReturnImag
public void initImageConfiguration_withSimpleDockerFileInProjectBaseDir_shouldCreateImageConfiguration() {
List<ImageConfiguration> images = new ArrayList<>();
File dockerFile = new File(getClass().getResource("/dummy-javaproject/Dockerfile").getFile());
new Expectations() {{
javaProject.getBaseDirectory();
result = dockerFile.getParentFile();

javaProject.getProperties();
result = new Properties();

javaProject.getGroupId();
result = "org.eclipse.jkube";

javaProject.getArtifactId();
result = "test-java-project";

javaProject.getVersion();
result = "0.0.1-SNAPSHOT";
}};
when(jKubeConfiguration.getBasedir()).thenReturn(dockerFile.getParentFile());
when(javaProject.getProperties()).thenReturn(new Properties());
when(javaProject.getGroupId()).thenReturn("org.eclipse.jkube");
when(javaProject.getArtifactId()).thenReturn("test-java-project");
when(javaProject.getVersion()).thenReturn("0.0.1-SNAPSHOT");

// When
List<ImageConfiguration> resolvedImages = ConfigHelper.initImageConfiguration("1.12", new Date(), javaProject, images, imageConfigResolver, logger, null, configs -> configs);
List<ImageConfiguration> resolvedImages = ConfigHelper.initImageConfiguration("1.12", new Date(), images, imageConfigResolver, logger, null, configs -> configs, jKubeConfiguration);

// Then
assertThat(resolvedImages)
Expand All @@ -112,19 +104,12 @@ public void initImageConfiguration_withSimpleDockerFileModeEnabledAndImageConfig
List<ImageConfiguration> images = new ArrayList<>();
images.add(dummyImageConfiguration);
File dockerFile = new File(getClass().getResource("/dummy-javaproject/Dockerfile").getFile());
new Expectations() {{
imageConfigResolver.resolve(dummyImageConfiguration, javaProject);
result = dummyImageConfiguration;

javaProject.getBaseDirectory();
result = dockerFile.getParentFile();

javaProject.getProperties();
result = new Properties();
}};
when(imageConfigResolver.resolve(dummyImageConfiguration, javaProject)).thenReturn(images);
when(jKubeConfiguration.getBasedir()).thenReturn(dockerFile.getParentFile());
when(javaProject.getProperties()).thenReturn(new Properties());

// When
List<ImageConfiguration> resolvedImages = ConfigHelper.initImageConfiguration("1.12", new Date(), javaProject, images, imageConfigResolver, logger, null, configs -> configs);
List<ImageConfiguration> resolvedImages = ConfigHelper.initImageConfiguration("1.12", new Date(), images, imageConfigResolver, logger, null, configs -> configs, jKubeConfiguration);

// Then
assertThat(resolvedImages)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ protected void doExecute() throws MojoExecutionException {
.buildServiceConfig(buildServiceConfigBuilder().build())
.offline(offline)
.build();
resolvedImages = ConfigHelper.initImageConfiguration(apiVersion, getBuildTimestamp(getPluginContext(), CONTEXT_KEY_BUILD_TIMESTAMP, project.getBuild().getDirectory(), DOCKER_BUILD_TIMESTAMP), javaProject, images, imageConfigResolver, log, filter, this);
resolvedImages = ConfigHelper.initImageConfiguration(apiVersion, getBuildTimestamp(getPluginContext(), CONTEXT_KEY_BUILD_TIMESTAMP, project.getBuild().getDirectory(), DOCKER_BUILD_TIMESTAMP), images, imageConfigResolver, log, filter, this, jkubeServiceHub.getConfiguration());
executeInternal();
} catch (IOException | DependencyResolutionRequiredException exp) {
logException(exp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import org.eclipse.jkube.kit.resource.service.DefaultResourceService;

import io.fabric8.kubernetes.api.model.KubernetesList;
import org.apache.maven.artifact.DependencyResolutionRequiredException;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.MojoFailureException;
import org.apache.maven.plugins.annotations.Component;
Expand Down Expand Up @@ -219,7 +218,7 @@ public void executeInternal() throws MojoExecutionException, MojoFailureExceptio
// Attach it to the Maven reactor so that it will also get deployed
projectHelper.attachArtifact(project, this.resourceFileType.getArtifactType(), resourceClassifier.getValue(), artifact);
}
} catch (IOException | DependencyResolutionRequiredException e) {
} catch (IOException e) {
throw new MojoExecutionException("Failed to generate kubernetes descriptor", e);
}
}
Expand Down Expand Up @@ -280,7 +279,7 @@ private void lateInit() {
RuntimeMode runtimeMode = getRuntimeMode();
jkubeServiceHub.setPlatformMode(runtimeMode);
if (runtimeMode.equals(RuntimeMode.OPENSHIFT)) {
Properties properties = project.getProperties();
Properties properties = javaProject.getProperties();
if (!properties.contains(DOCKER_IMAGE_USER)) {
String namespaceToBeUsed = this.namespace != null && !this.namespace.isEmpty() ?
this.namespace: clusterAccess.getNamespace();
Expand All @@ -294,10 +293,10 @@ private void lateInit() {
}

private KubernetesList generateResources()
throws IOException, DependencyResolutionRequiredException {
throws IOException {

JKubeEnricherContext.JKubeEnricherContextBuilder ctxBuilder = JKubeEnricherContext.builder()
.project(MavenUtil.convertMavenProjectToJKubeProject(project, session))
.project(javaProject)
.processorConfig(extractEnricherConfig())
.settings(MavenUtil.getRegistryServerFromMavenSettings(settings))
.resources(resources)
Expand All @@ -321,21 +320,19 @@ private ProcessorConfig extractGeneratorConfig() throws IOException {
// ==================================================================================

private List<ImageConfiguration> getResolvedImages(List<ImageConfiguration> images, final KitLogger log)
throws DependencyResolutionRequiredException, IOException {
final JavaProject jkubeProject = MavenUtil.convertMavenProjectToJKubeProject(project, session);
throws IOException {
return ConfigHelper.initImageConfiguration(
null /* no minimal api version */,
getBuildTimestamp(getPluginContext(), CONTEXT_KEY_BUILD_TIMESTAMP, project.getBuild().getDirectory(),
DOCKER_BUILD_TIMESTAMP),
jkubeProject,
images, imageConfigResolver,
log,
null, // no filter on image name yet (TODO: Maybe add this, too ?)
configs -> {
try {
GeneratorContext ctx = GeneratorContext.builder()
.config(extractGeneratorConfig())
.project(jkubeProject)
.project(javaProject)
.runtimeMode(getRuntimeMode())
.logger(log)
.strategy(JKubeBuildStrategy.docker)
Expand All @@ -345,7 +342,8 @@ private List<ImageConfiguration> getResolvedImages(List<ImageConfiguration> imag
} catch (Exception e) {
throw new IllegalArgumentException("Cannot extract generator: " + e, e);
}
});
},
jkubeServiceHub.getConfiguration());
}

private File[] mavenFilterFiles(File[] resourceFiles, File outDir) throws IOException {
Expand Down
4 changes: 4 additions & 0 deletions openshift-maven-plugin/plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@
<groupId>org.jmockit</groupId>
<artifactId>jmockit</artifactId>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
Expand Down