Skip to content

fix: fail deployment if prepare bootstrap fails #1657

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.

Sign up for GitHub

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

Open
alter-mage wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: fail deployment if prepare bootstrap fails #1657

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 19 additions & 7 deletions src/main/java/com/aws/greengrass/deployment/activator/KernelUpdateActivator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,13 @@ public void activate(Map<String, Object> newConfig, Deployment deployment,
kernelAlternatives.prepareBootstrap(deploymentDocument.getDeploymentId());
} catch (IOException e) {
// TODO: better handling of error codes for different IO operations
rollback(deployment, e);
rollback(deployment, e, false);
Copy link
Copy Markdown
Contributor

@jcosentino11 jcosentino11 Aug 28, 2024

Choose a reason for hiding this comment

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

Unit tests?

Copy link
Copy Markdown
Member Author

@alter-mage alter-mage Aug 28, 2024

Choose a reason for hiding this comment

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

oops :(

lifecycle.startupAllServices();

totallyCompleteFuture.complete(
new DeploymentResult(DeploymentResult.DeploymentStatus.FAILED_NO_STATE_CHANGE,
new DeploymentException("Unable to process deployment. Greengrass could not prepare for"
+ " bootstrap.", e)));
return;
}

Expand All @@ -105,11 +111,11 @@ public void activate(Map<String, Object> newConfig, Deployment deployment,

kernel.shutdown(30, exitCode == REQUEST_REBOOT ? REQUEST_REBOOT : REQUEST_RESTART);
} catch (ServiceUpdateException | IOException e) {
rollback(deployment, e);
rollback(deployment, e, true);
}
}

void rollback(Deployment deployment, Throwable failureCause) {
void rollback(Deployment deployment, Throwable failureCause, boolean requiresRestart) {
logger.atInfo(MERGE_CONFIG_EVENT_KEY, failureCause)
.kv(DEPLOYMENT_ID_LOG_KEY, deployment.getGreengrassDeploymentId())
.log("Rolling back failed deployment");
Expand All @@ -120,10 +126,13 @@ void rollback(Deployment deployment, Throwable failureCause) {
deployment.setErrorTypes(errorReport.getRight());
deployment.setStageDetails(Utils.generateFailureMessage(failureCause));

final boolean bootstrapOnRollbackRequired = kernelAlternatives.prepareBootstrapOnRollbackIfNeeded(
kernel.getContext(), deploymentDirectoryManager, bootstrapManager);
// Persist deployment metadata only if restart is required
if (requiresRestart) {
Copy link
Copy Markdown
Member

@yitingb yitingb Aug 29, 2024

Choose a reason for hiding this comment

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

QQ: when bootstrap prepared failed, and we don't persist the deployment metadata, if we use systemctl to restart Nucleus, would it still look for deployment metadata when restarting?

Copy link
Copy Markdown
Member Author

@alter-mage alter-mage Aug 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We will still persist deployment metadata but only in the launch directory of the deployment that failed. After rollback, the launch directory will revert back to previous one (before current deployment was received). So after restart, yes, Nucleus will still look for deployment metadata but its fine if it does not find it and will launch in default mode.

final boolean bootstrapOnRollbackRequired = kernelAlternatives.prepareBootstrapOnRollbackIfNeeded(
kernel.getContext(), deploymentDirectoryManager, bootstrapManager);

deployment.setDeploymentStage(bootstrapOnRollbackRequired ? ROLLBACK_BOOTSTRAP : KERNEL_ROLLBACK);
deployment.setDeploymentStage(bootstrapOnRollbackRequired ? ROLLBACK_BOOTSTRAP : KERNEL_ROLLBACK);
}

try {
deploymentDirectoryManager.writeDeploymentMetadata(deployment);
Expand All @@ -135,7 +144,10 @@ void rollback(Deployment deployment, Throwable failureCause) {
} catch (IOException e) {
logger.atError().setCause(e).log("Failed to set up rollback directory");
}

// Restart Kernel regardless and rely on loader orchestration
kernel.shutdown(30, REQUEST_RESTART);
if (requiresRestart) {
kernel.shutdown(30, REQUEST_RESTART);
}
}
}