Skip to content

Commit

Permalink
review: forgotten build warning limited scope
Browse files Browse the repository at this point in the history 
Rolled-back checks in many of the generators where this
logic wasn't applied correctly and some false positives
would be logged in case of different packaging formats.

In the future, we might want to consider adding the warning to the
AssemblyManager or AssemblyConfiguration classes where we do
know the exact artifacts that'll be included in the container image.

Signed-off-by: Marc Nuri <[email protected]>
  • Loading branch information
@manusa
manusa committed Nov 23, 2023
1 parent 352839d commit 111b6a2
Show file tree
Hide file tree
Showing 12 changed files with 164 additions and 511 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Usage:
```
### 1.16-SNAPSHOT
* Fix #1690: Base images based on ubi9
* Fix #2070: build goals/tasks log warning if user forgets to run package/build goal/task
* Fix #2390: support for all missing Chart.yaml fields
* Fix #2444: Add support for Spring Boot application properties placeholders

Expand Down
50 changes: 25 additions & 25 deletions jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/ArtifactUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,44 +13,44 @@
*/
package org.eclipse.jkube.kit.common.util;

import java.io.DataInputStream;
import java.io.DataOutputStream;
import org.eclipse.jkube.kit.common.KitLogger;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;

public class ArtifactUtil {
static final String LAST_MODIFIED_TIME_SAVE_FILENAME = ".jkube-final-output-artifact-last-modified";

private static final String LAST_MODIFIED_TIME_SAVE_FILENAME = ".jkube-last-modified";

private ArtifactUtil() {
}

/**
* Saves the last modified time of the current artifact to the given directory.
* Logs a warning in case the provided {@link File} artifact is stale.
*
* @param saveDirectoryPath the directory where the last modified time should be saved
* @param artifact the current artifact
* @throws IOException if an I/O error occurs
*/
public static void saveCurrentArtifactLastModifiedTime(Path saveDirectoryPath, File artifact) throws IOException {
try (DataOutputStream out = new DataOutputStream(
Files.newOutputStream(saveDirectoryPath.resolve(LAST_MODIFIED_TIME_SAVE_FILENAME)))) {
out.writeLong(artifact.lastModified());
}
}

/**
* Retrieves the last modified time of the previously built artifact from the given directory.
* This happens if the artifact was not rebuilt since the last time this method was called.
*
* @param loadDirectoryPath the directory where the last modified time was saved
* @return the last modified time of the previously built artifact
* @throws IOException if an I/O error occurs
* @param logger the {@link KitLogger} to use to log the warning.
* @param artifact the File artifact for which to check staleness.
*/
public static long retrievePreviousArtifactLastModifiedTime(Path loadDirectoryPath) throws IOException {
try (DataInputStream in = new DataInputStream(
Files.newInputStream(loadDirectoryPath.resolve(LAST_MODIFIED_TIME_SAVE_FILENAME)))) {
return in.readLong();
public static void warnStaleArtifact(KitLogger logger, File artifact) {
if (artifact == null || artifact.isDirectory() || !artifact.exists()) {
logger.warn("Final output artifact file was not detected. The project may have not been built. " +
"HINT: try to compile and package your application prior to running the container image build task.");
return;
}
final Path lastModifiedMarker = artifact.getParentFile().toPath().resolve(LAST_MODIFIED_TIME_SAVE_FILENAME);
try {
if (lastModifiedMarker.toFile().exists() &&
Long.parseLong(new String(Files.readAllBytes(lastModifiedMarker))) == artifact.lastModified()
) {
logger.info("Final output artifact file was not rebuilt since last build. " +
"HINT: try to compile and package your application prior to running the container image build task.");
}
Files.write(lastModifiedMarker, String.valueOf(artifact.lastModified()).getBytes());
} catch (Exception ex) {
// NOOP - prevent serious stuff from failing due to this hint warning
}
}
}
111 changes: 88 additions & 23 deletions jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/ArtifactUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,58 +13,123 @@
*/
package org.eclipse.jkube.kit.common.util;

import java.io.DataInputStream;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;

import org.eclipse.jkube.kit.common.KitLogger;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.jkube.kit.common.util.ArtifactUtil.LAST_MODIFIED_TIME_SAVE_FILENAME;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

class ArtifactUtilTest {

@TempDir
Path tempDir;

private File artifact;
private long currentArtifactLastModifiedTime;
private Path target;
private KitLogger kitLogger;

@BeforeEach
void setup() throws IOException {
artifact = Files.createFile(tempDir.resolve("test_artifact")).toFile();
BasicFileAttributes attr = Files.readAttributes(artifact.toPath(), BasicFileAttributes.class);
currentArtifactLastModifiedTime = attr.lastModifiedTime().toMillis();
target = tempDir.resolve("target");
FileUtil.createDirectory(target.toFile());
kitLogger = spy(new KitLogger.SilentLogger());
}

@Test
@DisplayName("Should save the Last modified time of the file correctly")
void testSavePreviousArtifactLastModifiedTime() throws IOException {
@DisplayName("with null artifact, should log missing build warning")
void nullArtifact() {
// When
ArtifactUtil.saveCurrentArtifactLastModifiedTime(tempDir, artifact);

ArtifactUtil.warnStaleArtifact(kitLogger, null);
// Then
assertThat(tempDir.resolve(LAST_MODIFIED_TIME_SAVE_FILENAME).toFile()).exists();
Path lastModifiedTimeSavePath = tempDir.resolve(LAST_MODIFIED_TIME_SAVE_FILENAME);
try (DataInputStream in = new DataInputStream(Files.newInputStream(lastModifiedTimeSavePath))) {
assertThat(in.readLong()).isEqualTo(currentArtifactLastModifiedTime);
}
verify(kitLogger).warn("Final output artifact file was not detected. The project may have not been built. " +
"HINT: try to compile and package your application prior to running the container image build task.");
}

@Test
@DisplayName("Should load the Last modified time of the file correctly")
void testRetrievePreviousArtifactLastModifiedTime() throws IOException {
@DisplayName("with nonexistent artifact, should log missing build warning")
void noArtifact() {
// When
ArtifactUtil.saveCurrentArtifactLastModifiedTime(tempDir, artifact);
long previousArtifactLastModifiedTime = ArtifactUtil.retrievePreviousArtifactLastModifiedTime(tempDir);

ArtifactUtil.warnStaleArtifact(kitLogger, target.resolve("i-dont-exist").toFile());
// Then
assertThat(previousArtifactLastModifiedTime).isEqualTo(currentArtifactLastModifiedTime);
verify(kitLogger).warn("Final output artifact file was not detected. The project may have not been built. " +
"HINT: try to compile and package your application prior to running the container image build task.");
}

@Nested
@DisplayName("with artifact")
class WithArtifact {

private long currentArtifactLastModifiedTime;
private File artifact;

@BeforeEach
void setup() throws IOException {
artifact = Files.createFile(target.resolve("the-artifact.file")).toFile();
currentArtifactLastModifiedTime = Files.readAttributes(artifact.toPath(), BasicFileAttributes.class).lastModifiedTime().toMillis();
}

@Test
@DisplayName("no previous build, should not log warning")
void noPreviousBuild() {
// When
ArtifactUtil.warnStaleArtifact(kitLogger, artifact);
// Then
verify(kitLogger, times(0)).warn(anyString());
}

@Test
@DisplayName("no previous build,saves artifact last modified time")
void noPreviousBuildSavesArtifactBuildTime() {
// When
ArtifactUtil.warnStaleArtifact(kitLogger, artifact);
// Then
assertThat(target.resolve(".jkube-last-modified").toFile())
.hasContent("" + currentArtifactLastModifiedTime);
}
@Test
@DisplayName("previous build with different time, should not log warning")
void previousBuildWithDifferentTime() throws IOException {
// Given
Files.write(target.resolve(".jkube-last-modified"), "123456789".getBytes());
// When
ArtifactUtil.warnStaleArtifact(kitLogger, artifact);
// Then
verify(kitLogger, times(0)).warn(anyString());
}

@Test
@DisplayName("previous build with different time, saves artifact last modified time")
void previousBuildWithDifferentTimeSavesArtifactBuildTime() throws IOException {
// Given
Files.write(target.resolve(".jkube-last-modified"), "123456789".getBytes());
// When
ArtifactUtil.warnStaleArtifact(kitLogger, artifact);
// Then
assertThat(target.resolve(".jkube-last-modified").toFile())
.hasContent("" + currentArtifactLastModifiedTime);
}

@Test
@DisplayName("previous build with same time, should log rebuild warning")
void previousBuildWithSameTime() throws IOException {
// Given
Files.write(target.resolve(".jkube-last-modified"), String.valueOf(currentArtifactLastModifiedTime).getBytes());
// When
ArtifactUtil.warnStaleArtifact(kitLogger, artifact);
// Then
verify(kitLogger).info("Final output artifact file was not rebuilt since last build. " +
"HINT: try to compile and package your application prior to running the container image build task.");
}
}
}
41 changes: 1 addition & 40 deletions ...it/generator/api/src/main/java/org/eclipse/jkube/generator/api/support/BaseGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,8 @@
*/
package org.eclipse.jkube.generator.api.support;

import java.io.File;
import java.io.IOException;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.format.FormatStyle;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -35,7 +31,6 @@
import org.eclipse.jkube.kit.common.JavaProject;
import org.eclipse.jkube.kit.common.PrefixedLogger;
import org.eclipse.jkube.kit.common.util.GitUtil;
import org.eclipse.jkube.kit.common.util.JKubeProjectUtil;
import org.eclipse.jkube.kit.config.image.ImageConfiguration;
import org.eclipse.jkube.kit.config.image.ImageName;
import org.eclipse.jkube.kit.config.image.build.BuildConfiguration;
Expand All @@ -49,9 +44,6 @@
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Repository;

import static org.eclipse.jkube.kit.common.util.ArtifactUtil.retrievePreviousArtifactLastModifiedTime;
import static org.eclipse.jkube.kit.common.util.ArtifactUtil.saveCurrentArtifactLastModifiedTime;

/**
* @author roland
*/
Expand Down Expand Up @@ -298,35 +290,4 @@ protected void addSchemaLabels(BuildConfiguration.BuildConfigurationBuilder buil
}
}

public void checkAndWarnIfProjectHasNotBeenBuilt() {
File finalOutputArtifact = null;
try {
finalOutputArtifact = JKubeProjectUtil.getFinalOutputArtifact(getProject());
if (finalOutputArtifact == null) {
log.error(
"Final output artifact file was not detected. The project may have not been built. " +
"HINT: try to compile and package your application prior to running the container image build task.");
return;
}
long currentLastModifiedTime = finalOutputArtifact.lastModified();
long previousLastModifiedTime = retrievePreviousArtifactLastModifiedTime(getProject().getBuildDirectory().toPath());
boolean isCurrentFinalArtifactSameAsPrevious = currentLastModifiedTime == previousLastModifiedTime;

if (isCurrentFinalArtifactSameAsPrevious) {
log.info(
"Final output artifact is the same as previous build. " +
"HINT: You might have forgotten to compile and package your application after making changes.");
}
} catch (Exception e) {
log.debug("Failed to check if final output artifact is the same as previous build. ", e);
} finally {
if (finalOutputArtifact != null) {
try {
saveCurrentArtifactLastModifiedTime(getProject().getBuildDirectory().toPath(), finalOutputArtifact);
} catch (IOException e) {
log.debug("Failed to save the last modified time of the final output artifact. ", e);
}
}
}
}
}
}
Loading

0 comments on commit 111b6a2

Please sign in to comment.