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

fix: link launch dir with existing nucleus package if does not exist #1675

Merged
merged 1 commit into from
Dec 9, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutorService;
Expand All @@ -71,6 +72,7 @@

import static com.aws.greengrass.componentmanager.KernelConfigResolver.PREV_VERSION_CONFIG_KEY;
import static com.aws.greengrass.componentmanager.KernelConfigResolver.VERSION_CONFIG_KEY;
import static com.aws.greengrass.deployment.DeviceConfiguration.DEFAULT_NUCLEUS_COMPONENT_NAME;
import static com.aws.greengrass.deployment.converter.DeploymentDocumentConverter.ANY_VERSION;
import static org.apache.commons.io.FileUtils.ONE_MB;

Expand Down Expand Up @@ -296,6 +298,37 @@ private void storeRecipeDigestInConfigStoreForPlugin(
}
}

/**
* Un-archives the artifacts for the current Nucleus version package.
*
* @return list of un-archived paths
* @throws PackageLoadingException when unable to load current Nucleus
*/
public List<Path> unArchiveCurrentNucleusVersionArtifacts() throws PackageLoadingException {
String currentNucleusVersion = deviceConfiguration.getNucleusVersion();
ComponentIdentifier nucleusComponentIdentifier =
new ComponentIdentifier(DEFAULT_NUCLEUS_COMPONENT_NAME, new Semver(currentNucleusVersion));
List<File> nucleusArtifactFileNames =
componentStore.getArtifactFiles(nucleusComponentIdentifier, artifactDownloaderFactory);
return nucleusArtifactFileNames.stream()
.map(file -> {
try {
Path unarchivePath =
nucleusPaths.unarchiveArtifactPath(nucleusComponentIdentifier, getFileName(file));
Copy link
Member

Choose a reason for hiding this comment

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

I understand that we have zip artifacts only in the nucleus component versions, but this line would always copy over the artifacts to unarchived-artifacts folder irrespective of the Unarchive type of the artifact in the recipe.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, since this code path is used only for Nucleus artifacts in the near future, I do not see a harm in hard coding the unarchive as ZIP. But I can modify to fetch unarchive from the recipe if you have a strong opinion on it.

/*
Using a hard-coded ZIP un-archiver as today this code path is only used to un-archive a Nucleus
.zip artifact.
*/
unarchiver.unarchive(Unarchive.ZIP, file, unarchivePath);
Copy link
Member

Choose a reason for hiding this comment

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

Should we check if the archive file exists on the disk before unarchiving?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is not necessary, if the file does not exist, unarchiver will throw an IOException which ComponentManager will catch and just return null. The stream will filter out nonNull objects later on. If no artifact file existed then the list returned by ComponentManager will be empty and KernelAlternatives will handle that appropriately.

return unarchivePath;
} catch (IOException e) {
logger.atDebug().setCause(e).kv("comp-id", nucleusComponentIdentifier)
.log("Could not un-archive Nucleus artifact");
return null;
}
}).filter(Objects::nonNull).collect(Collectors.toList());
}

private Optional<ComponentIdentifier> findBestCandidateLocally(String componentName,
Map<String, Requirement> versionRequirements)
throws PackagingException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.LongStream;
import javax.inject.Inject;

Expand Down Expand Up @@ -412,6 +414,37 @@ public Path resolveArtifactDirectoryPath(@NonNull ComponentIdentifier componentI
}
}

/**
* Returns the artifact file name.
*
* @param componentIdentifier packageIdentifier
* @param artifactDownloaderFactory artifact downloader factory
* @return the unarchive artifact directory path for target package.
* @throws PackageLoadingException if creating the directory fails
*/
public List<File> getArtifactFiles(@NonNull ComponentIdentifier componentIdentifier,
@NonNull ArtifactDownloaderFactory artifactDownloaderFactory)
throws PackageLoadingException {
Optional<String> componentRecipeContent = findComponentRecipeContent(componentIdentifier);
if (!componentRecipeContent.isPresent()) {
return Collections.emptyList();
}

ComponentRecipe recipe = getPackageRecipe(componentIdentifier);
Path packageArtifactDirectory = resolveArtifactDirectoryPath(componentIdentifier);
return recipe.getArtifacts().stream().map(artifact -> {
try {
return artifactDownloaderFactory
.getArtifactDownloader(componentIdentifier, artifact, packageArtifactDirectory)
.getArtifactFile();
} catch (PackageLoadingException | InvalidArtifactUriException e) {
logger.atDebug().setCause(e).kv("comp-id", componentRecipeContent)
.log("Could not get artifact file");
return null;
}
}).filter(Objects::nonNull).collect(Collectors.toList());
}

/**
* Resolve the recipe file path for a target package id.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public enum DeploymentErrorCode {
// JVM hashing issue
HASHING_ALGORITHM_UNAVAILABLE(DeploymentErrorType.DEVICE_ERROR),
// Could be a local file issue or a Nucleus issue; we will categorize as the latter for visibility
LAUNCH_DIRECTORY_CORRUPTED(DeploymentErrorType.NUCLEUS_ERROR),
LAUNCH_DIRECTORY_CORRUPTED(DeploymentErrorType.DEVICE_ERROR),

/* Component recipe errors */
RECIPE_PARSE_ERROR(DeploymentErrorType.COMPONENT_RECIPE_ERROR),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package com.aws.greengrass.lifecyclemanager;

import com.aws.greengrass.componentmanager.ComponentManager;
import com.aws.greengrass.componentmanager.exceptions.PackageLoadingException;
import com.aws.greengrass.config.Configuration;
import com.aws.greengrass.config.Topics;
import com.aws.greengrass.dependency.Context;
Expand Down Expand Up @@ -35,6 +37,8 @@
import java.nio.file.Path;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import javax.inject.Inject;

Expand Down Expand Up @@ -70,15 +74,18 @@ public class KernelAlternatives {
private static final String BOOTSTRAP_ON_ROLLBACK_CONFIG_KEY = "bootstrapOnRollback";

private final NucleusPaths nucleusPaths;
private final ComponentManager componentManager;

/**
* Constructor for KernelAlternatives, which manages the alternative launch directory of Kernel.
*
* @param nucleusPaths nucleus paths
* @param componentManager component manager
*/
@Inject
public KernelAlternatives(NucleusPaths nucleusPaths) {
public KernelAlternatives(NucleusPaths nucleusPaths, ComponentManager componentManager) {
this.nucleusPaths = nucleusPaths;
this.componentManager = componentManager;
try {
setupInitLaunchDirIfAbsent();
} catch (IOException e) {
Expand Down Expand Up @@ -162,31 +169,69 @@ public boolean isLaunchDirSetup() {
return Files.isSymbolicLink(getCurrentDir()) && validateLaunchDirSetup(getCurrentDir());
}

protected boolean canRecoverMissingLaunchDirSetup()
throws IOException, URISyntaxException, PackageLoadingException {
/*
Try and relink launch dir with the following replacement criteria
1. check if current Nucleus execution package is valid
2. un-archive current Nucleus version from component store
3. fail with DirectoryValidationException if above steps do not satisfy
*/
Path currentNucleusExecutablePath = locateCurrentKernelUnpackDir();
if (Files.exists(currentNucleusExecutablePath.resolve(KERNEL_BIN_DIR)
.resolve(Platform.getInstance().loaderFilename()))) {
logger.atDebug().kv("path", currentNucleusExecutablePath)
.log("Current Nucleus executable is valid, setting up launch dir");
relinkInitLaunchDir(currentNucleusExecutablePath, true);
return true;
}

List<Path> localNucleusExecutablePaths = componentManager.unArchiveCurrentNucleusVersionArtifacts();
saranyailla marked this conversation as resolved.
Show resolved Hide resolved
if (!localNucleusExecutablePaths.isEmpty()) {
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 you calling isEmpty on null object?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. This will still be an empty check in case artifacts dir is empty (initial installation) but this code path should only hit if a customer purposefully deletes the loader script from the unarchived dir. Imo, this is low risk.

Optional<Path> validNucleusExecutablePath = localNucleusExecutablePaths.stream()
saranyailla marked this conversation as resolved.
Show resolved Hide resolved
.filter(path -> Files.exists(path.resolve(KERNEL_BIN_DIR)
.resolve(Platform.getInstance().loaderFilename())))
.findFirst();
if (validNucleusExecutablePath.isPresent()) {
logger.atDebug().kv("path", validNucleusExecutablePath.get())
.log("Un-archived current Nucleus artifact");
relinkInitLaunchDir(validNucleusExecutablePath.get(), true);
return true;
}
}
throw new PackageLoadingException("Could not find a valid Nucleus package to recover launch dir setup");
}

/**
* Validate that launch directory is set up.
*
* @throws DirectoryValidationException when a file is missing
* @throws DeploymentException when user is not allowed to change file permission
*/
public void validateLaunchDirSetupVerbose() throws DirectoryValidationException, DeploymentException {
Path currentDir = getCurrentDir();
if (!Files.isSymbolicLink(currentDir)) {
throw new DirectoryValidationException("Missing symlink to current nucleus launch directory");
try {
if (!Files.isSymbolicLink(getCurrentDir()) || !Files.exists(getLoaderPathFromLaunchDir(getCurrentDir()))) {
logger.atInfo().log("Current launch dir setup is missing, attempting to recover");
canRecoverMissingLaunchDirSetup();
}
} catch (PackageLoadingException | IOException ex) {
throw new DirectoryValidationException("Unable to relink init launch directory", ex);
} catch (URISyntaxException ex) {
// TODO: Fix usage of root path with spaces on linux
throw new DeploymentException("Could not parse init launch directory path", ex);
}

Path currentDir = getCurrentDir();
Path loaderPath = getLoaderPathFromLaunchDir(currentDir);
if (Files.exists(loaderPath)) {
if (!loaderPath.toFile().canExecute()) {
// Ensure that the loader is executable so that we can exec it when restarting Nucleus
try {
Platform.getInstance().setPermissions(OWNER_RWX_EVERYONE_RX, loaderPath);
} catch (IOException e) {
throw new DeploymentException(
String.format("Unable to set loader script at %s as executable", loaderPath), e)
.withErrorContext(e, DeploymentErrorCode.SET_PERMISSION_ERROR);
}
if (!loaderPath.toFile().canExecute()) {
// Ensure that the loader is executable so that we can exec it when restarting Nucleus
try {
Platform.getInstance().setPermissions(OWNER_RWX_EVERYONE_RX, loaderPath);
} catch (IOException e) {
throw new DeploymentException(
String.format("Unable to set loader script at %s as executable", loaderPath), e)
.withErrorContext(e, DeploymentErrorCode.SET_PERMISSION_ERROR);
}
} else {
throw new DirectoryValidationException("Missing loader file at " + currentDir.toAbsolutePath());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,9 @@ public DirectoryValidationException(String message) {
super(message);
super.addErrorCode(DeploymentErrorCode.LAUNCH_DIRECTORY_CORRUPTED);
}

public DirectoryValidationException(String message, Throwable throwable) {
super(message, throwable);
super.addErrorCode(DeploymentErrorCode.LAUNCH_DIRECTORY_CORRUPTED);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ void GIVEN_launch_dir_corrupted_WHEN_deployment_activate_THEN_deployment_fail(Ex
assertEquals(mockException, result.getFailureCause().getCause());

List<String> expectedStack = Arrays.asList("DEPLOYMENT_FAILURE", "LAUNCH_DIRECTORY_CORRUPTED");
List<String> expectedTypes = Collections.singletonList("NUCLEUS_ERROR");
List<String> expectedTypes = Collections.singletonList("DEVICE_ERROR");
TestUtils.validateGenerateErrorReport(result.getFailureCause(), expectedStack, expectedTypes);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@

package com.aws.greengrass.lifecyclemanager;

import com.aws.greengrass.componentmanager.ComponentManager;
import com.aws.greengrass.config.PlatformResolver;
import com.aws.greengrass.deployment.DeploymentDirectoryManager;
import com.aws.greengrass.deployment.bootstrap.BootstrapManager;
import com.aws.greengrass.lifecyclemanager.exceptions.DirectoryValidationException;
import com.aws.greengrass.testcommons.testutilities.GGExtension;
import com.aws.greengrass.util.NucleusPaths;
import com.aws.greengrass.util.Utils;
Expand All @@ -34,9 +36,12 @@
import static org.hamcrest.io.FileMatchers.anExistingFileOrDirectory;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.internal.verification.VerificationModeFactory.times;
Expand All @@ -46,8 +51,7 @@ class KernelAlternativesTest {
@TempDir
Path altsDir;
@Mock
NucleusPaths nucleusPaths;

ComponentManager componentManager;
private KernelAlternatives kernelAlternatives;
@Mock
BootstrapManager bootstrapManager;
Expand All @@ -58,7 +62,7 @@ class KernelAlternativesTest {
void beforeEach() throws IOException {
NucleusPaths paths = new NucleusPaths("mock_loader_logs.log");
paths.setKernelAltsPath(altsDir);
kernelAlternatives = spy(new KernelAlternatives(paths));
kernelAlternatives = spy(new KernelAlternatives(paths, componentManager));
}

@Test
Expand Down Expand Up @@ -212,6 +216,38 @@ void GIVEN_launch_params_THEN_write_to_file() throws Exception {
assertEquals("mock string", new String(Files.readAllBytes(expectedLaunchParamsPath)));
}

@Test
void GIVEN_validate_launch_dir_setup_WHEN_current_link_missing_and_exception_THEN_directory_validation_exception() throws IOException {
// GIVEN
Path outsidePath = createRandomDirectory();
Path unpackPath = createRandomDirectory();
Files.createDirectories(unpackPath.resolve("bin"));
String loaderName = "loader";
if (PlatformResolver.isWindows) {
loaderName = "loader.cmd";
}
Files.createFile(unpackPath.resolve("bin").resolve(loaderName));

Path distroPath = kernelAlternatives.getInitDir().resolve(KERNEL_DISTRIBUTION_DIR);
Files.createDirectories(kernelAlternatives.getInitDir());
// current -> init
kernelAlternatives.setupLinkToDirectory(kernelAlternatives.getCurrentDir(), kernelAlternatives.getInitDir());
// init/distro -> outsidePath
kernelAlternatives.setupLinkToDirectory(distroPath, outsidePath);
assertEquals(kernelAlternatives.getInitDir(), Files.readSymbolicLink(kernelAlternatives.getCurrentDir()));
assertEquals(outsidePath, Files.readSymbolicLink(distroPath));

// WHEN
Files.deleteIfExists(kernelAlternatives.getCurrentDir());
lenient().doThrow(new IOException("Random test failure"))
.when(kernelAlternatives).relinkInitLaunchDir(any(Path.class), eq(true));

// THEN
DirectoryValidationException ex = assertThrows(DirectoryValidationException.class,
() -> kernelAlternatives.validateLaunchDirSetupVerbose());
assertEquals(ex.getMessage(), "Unable to relink init launch directory");
}

private Path createRandomDirectory() throws IOException {
Path path = altsDir.resolve(Utils.generateRandomString(4));
Utils.createPaths(path);
Expand Down
Loading