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: fail deployment if prepare bootstrap fails #1657

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

Conversation

alter-mage
Copy link
Member

@alter-mage alter-mage commented Aug 28, 2024

Issue #, if available:

Description of changes:
Do not restart Nucleus if kernel update deployment fails while preparing bootstrap. Instead, fail the deployment and start up all the services currently configured.

Something which is a bit unknown is what should the behavior be like if rollback fails.

Why is this change necessary:
Nucleus restart is unnecessary if preparing bootstrap fails and adds the possibility of a Nucleus Restart Failure when the Nucleus does eventually start up.

How was this change tested:

  • Updated or added new unit tests.
  • Updated or added new integration tests.
  • Updated or added new end-to-end tests.
  • If my code makes a remote network call, it was tested with a proxy.

Any additional information or context required to review the change:

Documentation Checklist:

  • Updated the README if applicable.

Compatibility Checklist:

  • I confirm that the change is backwards compatible.
  • Any modification or deletion of public interfaces does not impact other plugin components.
  • For external library version updates, I have reviewed its change logs and Nucleus does not consume
    any deprecated method or type.

Refer to Compatibility Guidelines for more information.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@@ -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
Member

Choose a reason for hiding this comment

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

Unit tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops :(

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

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
Member Author

@alter-mage alter-mage Aug 29, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants