Skip to content

Commit

Permalink
ARTEMIS-4397 Fixing Upgrade command
Browse files Browse the repository at this point in the history 
co-authored with Domenico Francesco Bruscino <brusdev@apache.org>
  • Loading branch information
@clebertsuconic
clebertsuconic committed Sep 14, 2023
1 parent 6ec2131 commit 536174e
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 27 deletions.
33 changes: 31 additions & 2 deletions artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/InstallAbstract.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ public class InstallAbstract extends InputAbstract {
@Option(names = "--java-memory", description = "Define the -Xmx memory parameter for the broker. Default: 2G.")
protected String javaMemory = "2G";

// "calculated" during run
protected boolean IS_WINDOWS;
// "calculated" during run
protected boolean IS_NIX;

protected String getJavaOptions() {
StringBuilder builder = new StringBuilder();
if (javaOptions != null) {
Expand Down Expand Up @@ -92,8 +97,32 @@ public File getHome() {
return home;
}

protected boolean IS_WINDOWS;
protected boolean IS_NIX;
public File getDirectory() {
return directory;
}

public InstallAbstract setDirectory(File directory) {
this.directory = directory;
return this;
}

public boolean isWindows() {
return windows;
}

public InstallAbstract setWindows(boolean windows) {
this.windows = windows;
return this;
}

public boolean isNix() {
return nix;
}

public InstallAbstract setNix(boolean nix) {
this.nix = nix;
return this;
}

public Object run(ActionContext context) throws Exception {
IS_NIX = false;
Expand Down
6 changes: 2 additions & 4 deletions artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Upgrade.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ protected void checkDirectory() {
}
}


@Override
public Object execute(ActionContext context) throws Exception {
this.checkDirectory();
Expand Down Expand Up @@ -185,10 +184,9 @@ public Object run(ActionContext context) throws Exception {
replaceLines(context, bootstrapXmlTmp, bootstrapXml, bootstrapXmlBkp,
"^(.*)<web path.*$", "$1<web path=\"web\" rootRedirectLocation=\"console\">",
"^(.*)<binding uri=\"http://localhost:8161\"(.*)$", "$1<binding name=\"artemis\" uri=\"http://localhost:8161\"$2",
"^(.*)<app(.*branding.*)$", "$1<app name=\"branding\"$2",
"^(.*)<app(.*plugin.*)$", "$1<app name=\"plugin\"$2",
"^(.*)<app url=(.*branding.*)$", "$1<app name=\"branding\" url=$2",
"^(.*)<app url=(.*plugin.*)$", "$1<app name=\"plugin\" url=$2",
"^(.*)<app url=\"([^\"]+)\"(.*)$", "$1<app name=\"$2\" url=\"$2\"$3");

upgradeLogging(context, etcFolder, etcBkp);

context.out.println();
Expand Down
33 changes: 33 additions & 0 deletions artemis-commons/src/main/java/org/apache/activemq/artemis/utils/FileUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,19 @@

import java.io.File;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Arrays;
import java.util.HashSet;

import org.apache.activemq.artemis.logs.ActiveMQUtilLogger;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static java.nio.file.attribute.PosixFilePermission.GROUP_EXECUTE;
import static java.nio.file.attribute.PosixFilePermission.GROUP_READ;
Expand All @@ -35,6 +43,8 @@

public class FileUtil {

private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

public static void makeExec(File file) throws IOException {
try {
Files.setPosixFilePermissions(file.toPath(), new HashSet<>(Arrays.asList(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE, GROUP_READ, GROUP_WRITE, GROUP_EXECUTE, OTHERS_READ, OTHERS_EXECUTE)));
Expand Down Expand Up @@ -72,4 +82,27 @@ public static final boolean deleteDirectory(final File directory) {
return directory.delete();
}

public static final void copyDirectory(final File directorySource, final File directoryTarget) throws Exception {
Path sourcePath = directorySource.toPath();
Path targetPath = directoryTarget.toPath();

try {
Files.walkFileTree(sourcePath, new SimpleFileVisitor<>() {
@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
Path targetDir = targetPath.resolve(sourcePath.relativize(dir));
Files.createDirectories(targetDir);
return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
Files.copy(file, targetPath.resolve(sourcePath.relativize(file)), StandardCopyOption.REPLACE_EXISTING);
return FileVisitResult.CONTINUE;
}
});
} catch (IOException e) {
e.printStackTrace();
}
}
}
15 changes: 15 additions & 0 deletions tests/smoke-tests/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1537,6 +1537,21 @@
<staticCluster>tcp://localhost:61616</staticCluster>
</configuration>
</execution>
<execution>
<phase>test-compile</phase>
<id>upgrade-current-version-phrase1</id>
<goals>
<goal>create</goal>
</goals>
<configuration>
<noWeb>false</noWeb>
<instance>${basedir}/target/upgrade/currentVersion</instance>
<args>
<arg>--disable-persistence</arg>
<arg>--linux</arg>
</args>
</configuration>
</execution>
</executions>
<dependencies>
<dependency>
Expand Down
127 changes: 106 additions & 21 deletions ...src/test/java/org/apache/activemq/artemis/tests/smoke/upgradeTest/CompareUpgradeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,24 @@
import static org.junit.Assert.assertTrue;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Properties;
import java.util.stream.Stream;

import org.apache.activemq.artemis.cli.commands.ActionContext;
import org.apache.activemq.artemis.cli.commands.Create;
import org.apache.activemq.artemis.cli.commands.Upgrade;
import org.apache.activemq.artemis.utils.FileUtil;
import org.junit.Assert;
import org.junit.Test;
import org.slf4j.Logger;
Expand All @@ -54,8 +63,8 @@ public void testCompareWindowsFreshInstance() throws Exception {
String windowsExpectedBin = windowsExpected + "/bin";
String windowsExpectedETC = basedir + "/target/classes/servers/windowsUpgradeETCExpected";

compareDirectories(windowsExpectedBin, windowsBin);
compareDirectories(windowsExpectedETC, windowsETC, "broker.xml", "artemis-users.properties", "management.xml");
compareDirectories(true, windowsExpectedBin, windowsBin);
compareDirectories(true, windowsExpectedETC, windowsETC, "broker.xml", "artemis-users.properties", "management.xml");

String referenceBin = basedir + "/target/reference-for-backup-check/servers/windowsUpgrade/bin";
String referenceEtc = basedir + "/target/reference-for-backup-check/servers/windowsUpgradeETC";
Expand All @@ -74,8 +83,8 @@ public void testCompareLinuxFreshInstance() throws Exception {
String linuxExpectedBin = linuxExpected + "/bin";
String linuxExpectedETC = basedir + "/target/classes/servers/linuxUpgradeETCExpected";

compareDirectories(linuxExpectedBin, linuxBin);
compareDirectories(linuxExpectedETC, linuxETC, "broker.xml", "artemis-users.properties", "management.xml");
compareDirectories(true, linuxExpectedBin, linuxBin);
compareDirectories(true, linuxExpectedETC, linuxETC, "broker.xml", "artemis-users.properties", "management.xml");

String referenceBin = basedir + "/target/reference-for-backup-check/servers/linuxUpgrade/bin";
String referenceEtc = basedir + "/target/reference-for-backup-check/servers/linuxUpgradeETC";
Expand Down Expand Up @@ -113,7 +122,7 @@ private void verifyBackupFiles(String backupFolder, String referenceFolder, Stri
}
}

private void compareDirectories(String expectedFolder, String upgradeFolder, String... ignoredFiles) throws Exception {
private void compareDirectories(boolean allowExpectedWord, String expectedFolder, String upgradeFolder, String... ignoredFiles) throws Exception {
File expectedFolderFile = new File(expectedFolder);
File[] foundFiles = expectedFolderFile.listFiles(pathname -> {
for (String i :ignoredFiles) {
Expand All @@ -137,30 +146,39 @@ private void compareDirectories(String expectedFolder, String upgradeFolder, Str
if (f.getName().endsWith(".exe")) {
Assert.assertArrayEquals(f.getName() + " is different after upgrade", Files.readAllBytes(f.toPath()), Files.readAllBytes(upgradeFile.toPath()));
} else {
try (Stream<String> expectedStream = Files.lines(f.toPath());
Stream<String> upgradeStream = Files.lines(upgradeFile.toPath())) {
compareFiles(allowExpectedWord, f, upgradeFile);
}
}
}

Iterator<String> expectedIterator = expectedStream.iterator();
Iterator<String> upgradeIterator = upgradeStream.iterator();
private static void compareFiles(boolean allowExpectedWord, File expectedFile, File upgradeFile) throws IOException {
logger.debug("comparing: {} to {}", upgradeFile, upgradeFile);
try (Stream<String> expectedStream = Files.lines(expectedFile.toPath());
Stream<String> upgradeStream = Files.lines(upgradeFile.toPath())) {

int line = 1;
Iterator<String> expectedIterator = expectedStream.iterator();
Iterator<String> upgradeIterator = upgradeStream.iterator();

while (expectedIterator.hasNext()) {
Assert.assertTrue(upgradeIterator.hasNext());
int line = 1;

String expectedString = expectedIterator.next().replace("Expected", "").trim();
String upgradeString = upgradeIterator.next().trim();
Assert.assertEquals("error on line " + line + " at " + upgradeFile, expectedString, upgradeString);
line++;
}
while (expectedIterator.hasNext()) {
Assert.assertTrue(upgradeIterator.hasNext());

Assert.assertFalse(upgradeIterator.hasNext());
}
}
}
String expectedString = expectedIterator.next().replace("Expected", "").trim();
String upgradeString = upgradeIterator.next().trim();

// there's a test in this class that will use a different name ID. on that case we replace Expected by ""
// on the comparison
if (allowExpectedWord) {
expectedString = expectedString.replace("Expected", "");
}

Assert.assertEquals("error on line " + line + " at " + upgradeFile, expectedString, upgradeString);
line++;
}

Assert.assertFalse(upgradeIterator.hasNext());
}
}

@Test
Expand Down Expand Up @@ -235,6 +253,73 @@ public void testLinux() throws Exception {
Assert.assertTrue("New Logging must be installed by upgrade", newLogging.exists());
}

@Test
public void testCompareUpgradeCurrentVersion() throws Throwable {
File upgradeConfig = new File(basedir + "/target/upgrade/currentVersion");
File originalConfig = new File(basedir + "/target/upgrade/currentVersionCopy");
FileUtil.deleteDirectory(originalConfig); // removing eventual previous runs

// for previous runs
removeBackups(upgradeConfig);

// I'm keeping the current configuration as originalConfig, to make a comparisson after upgrade is called
FileUtil.copyDirectory(upgradeConfig, originalConfig);

// looking up for the ARTEMIS_HOME from the profile file
Properties properties = new Properties();
properties.load(new FileInputStream(new File(originalConfig, "/etc/artemis.profile")));
File home = new File(parseProperty(properties, "ARTEMIS_HOME"));

Upgrade upgrade = new Upgrade();
upgrade.setHomeValues(home, null, null);
upgrade.setDirectory(upgradeConfig);
upgrade.run(new ActionContext());

// for current run
removeBackups(upgradeConfig);

// Calling upgrade on itself should not cause any changes to *any* file
// output should be exactly the same
Assert.assertTrue(compareDirectories(false, originalConfig.toPath(), upgradeConfig.toPath()));
}

private void removeBackups(File upgradeConfig) {
File[] bkpFiles = upgradeConfig.listFiles(f -> f.isDirectory() && f.getName().contains("config-bkp"));
for (File f : bkpFiles) {
logger.debug("Removing {}", f);
FileUtil.deleteDirectory(f);
}
}

private static String parseProperty(Properties properties, String name) {
String property = properties.getProperty(name);
if (property == null) {
return null;
}
// the property value might have quotes needed for bash. We need to remove those here
if (property.startsWith("'") && property.endsWith("'")) {
property = property.substring(1, property.length() - 1);
}
return property;
}


public static boolean compareDirectories(boolean allowExpectedWord, Path expected, Path upgrade) throws IOException {
Files.walkFileTree(expected, new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(Path expectedFile, BasicFileAttributes attrs) throws IOException {
FileVisitResult result = super.visitFile(expectedFile, attrs);

Path relativize = expected.relativize(expectedFile);
Path upgradeFile = upgrade.resolve(relativize);

compareFiles(allowExpectedWord, expectedFile.toFile(), upgradeFile.toFile());

return result;
}
});
return true;
}

private Map<String, String> checkExpectedValues(String fileName, String... expectedPairs) throws Exception {
Assert.assertTrue("You must pass a pair of expected values", expectedPairs.length > 0 && expectedPairs.length % 2 == 0);
Expand Down

0 comments on commit 536174e

Please sign in to comment.