-
Notifications
You must be signed in to change notification settings - Fork 47
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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; | ||
|
||
|
@@ -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)); | ||
/* | ||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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; | ||
|
||
|
@@ -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) { | ||
|
@@ -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()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aren't you calling isEmpty on null object? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.