From 82650fccf4bf5005dead91e1c6d4764b29f03b97 Mon Sep 17 00:00:00 2001 From: Kristian Amlie Date: Tue, 3 Sep 2024 14:06:30 +0200 Subject: [PATCH 1/5] feat: Rename `--stop-after` flag to `--stop-before`. This is done for two main reasons: First, it is more intuitive to name the state you want to stop before instead of the ones you want to stop after. For example, multiple states lead to `ArtifactRollback_Enter`; with the `--stop-after` flag you'd have to specify both `ArtifactInstall_Error` and `ArtifactCommit_Error`. Second, in the orchestrator it is not a good fit to use `--stop-after`, for reasons explained in the corresponding commits there, and renaming keeps them more in sync. This has little effect on how it works, it's just using different names, so both tests and usage stay nearly the same, just name-adjusted. Cancel-changelog: b60beaf1e16fdee593f5d173647fa5d18f3340a1 Cancel-changelog: 2c1d92212dcdba383ce0872ad108b3f6cde8c0f9 Changelog: Add `--stop-before` flag which can be used with the `install`, `commit`, and `rollback` standalone commands to stop before certain states. Use `resume` to continue, which also supports the same flag. These are the allowed states: * `ArtifactInstall_Enter` * `ArtifactCommit_Enter` * `ArtifactCommit_Leave` * `ArtifactRollback_Enter` * `ArtifactFailure_Enter` * `Cleanup` The flag can be specified multiple times. Ticket: MEN-7115 Signed-off-by: Kristian Amlie --- src/mender-update/cli/actions.cpp | 8 +-- src/mender-update/cli/actions.hpp | 6 +-- src/mender-update/cli/cli.cpp | 55 ++++++++++++--------- src/mender-update/standalone.hpp | 2 +- src/mender-update/standalone/context.hpp | 2 +- src/mender-update/standalone/standalone.cpp | 37 +++++++------- tests/src/mender-update/cli/cli_test.cpp | 48 +++++++++--------- 7 files changed, 80 insertions(+), 78 deletions(-) diff --git a/src/mender-update/cli/actions.cpp b/src/mender-update/cli/actions.cpp index 52974472b..b8cb6ee24 100644 --- a/src/mender-update/cli/actions.cpp +++ b/src/mender-update/cli/actions.cpp @@ -254,7 +254,7 @@ error::Error InstallAction::Execute(context::MenderContext &main_context) { } events::EventLoop loop; standalone::Context ctx {main_context, loop}; - ctx.stop_after = std::move(stop_after_); + ctx.stop_before = std::move(stop_before_); auto result = standalone::Install(ctx, src_); err = ResultHandler(result); if (!reboot_exit_code_ @@ -269,7 +269,7 @@ error::Error InstallAction::Execute(context::MenderContext &main_context) { error::Error ResumeAction::Execute(context::MenderContext &main_context) { events::EventLoop loop; standalone::Context ctx {main_context, loop}; - ctx.stop_after = std::move(stop_after_); + ctx.stop_before = std::move(stop_before_); auto result = standalone::Resume(ctx); auto err = ResultHandler(result); @@ -286,7 +286,7 @@ error::Error ResumeAction::Execute(context::MenderContext &main_context) { error::Error CommitAction::Execute(context::MenderContext &main_context) { events::EventLoop loop; standalone::Context ctx {main_context, loop}; - ctx.stop_after = std::move(stop_after_); + ctx.stop_before = std::move(stop_before_); auto result = standalone::Commit(ctx); return ResultHandler(result); } @@ -294,7 +294,7 @@ error::Error CommitAction::Execute(context::MenderContext &main_context) { error::Error RollbackAction::Execute(context::MenderContext &main_context) { events::EventLoop loop; standalone::Context ctx {main_context, loop}; - ctx.stop_after = std::move(stop_after_); + ctx.stop_before = std::move(stop_before_); auto result = standalone::Rollback(ctx); return ResultHandler(result); } diff --git a/src/mender-update/cli/actions.hpp b/src/mender-update/cli/actions.hpp index d04a39e64..3e6641c6c 100644 --- a/src/mender-update/cli/actions.hpp +++ b/src/mender-update/cli/actions.hpp @@ -56,13 +56,13 @@ class BaseInstallAction : virtual public Action { reboot_exit_code_ = val; } - void SetStopAfter(vector val) { - stop_after_ = std::move(val); + void SetStopBefore(vector val) { + stop_before_ = std::move(val); } protected: bool reboot_exit_code_ {false}; - vector stop_after_; + vector stop_before_; }; class InstallAction : public BaseInstallAction { diff --git a/src/mender-update/cli/cli.cpp b/src/mender-update/cli/cli.cpp index 06eed69d6..942d77d35 100644 --- a/src/mender-update/cli/cli.cpp +++ b/src/mender-update/cli/cli.cpp @@ -47,11 +47,17 @@ const conf::CliCommand cmd_check_update { .description = "Force update check", }; -const conf::CliOption opt_stop_after { - .long_option = "stop-after", +const conf::CliOption opt_stop_before { + .long_option = "stop-before", .description = - "Stop after the given state has completed. " - "Choices are `Download`, `ArtifactInstall` and `ArtifactCommit`. " + "Stop before entering the given state. " + "Choices are " + "`ArtifactInstall_Enter`, " + "`ArtifactCommit_Enter`, " + "`ArtifactCommit_Leave`, " + "`ArtifactRollback_Enter`, " + "`ArtifactFailure_Enter`, " + "and `Cleanup`. " "You can later resume the installation by using the `resume` command. " "Note that the client always stops after `ArtifactInstall` if the update module supports rollback.", .parameter = "STATE", @@ -62,7 +68,7 @@ const conf::CliCommand cmd_commit { .description = "Commit current Artifact. Returns (2) if no update in progress", .options = { - opt_stop_after, + opt_stop_before, }, }; @@ -86,7 +92,7 @@ const conf::CliCommand cmd_install { .description = "Return exit code 4 if a manual reboot is required after the Artifact installation.", }, - opt_stop_after, + opt_stop_before, }, }; @@ -100,7 +106,7 @@ const conf::CliCommand cmd_resume { .description = "Return exit code 4 if a manual reboot is required after the Artifact installation.", }, - opt_stop_after, + opt_stop_before, }, }; @@ -109,7 +115,7 @@ const conf::CliCommand cmd_rollback { .description = "Rollback current Artifact. Returns (2) if no update in progress", .options = { - opt_stop_after, + opt_stop_before, }, }; @@ -155,7 +161,7 @@ static error::Error CommonInstallFlagsHandler( conf::CmdlineOptionsIterator &iter, string *filename, bool *reboot_exit_code, - vector *stop_after) { + vector *stop_before) { while (true) { auto arg = iter.Next(); if (!arg) { @@ -166,11 +172,12 @@ static error::Error CommonInstallFlagsHandler( if (reboot_exit_code != nullptr and value.option == "--reboot-exit-code") { *reboot_exit_code = true; continue; - } else if (stop_after != nullptr and value.option == "--stop-after") { + } else if (stop_before != nullptr and value.option == "--stop-before") { if (value.value == "") { - return conf::MakeError(conf::InvalidOptionsError, "--stop-after needs an argument"); + return conf::MakeError( + conf::InvalidOptionsError, "--stop-before needs an argument"); } - stop_after->push_back(value.value); + stop_before->push_back(value.value); continue; } else if (value.option != "") { return conf::MakeError(conf::InvalidOptionsError, "No such option: " + value.option); @@ -229,53 +236,53 @@ ExpectedActionPtr ParseUpdateArguments( string filename; bool reboot_exit_code = false; - vector stop_after; - auto err = CommonInstallFlagsHandler(iter, &filename, &reboot_exit_code, &stop_after); + vector stop_before; + auto err = CommonInstallFlagsHandler(iter, &filename, &reboot_exit_code, &stop_before); if (err != error::NoError) { return expected::unexpected(err); } auto install_action = make_shared(filename); install_action->SetRebootExitCode(reboot_exit_code); - install_action->SetStopAfter(std::move(stop_after)); + install_action->SetStopBefore(std::move(stop_before)); return install_action; } else if (start[0] == "resume") { conf::CmdlineOptionsIterator iter(start + 1, end, cmd_resume.options); bool reboot_exit_code = false; - vector stop_after; - auto err = CommonInstallFlagsHandler(iter, nullptr, &reboot_exit_code, &stop_after); + vector stop_before; + auto err = CommonInstallFlagsHandler(iter, nullptr, &reboot_exit_code, &stop_before); if (err != error::NoError) { return expected::unexpected(err); } auto resume_action = make_shared(); resume_action->SetRebootExitCode(reboot_exit_code); - resume_action->SetStopAfter(std::move(stop_after)); + resume_action->SetStopBefore(std::move(stop_before)); return resume_action; } else if (start[0] == "commit") { conf::CmdlineOptionsIterator iter(start + 1, end, cmd_commit.options); - vector stop_after; - auto err = CommonInstallFlagsHandler(iter, nullptr, nullptr, &stop_after); + vector stop_before; + auto err = CommonInstallFlagsHandler(iter, nullptr, nullptr, &stop_before); if (err != error::NoError) { return expected::unexpected(err); } auto commit_action = make_shared(); - commit_action->SetStopAfter(std::move(stop_after)); + commit_action->SetStopBefore(std::move(stop_before)); return commit_action; } else if (start[0] == "rollback") { conf::CmdlineOptionsIterator iter(start + 1, end, cmd_rollback.options); - vector stop_after; - auto err = CommonInstallFlagsHandler(iter, nullptr, nullptr, &stop_after); + vector stop_before; + auto err = CommonInstallFlagsHandler(iter, nullptr, nullptr, &stop_before); if (err != error::NoError) { return expected::unexpected(err); } auto rollback_action = make_shared(); - rollback_action->SetStopAfter(std::move(stop_after)); + rollback_action->SetStopBefore(std::move(stop_before)); return rollback_action; } else if (start[0] == "daemon") { conf::CmdlineOptionsIterator iter(start + 1, end, cmd_daemon.options); diff --git a/src/mender-update/standalone.hpp b/src/mender-update/standalone.hpp index c011597a2..c40cc3c5e 100644 --- a/src/mender-update/standalone.hpp +++ b/src/mender-update/standalone.hpp @@ -64,7 +64,7 @@ class StateMachine { StateMachine(Context &ctx); error::Error SetStartStateFromStateData(const string &completed_state); - error::Error AddStopAfterState(const string &state); + error::Error AddStopBeforeState(const string &state); void StartOnRollback(); void Run(); diff --git a/src/mender-update/standalone/context.hpp b/src/mender-update/standalone/context.hpp index 8436e7bde..920d6c2cb 100644 --- a/src/mender-update/standalone/context.hpp +++ b/src/mender-update/standalone/context.hpp @@ -149,7 +149,7 @@ struct Context { StateData state_data; - vector stop_after; + vector stop_before; string artifact_src; diff --git a/src/mender-update/standalone/standalone.cpp b/src/mender-update/standalone/standalone.cpp index ee19a0dd2..c87152f7b 100644 --- a/src/mender-update/standalone/standalone.cpp +++ b/src/mender-update/standalone/standalone.cpp @@ -349,7 +349,7 @@ StateMachine::StateMachine(Context &ctx) : s.AddTransition(artifact_commit_state_, se::Success, save_post_artifact_commit_state_, tf::Immediate); s.AddTransition(artifact_commit_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); - // The reason we have two save states in a row here, is that using the `--stop-after` + // The reason we have two save states in a row here, is that using the `--stop-before` // option, one can exit at the point of save_post_artifact_commit_state, and it is allowed // both to resume and roll back from here. However, once we have started executing the // `ArtifactCommit_Leave` scripts, it is no longer allowed to roll back, therefore it is @@ -436,36 +436,33 @@ error::Error StateMachine::SetStartStateFromStateData(const string &in_state) { return error::NoError; } -error::Error StateMachine::AddStopAfterState(const string &state) { +error::Error StateMachine::AddStopBeforeState(const string &state) { using tf = common::state_machine::TransitionFlag; using se = StateEvent; auto &s = state_machine_; // Replace transition in state machine in order to exit at given point. - if (state == "Download_Leave") { + if (state == "ArtifactInstall_Enter") { s.AddTransition(save_artifact_install_state_, se::Success, exit_state_, tf::Immediate); - } else if (state == "ArtifactInstall_Leave") { + } else if (state == "ArtifactCommit_Enter") { s.AddTransition(save_artifact_commit_state_, se::Success, exit_state_, tf::Immediate); - } else if (state == "ArtifactCommit") { + } else if (state == "ArtifactCommit_Leave") { s.AddTransition(save_post_artifact_commit_state_, se::Success, exit_state_, tf::Immediate); - } else if (state == "ArtifactInstall_Error" or state == "ArtifactCommit_Error") { - // Either of these two states could be entered after an error, depending on rollback - // support. + } else if (state == "ArtifactRollback_Enter") { s.AddTransition(save_artifact_rollback_state_, se::Success, exit_state_, tf::Immediate); - s.AddTransition(save_artifact_failure_state_, se::Success, exit_state_, tf::Immediate); - } else if (state == "ArtifactRollback_Leave") { + } else if (state == "ArtifactFailure_Enter") { s.AddTransition(save_artifact_failure_state_, se::Success, exit_state_, tf::Immediate); - } else if (state == "ArtifactFailure_Leave") { + } else if (state == "Cleanup") { s.AddTransition(save_cleanup_state_, se::Success, exit_state_, tf::Immediate); } else if (state != "") { return context::MakeError( - context::ValueError, "Cannot stop after unsupported state " + state); + context::ValueError, "Cannot stop before unsupported state " + state); } return error::NoError; @@ -543,8 +540,8 @@ ResultAndError Install( StateMachine state_machine {ctx}; - for (const auto &state : ctx.stop_after) { - err = state_machine.AddStopAfterState(state); + for (const auto &state : ctx.stop_before) { + err = state_machine.AddStopBeforeState(state); if (err != error::NoError) { return {Result::Failed, err}; } @@ -586,8 +583,8 @@ ResultAndError Resume(Context &ctx) { return {Result::Failed, err}; } - for (const auto &state : ctx.stop_after) { - err = state_machine.AddStopAfterState(state); + for (const auto &state : ctx.stop_before) { + err = state_machine.AddStopBeforeState(state); if (err != error::NoError) { return {Result::Failed, err}; } @@ -638,8 +635,8 @@ ResultAndError Commit(Context &ctx) { return {Result::Failed, err}; } - for (const auto &state : ctx.stop_after) { - err = state_machine.AddStopAfterState(state); + for (const auto &state : ctx.stop_before) { + err = state_machine.AddStopBeforeState(state); if (err != error::NoError) { return {Result::Failed, err}; } @@ -692,8 +689,8 @@ ResultAndError Rollback(Context &ctx) { return {Result::Failed, err}; } - for (const auto &state : ctx.stop_after) { - err = state_machine.AddStopAfterState(state); + for (const auto &state : ctx.stop_before) { + err = state_machine.AddStopBeforeState(state); if (err != error::NoError) { return {Result::Failed, err}; } diff --git a/tests/src/mender-update/cli/cli_test.cpp b/tests/src/mender-update/cli/cli_test.cpp index a95a97696..c711b52df 100644 --- a/tests/src/mender-update/cli/cli_test.cpp +++ b/tests/src/mender-update/cli/cli_test.cpp @@ -2040,7 +2040,7 @@ artifact_name=test )")); } -TEST(CliTest, StopAfterDownloadThenResume) { +TEST(CliTest, StopBeforeArtifactInstallThenResume) { mtesting::TemporaryDirectory tmpdir; ASSERT_TRUE(InitDefaultProvides(tmpdir.Path())); @@ -2070,8 +2070,8 @@ exit 0 "--datastore", tmpdir.Path(), "install", - "--stop-after", - "Download_Leave", + "--stop-before", + "ArtifactInstall_Enter", artifact, }; @@ -2150,7 +2150,7 @@ artifact_name=test )")); } -TEST(CliTest, StopAfterCommitThenResume) { +TEST(CliTest, StopBeforeArtifactCommit_LeaveThenResume) { mtesting::TemporaryDirectory tmpdir; ASSERT_TRUE(InitDefaultProvides(tmpdir.Path())); @@ -2174,8 +2174,8 @@ exit 0 "--datastore", tmpdir.Path(), "install", - "--stop-after", - "ArtifactCommit", + "--stop-before", + "ArtifactCommit_Leave", artifact, }; @@ -2233,7 +2233,7 @@ artifact_name=test )")); } -TEST(CliTest, StopAfterCommitCommandThenResume) { +TEST(CliTest, StopBeforeArtifactCommit_EnterThenResume) { mtesting::TemporaryDirectory tmpdir; ASSERT_TRUE(InitDefaultProvides(tmpdir.Path())); @@ -2269,8 +2269,8 @@ exit 0 "--datastore", tmpdir.Path(), "install", - "--stop-after", - "ArtifactInstall_Leave", + "--stop-before", + "ArtifactCommit_Enter", artifact, }; @@ -2297,8 +2297,8 @@ ArtifactInstall "--datastore", tmpdir.Path(), "commit", - "--stop-after", - "ArtifactCommit", + "--stop-before", + "ArtifactCommit_Leave", }; mtesting::RedirectStreamOutputs output; @@ -2355,7 +2355,7 @@ artifact_name=test EXPECT_TRUE(path::FileExists(commit_leave_run)); } -TEST(CliTest, StopAfterCommitCommandThenRollback) { +TEST(CliTest, StopBeforeArtifactCommit_EnterCommandThenRollback) { mtesting::TemporaryDirectory tmpdir; ASSERT_TRUE(InitDefaultProvides(tmpdir.Path())); @@ -2395,8 +2395,8 @@ exit 0 "--datastore", tmpdir.Path(), "install", - "--stop-after", - "ArtifactInstall_Leave", + "--stop-before", + "ArtifactCommit_Enter", artifact, }; @@ -2423,8 +2423,8 @@ ArtifactInstall "--datastore", tmpdir.Path(), "commit", - "--stop-after", - "ArtifactCommit", + "--stop-before", + "ArtifactCommit_Leave", }; mtesting::RedirectStreamOutputs output; @@ -2484,7 +2484,7 @@ artifact_name=previous EXPECT_FALSE(path::FileExists(commit_leave_run)); } -TEST(CliTest, StopAfterArtifactInstallFailure) { +TEST(CliTest, StopBeforeArtifactRollback_Enter) { mtesting::TemporaryDirectory tmpdir; ASSERT_TRUE(InitDefaultProvides(tmpdir.Path())); @@ -2514,10 +2514,8 @@ exit 0 "--datastore", tmpdir.Path(), "install", - "--stop-after", - "ArtifactInstall_Leave", - "--stop-after", - "ArtifactInstall_Error", + "--stop-before", + "ArtifactRollback_Enter", artifact, }; @@ -2545,8 +2543,8 @@ SupportsRollback "--datastore", tmpdir.Path(), "rollback", - "--stop-after", - "ArtifactRollback_Leave", + "--stop-before", + "ArtifactFailure_Enter", }; mtesting::RedirectStreamOutputs output; @@ -2573,8 +2571,8 @@ ArtifactRollback "--datastore", tmpdir.Path(), "resume", - "--stop-after", - "ArtifactFailure_Leave", + "--stop-before", + "Cleanup", }; mtesting::RedirectStreamOutputs output; From a8b36bb40ab0207650183b7a81d43bda2f974095 Mon Sep 17 00:00:00 2001 From: Kristian Amlie Date: Tue, 24 Sep 2024 13:25:21 +0200 Subject: [PATCH 2/5] refac: Move info output to the final logging step. This allows us to be a bit more selective when deciding whether or not to print it, which helps in upcoming commits. Signed-off-by: Kristian Amlie --- src/mender-update/cli/actions.cpp | 9 +++++++++ src/mender-update/standalone/context.hpp | 1 + src/mender-update/standalone/states.cpp | 3 ++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/mender-update/cli/actions.cpp b/src/mender-update/cli/actions.cpp index b8cb6ee24..7a795a475 100644 --- a/src/mender-update/cli/actions.cpp +++ b/src/mender-update/cli/actions.cpp @@ -207,8 +207,14 @@ static error::Error ResultHandler(standalone::ResultAndError result) { ? operation_failure : operation_done; + string prefix; string additional; + if (contains(r::AutoCommitWanted) and contains(r::Installed) + and (contains(r::Committed) or contains(r::Failed))) { + prefix = "Update Module doesn't support rollback. Committing immediately."; + } + if (contains(r::RollbackFailed)) { additional = "Rollback failed. " @@ -237,6 +243,9 @@ static error::Error ResultHandler(standalone::ResultAndError result) { } } + if (prefix.size() > 0) { + cout << prefix << endl; + } if (operation.size() > 0) { cout << operation << endl; } diff --git a/src/mender-update/standalone/context.hpp b/src/mender-update/standalone/context.hpp index 920d6c2cb..74023cd78 100644 --- a/src/mender-update/standalone/context.hpp +++ b/src/mender-update/standalone/context.hpp @@ -107,6 +107,7 @@ enum class Result { RollbackFailed = 0x2000, Cleaned = 0x4000, CleanupFailed = 0x8000, + AutoCommitWanted = 0x10000, }; // enum classes cannot ordinarily be used as bit flags, but let's provide some convenience functions diff --git a/src/mender-update/standalone/states.cpp b/src/mender-update/standalone/states.cpp index 8ee8d8b62..4cb80e294 100644 --- a/src/mender-update/standalone/states.cpp +++ b/src/mender-update/standalone/states.cpp @@ -386,9 +386,10 @@ void RebootAndRollbackQueryState::OnEnter(Context &ctx, sm::EventPoster Date: Wed, 25 Sep 2024 09:43:05 +0200 Subject: [PATCH 3/5] fix: Fall back on bootloader when boot env modification tool is broken. Changelog: Title Ticket: None Signed-off-by: Kristian Amlie --- support/modules/rootfs-image | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/support/modules/rootfs-image b/support/modules/rootfs-image index b7de1c1fa..c52b171fc 100755 --- a/support/modules/rootfs-image +++ b/support/modules/rootfs-image @@ -278,7 +278,9 @@ EOF # this point. if test "$upgrade_available" = 1; then - ${SETENV} -s - < Date: Wed, 25 Sep 2024 10:54:35 +0200 Subject: [PATCH 4/5] test: Simplify test conditions by removing the query states. They have no effect on the flow anyway. Signed-off-by: Kristian Amlie --- tests/src/mender-update/cli/cli_test.cpp | 337 +++++++++++++++-------- 1 file changed, 216 insertions(+), 121 deletions(-) diff --git a/tests/src/mender-update/cli/cli_test.cpp b/tests/src/mender-update/cli/cli_test.cpp index c711b52df..d39d08c71 100644 --- a/tests/src/mender-update/cli/cli_test.cpp +++ b/tests/src/mender-update/cli/cli_test.cpp @@ -434,7 +434,14 @@ TEST(CliTest, InstallAndCommitArtifact) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac exit 0 )")); @@ -463,8 +470,6 @@ Installed and committed. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -489,7 +494,14 @@ TEST(CliTest, InstallAndCommitArtifactCheckProvidesDepends) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac exit 0 )")); @@ -518,8 +530,6 @@ Installed and committed. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -579,7 +589,14 @@ TEST(CliTest, DownloadWithFileSizesInstallAndCommitArtifact) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in ProvidePayloadFileSizes) @@ -620,8 +637,6 @@ Installed and committed. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes DownloadWithFileSizes ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -647,7 +662,14 @@ TEST(CliTest, InstallAndThenCommitArtifact) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in SupportsRollback) @@ -682,8 +704,6 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); { @@ -707,8 +727,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -733,7 +751,14 @@ TEST(CliTest, InstallAndThenRollBackArtifact) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in SupportsRollback) @@ -768,8 +793,6 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); { @@ -793,9 +816,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback -SupportsRollback ArtifactRollback ArtifactFailure Cleanup @@ -821,7 +841,14 @@ TEST(CliTest, RollbackAfterFailure) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in ArtifactInstall) @@ -861,7 +888,6 @@ Rolled back. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -SupportsRollback ArtifactRollback ArtifactFailure Cleanup @@ -888,7 +914,14 @@ TEST(CliTest, RollbackAfterFailureInDownload) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in Download) @@ -949,7 +982,14 @@ TEST(CliTest, FailedRollbackAfterFailure) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in ArtifactInstall) @@ -989,7 +1029,6 @@ Rollback failed. System may be in an inconsistent state. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -SupportsRollback ArtifactRollback ArtifactFailure Cleanup @@ -1012,7 +1051,14 @@ TEST(CliTest, NoRollbackAfterFailure) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in ArtifactInstall) @@ -1049,7 +1095,6 @@ Update Module does not support rollback. System may be in an inconsistent state. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -SupportsRollback ArtifactFailure Cleanup )")); @@ -1135,7 +1180,14 @@ TEST(CliTest, TryToRollBackWithoutSupport) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in SupportsRollback) @@ -1170,15 +1222,20 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); ASSERT_TRUE(PrepareUpdateModule(update_module, R"(#!/bin/bash TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac exit 0 )")); @@ -1206,9 +1263,6 @@ exit 0 path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback -SupportsRollback )")); EXPECT_TRUE(VerifyProvides(tmpdir.Path(), R"(rootfs-image.version=previous @@ -1228,7 +1282,14 @@ TEST(CliTest, InstallWithRebootRequiredNoArgument) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in NeedsArtifactReboot) @@ -1264,8 +1325,6 @@ At least one payload requested a reboot of the device it updated. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -1287,7 +1346,14 @@ TEST(CliTest, InstallWithRebootRequiredWithArgument) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in NeedsArtifactReboot) @@ -1324,8 +1390,6 @@ At least one payload requested a reboot of the device it updated. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -1350,7 +1414,14 @@ TEST(CliTest, InstallWhenUpdateInProgress) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in SupportsRollback) @@ -1385,8 +1456,6 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); { @@ -1406,8 +1475,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); } @@ -1425,7 +1492,14 @@ TEST(CliTest, InstallAndThenFailRollBack) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in ArtifactRollback) @@ -1463,8 +1537,6 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); { @@ -1491,9 +1563,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback -SupportsRollback ArtifactRollback ArtifactFailure Cleanup @@ -1534,7 +1603,14 @@ TEST(CliTest, InstallAndFailCleanup) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in Cleanup) @@ -1573,8 +1649,6 @@ Cleanup failed. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -1596,7 +1670,14 @@ TEST(CliTest, FailureInArtifactFailure) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in ArtifactInstall) @@ -1636,7 +1717,6 @@ Rollback failed. System may be in an inconsistent state. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -SupportsRollback ArtifactRollback ArtifactFailure Cleanup @@ -1693,7 +1773,14 @@ TEST(CliTest, InstallAndThenCommitLegacyArtifact) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in SupportsRollback) @@ -1728,8 +1815,6 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); { @@ -1753,8 +1838,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -1778,7 +1861,14 @@ TEST(CliTest, InstallUsingOldClientAndThenCommitArtifact) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in SupportsRollback) @@ -1815,8 +1905,6 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); // Remove the Update Module working directory. This is what would have happened if upgrading @@ -1847,8 +1935,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -1874,7 +1960,14 @@ TEST(CliTest, InstallUsingOldClientAndThenRollBackArtifact) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in SupportsRollback) @@ -1911,8 +2004,6 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); // Remove the Update Module working directory. This is what would have happened if upgrading @@ -1943,9 +2034,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback -SupportsRollback ArtifactRollback ArtifactFailure Cleanup @@ -1997,7 +2085,14 @@ TEST(CliTest, InstallAndCommitArtifactFromNetwork) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac exit 0 )")); @@ -2028,8 +2123,6 @@ Installed and committed. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -2054,7 +2147,14 @@ TEST(CliTest, StopBeforeArtifactInstallThenResume) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac case "$1" in SupportsRollback) @@ -2113,8 +2213,6 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback )")); { @@ -2138,8 +2236,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -2164,7 +2260,14 @@ TEST(CliTest, StopBeforeArtifactCommit_LeaveThenResume) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac exit 0 )")); @@ -2195,8 +2298,6 @@ Installed and committed. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit )")); @@ -2221,8 +2322,6 @@ ArtifactCommit path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -NeedsArtifactReboot -SupportsRollback ArtifactCommit Cleanup )")); @@ -2259,7 +2358,14 @@ touch )" << commit_leave_run TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac exit 0 )")); @@ -2381,7 +2487,14 @@ touch )" << commit_leave_run TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac if [ "$1" = "SupportsRollback" ]; then echo "Yes" @@ -2469,7 +2582,6 @@ ArtifactCommit Download ArtifactInstall ArtifactCommit -SupportsRollback ArtifactRollback ArtifactFailure Cleanup @@ -2498,7 +2610,14 @@ TEST(CliTest, StopBeforeArtifactRollback_Enter) { TEST_DIR=")" + tmpdir.Path() + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac if [ "$1" = "SupportsRollback" ]; then echo "Yes" @@ -2535,7 +2654,6 @@ Installation failed. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -SupportsRollback )")); { @@ -2561,8 +2679,6 @@ SupportsRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -SupportsRollback -SupportsRollback ArtifactRollback )")); @@ -2589,8 +2705,6 @@ ArtifactRollback path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -SupportsRollback -SupportsRollback ArtifactRollback ArtifactFailure )")); @@ -2616,8 +2730,6 @@ ArtifactFailure path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall -SupportsRollback -SupportsRollback ArtifactRollback ArtifactFailure Cleanup @@ -2663,8 +2775,6 @@ Download_Leave_01 ArtifactInstall_Enter_01 ArtifactInstall ArtifactInstall_Leave_01 -NeedsArtifactReboot -SupportsRollback ArtifactCommit_Enter_01 ArtifactCommit ArtifactCommit_Leave_01 @@ -2778,7 +2888,6 @@ Download_Leave_01 ArtifactInstall_Enter_01 ArtifactInstall_Error_01 ArtifactInstall_Error_02 -SupportsRollback ArtifactFailure_Enter_01 ArtifactFailure ArtifactFailure_Leave_01 @@ -2810,7 +2919,6 @@ ArtifactInstall_Enter_01 ArtifactInstall ArtifactInstall_Error_01 ArtifactInstall_Error_02 -SupportsRollback ArtifactFailure_Enter_01 ArtifactFailure ArtifactFailure_Leave_01 @@ -2846,7 +2954,6 @@ ArtifactInstall_Leave_01 ArtifactInstall_Leave_02 ArtifactInstall_Error_01 ArtifactInstall_Error_02 -SupportsRollback ArtifactFailure_Enter_01 ArtifactFailure ArtifactFailure_Leave_01 @@ -2881,12 +2988,9 @@ ArtifactInstall_Enter_01 ArtifactInstall_Enter_02 ArtifactInstall ArtifactInstall_Leave_01 -NeedsArtifactReboot -SupportsRollback ArtifactCommit_Enter_01 ArtifactCommit_Error_01 ArtifactCommit_Error_02 -SupportsRollback ArtifactFailure_Enter_01 ArtifactFailure ArtifactFailure_Leave_01 @@ -2919,13 +3023,10 @@ ArtifactInstall_Enter_01 ArtifactInstall_Enter_02 ArtifactInstall ArtifactInstall_Leave_01 -NeedsArtifactReboot -SupportsRollback ArtifactCommit_Enter_01 ArtifactCommit ArtifactCommit_Error_01 ArtifactCommit_Error_02 -SupportsRollback ArtifactFailure_Enter_01 ArtifactFailure ArtifactFailure_Leave_01 @@ -2958,8 +3059,6 @@ ArtifactInstall_Enter_01 ArtifactInstall ArtifactInstall_Leave_01 ArtifactInstall_Leave_02 -NeedsArtifactReboot -SupportsRollback ArtifactCommit_Enter_01 ArtifactCommit ArtifactCommit_Leave_01 @@ -2993,8 +3092,6 @@ ArtifactInstall_Enter_01 ArtifactInstall ArtifactInstall_Leave_01 ArtifactInstall_Leave_02 -NeedsArtifactReboot -SupportsRollback ArtifactCommit_Enter_01 ArtifactCommit ArtifactCommit_Leave_01 @@ -3030,10 +3127,7 @@ ArtifactInstall_Enter_01 ArtifactInstall_Enter_02 ArtifactInstall ArtifactInstall_Leave_01 -NeedsArtifactReboot -SupportsRollback ArtifactCommit_Enter_01 -SupportsRollback ArtifactFailure_Enter_01 ArtifactFailure_Enter_02 ArtifactFailure @@ -3069,10 +3163,7 @@ ArtifactInstall_Enter_01 ArtifactInstall_Enter_02 ArtifactInstall ArtifactInstall_Leave_01 -NeedsArtifactReboot -SupportsRollback ArtifactCommit_Enter_01 -SupportsRollback ArtifactFailure_Enter_01 ArtifactFailure_Enter_02 ArtifactFailure @@ -3109,9 +3200,6 @@ ArtifactInstall_Enter_01 ArtifactInstall_Enter_02 ArtifactInstall ArtifactInstall_Leave_01 -NeedsArtifactReboot -SupportsRollback -SupportsRollback ArtifactRollback_Enter_01 ArtifactRollback ArtifactRollback_Leave_01 @@ -3210,7 +3298,14 @@ TEST_P(StandaloneStateScriptTest, AllScriptSuccess) { string script = R"(#!/bin/bash TEST_DIR=")" + tmpdir_path + R"(" -echo "$1" >> $TEST_DIR/call.log +case "$1" in + NeedsArtifactReboot|SupportsRollback) + : + ;; + *) + echo "$1" >> $TEST_DIR/call.log + ;; +esac )"; if (common::StartsWith(GetParam().case_name, "rollback")) { script += R"( From 056d9c26fa4261d09b40de9244be535209bb3e05 Mon Sep 17 00:00:00 2001 From: Kristian Amlie Date: Wed, 25 Sep 2024 10:54:35 +0200 Subject: [PATCH 5/5] chore: Increase robustness of `--stop-before flag. First and foremost, make sure that repeated resuming before the same state, with the same `--stop-before` flag, is a noop. It should keep stopping there. Also make sure that once we have started executing the path after that, it should no longer be possible to go back. For this we introduce some new DB flags, but since the existing ones have not been released yet, this should be fine. Signed-off-by: Kristian Amlie --- src/mender-update/standalone.hpp | 16 +- src/mender-update/standalone/context.cpp | 4 +- src/mender-update/standalone/context.hpp | 4 +- src/mender-update/standalone/standalone.cpp | 200 ++++++++++++-------- src/mender-update/standalone/states.cpp | 12 +- src/mender-update/standalone/states.hpp | 16 +- tests/src/mender-update/cli/cli_test.cpp | 180 ++++++++++++++++++ 7 files changed, 337 insertions(+), 95 deletions(-) diff --git a/src/mender-update/standalone.hpp b/src/mender-update/standalone.hpp index c40cc3c5e..600e4c266 100644 --- a/src/mender-update/standalone.hpp +++ b/src/mender-update/standalone.hpp @@ -81,7 +81,8 @@ class StateMachine { ScriptRunnerState download_leave_state_; ScriptRunnerState download_error_state_; - SaveState save_artifact_install_state_; + JustSaveState save_before_artifact_install_state_; + JustSaveState save_artifact_install_state_; ScriptRunnerState artifact_install_enter_state_; ArtifactInstallState artifact_install_state_; ScriptRunnerState artifact_install_leave_state_; @@ -89,27 +90,28 @@ class StateMachine { RebootAndRollbackQueryState reboot_and_rollback_query_state_; - SaveState save_artifact_commit_state_; + JustSaveState save_before_artifact_commit_state_; + JustSaveState save_artifact_commit_state_; ScriptRunnerState artifact_commit_enter_state_; ArtifactCommitState artifact_commit_state_; - SaveState save_post_artifact_commit_state_; - SaveState save_artifact_commit_leave_state_; + JustSaveState save_before_artifact_commit_leave_state_; + JustSaveState save_artifact_commit_leave_state_; ScriptRunnerState artifact_commit_leave_state_; ScriptRunnerState artifact_commit_error_state_; RollbackQueryState rollback_query_state_; - SaveState save_artifact_rollback_state_; + JustSaveState save_artifact_rollback_state_; ScriptRunnerState artifact_rollback_enter_state_; ArtifactRollbackState artifact_rollback_state_; ScriptRunnerState artifact_rollback_leave_state_; - SaveState save_artifact_failure_state_; + JustSaveState save_artifact_failure_state_; ScriptRunnerState artifact_failure_enter_state_; ArtifactFailureState artifact_failure_state_; ScriptRunnerState artifact_failure_leave_state_; - SaveState save_cleanup_state_; + JustSaveState save_cleanup_state_; CleanupState cleanup_state_; ExitState exit_state_; diff --git a/src/mender-update/standalone/context.cpp b/src/mender-update/standalone/context.cpp index 11fb6afbb..303700a42 100644 --- a/src/mender-update/standalone/context.cpp +++ b/src/mender-update/standalone/context.cpp @@ -28,9 +28,11 @@ const string StateDataKeys::in_state {"InState"}; const string StateDataKeys::failed {"Failed"}; const string StateDataKeys::rolled_back {"RolledBack"}; +const string StateData::kBeforeStateArtifactInstall_Enter {"Before_ArtifactInstall_Enter"}; const string StateData::kInStateArtifactInstall_Enter {"ArtifactInstall_Enter"}; +const string StateData::kBeforeStateArtifactCommit_Enter {"Before_ArtifactCommit_Enter"}; const string StateData::kInStateArtifactCommit_Enter {"ArtifactCommit_Enter"}; -const string StateData::kInStatePostArtifactCommit {"PostArtifactCommit"}; +const string StateData::kBeforeStateArtifactCommit_Leave {"Before_ArtifactCommit_Leave"}; const string StateData::kInStateArtifactCommit_Leave {"ArtifactCommit_Leave"}; const string StateData::kInStateArtifactRollback_Enter {"ArtifactRollback_Enter"}; const string StateData::kInStateArtifactFailure_Enter {"ArtifactFailure_Enter"}; diff --git a/src/mender-update/standalone/context.hpp b/src/mender-update/standalone/context.hpp index 74023cd78..1de0acfcf 100644 --- a/src/mender-update/standalone/context.hpp +++ b/src/mender-update/standalone/context.hpp @@ -76,9 +76,11 @@ struct StateData { bool failed {false}; bool rolled_back {false}; + static const string kBeforeStateArtifactInstall_Enter; static const string kInStateArtifactInstall_Enter; + static const string kBeforeStateArtifactCommit_Enter; static const string kInStateArtifactCommit_Enter; - static const string kInStatePostArtifactCommit; + static const string kBeforeStateArtifactCommit_Leave; static const string kInStateArtifactCommit_Leave; static const string kInStateArtifactRollback_Enter; static const string kInStateArtifactFailure_Enter; diff --git a/src/mender-update/standalone/standalone.cpp b/src/mender-update/standalone/standalone.cpp index c87152f7b..8808eb97f 100644 --- a/src/mender-update/standalone/standalone.cpp +++ b/src/mender-update/standalone/standalone.cpp @@ -240,6 +240,7 @@ StateMachine::StateMachine(Context &ctx) : executor::Action::Error, executor::OnError::Ignore, Result::NoResult}, + save_before_artifact_install_state_ {StateData::kBeforeStateArtifactInstall_Enter}, save_artifact_install_state_ {StateData::kInStateArtifactInstall_Enter}, artifact_install_enter_state_ { executor::State::ArtifactInstall, @@ -256,13 +257,14 @@ StateMachine::StateMachine(Context &ctx) : executor::Action::Error, executor::OnError::Ignore, Result::NoResult}, + save_before_artifact_commit_state_ {StateData::kBeforeStateArtifactCommit_Enter}, save_artifact_commit_state_ {StateData::kInStateArtifactCommit_Enter}, artifact_commit_enter_state_ { executor::State::ArtifactCommit, executor::Action::Enter, executor::OnError::Fail, Result::CommitFailed | Result::Failed}, - save_post_artifact_commit_state_ {StateData::kInStatePostArtifactCommit}, + save_before_artifact_commit_leave_state_ {StateData::kBeforeStateArtifactCommit_Leave}, save_artifact_commit_leave_state_ {StateData::kInStateArtifactCommit_Leave}, artifact_commit_leave_state_ { executor::State::ArtifactCommit, @@ -305,101 +307,122 @@ StateMachine::StateMachine(Context &ctx) : auto &s = state_machine_; // clang-format off - s.AddTransition(prepare_download_state_, se::Success, download_enter_state_, tf::Immediate); - s.AddTransition(prepare_download_state_, se::Failure, exit_state_, tf::Immediate); - s.AddTransition(prepare_download_state_, se::EmptyPayloadArtifact, exit_state_, tf::Immediate); + s.AddTransition(prepare_download_state_, se::Success, download_enter_state_, tf::Immediate); + s.AddTransition(prepare_download_state_, se::Failure, exit_state_, tf::Immediate); + s.AddTransition(prepare_download_state_, se::EmptyPayloadArtifact, exit_state_, tf::Immediate); - s.AddTransition(download_enter_state_, se::Success, download_state_, tf::Immediate); - s.AddTransition(download_enter_state_, se::Failure, download_error_state_, tf::Immediate); + s.AddTransition(download_enter_state_, se::Success, download_state_, tf::Immediate); + s.AddTransition(download_enter_state_, se::Failure, download_error_state_, tf::Immediate); - s.AddTransition(download_state_, se::Success, download_leave_state_, tf::Immediate); - s.AddTransition(download_state_, se::Failure, download_error_state_, tf::Immediate); + s.AddTransition(download_state_, se::Success, download_leave_state_, tf::Immediate); + s.AddTransition(download_state_, se::Failure, download_error_state_, tf::Immediate); - s.AddTransition(download_leave_state_, se::Success, save_artifact_install_state_, tf::Immediate); - s.AddTransition(download_leave_state_, se::Failure, download_error_state_, tf::Immediate); + s.AddTransition(download_leave_state_, se::Success, save_before_artifact_install_state_, tf::Immediate); + s.AddTransition(download_leave_state_, se::Failure, download_error_state_, tf::Immediate); - s.AddTransition(download_error_state_, se::Success, save_cleanup_state_, tf::Immediate); - s.AddTransition(download_error_state_, se::Failure, save_cleanup_state_, tf::Immediate); + s.AddTransition(download_error_state_, se::Success, save_cleanup_state_, tf::Immediate); + s.AddTransition(download_error_state_, se::Failure, save_cleanup_state_, tf::Immediate); - s.AddTransition(save_artifact_install_state_, se::Success, artifact_install_enter_state_, tf::Immediate); - s.AddTransition(save_artifact_install_state_, se::Failure, save_cleanup_state_, tf::Immediate); + // The reason we have a "save_before" state followed by a "save" state is the + // `--stop-before` argument. We want to make sure that: - s.AddTransition(artifact_install_enter_state_, se::Success, artifact_install_state_, tf::Immediate); - s.AddTransition(artifact_install_enter_state_, se::Failure, artifact_install_error_state_, tf::Immediate); + // 1. If you specify the flag twice in a row, the second run is a noop (just stops at the + // same point). This is accomplished using the "save_before" state, which we return to + // during a DB recovery. - s.AddTransition(artifact_install_state_, se::Success, artifact_install_leave_state_, tf::Immediate); - s.AddTransition(artifact_install_state_, se::Failure, artifact_install_error_state_, tf::Immediate); + // 2. If we have started executing the following states, it should no longer we possible to + // use the `--stop-before` flag for that state, since the state execution has started + // already. This is done by saving a different value in the "save" state, and is thus + // preserved even after a spontaneous reboot. Once we have gone there, there is no going + // back. + s.AddTransition(save_before_artifact_install_state_, se::Success, save_artifact_install_state_, tf::Immediate); + s.AddTransition(save_before_artifact_install_state_, se::Failure, save_cleanup_state_, tf::Immediate); - s.AddTransition(artifact_install_leave_state_, se::Success, save_artifact_commit_state_, tf::Immediate); - s.AddTransition(artifact_install_leave_state_, se::Failure, artifact_install_error_state_, tf::Immediate); + s.AddTransition(save_artifact_install_state_, se::Success, artifact_install_enter_state_, tf::Immediate); + s.AddTransition(save_artifact_install_state_, se::Failure, save_cleanup_state_, tf::Immediate); - s.AddTransition(artifact_install_error_state_, se::Success, rollback_query_state_, tf::Immediate); - s.AddTransition(artifact_install_error_state_, se::Failure, rollback_query_state_, tf::Immediate); + s.AddTransition(artifact_install_enter_state_, se::Success, artifact_install_state_, tf::Immediate); + s.AddTransition(artifact_install_enter_state_, se::Failure, artifact_install_error_state_, tf::Immediate); - s.AddTransition(save_artifact_commit_state_, se::Success, reboot_and_rollback_query_state_, tf::Immediate); - s.AddTransition(save_artifact_commit_state_, se::Failure, reboot_and_rollback_query_state_, tf::Immediate); + s.AddTransition(artifact_install_state_, se::Success, artifact_install_leave_state_, tf::Immediate); + s.AddTransition(artifact_install_state_, se::Failure, artifact_install_error_state_, tf::Immediate); - s.AddTransition(reboot_and_rollback_query_state_, se::Success, artifact_commit_enter_state_, tf::Immediate); - s.AddTransition(reboot_and_rollback_query_state_, se::Failure, rollback_query_state_, tf::Immediate); - s.AddTransition(reboot_and_rollback_query_state_, se::NeedsInteraction, exit_state_, tf::Immediate); + s.AddTransition(artifact_install_leave_state_, se::Success, reboot_and_rollback_query_state_, tf::Immediate); + s.AddTransition(artifact_install_leave_state_, se::Failure, artifact_install_error_state_, tf::Immediate); - s.AddTransition(artifact_commit_enter_state_, se::Success, artifact_commit_state_, tf::Immediate); - s.AddTransition(artifact_commit_enter_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); + s.AddTransition(artifact_install_error_state_, se::Success, rollback_query_state_, tf::Immediate); + s.AddTransition(artifact_install_error_state_, se::Failure, rollback_query_state_, tf::Immediate); - s.AddTransition(artifact_commit_state_, se::Success, save_post_artifact_commit_state_, tf::Immediate); - s.AddTransition(artifact_commit_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); + s.AddTransition(reboot_and_rollback_query_state_, se::Success, save_before_artifact_commit_state_, tf::Immediate); + s.AddTransition(reboot_and_rollback_query_state_, se::Failure, rollback_query_state_, tf::Immediate); + s.AddTransition(reboot_and_rollback_query_state_, se::NeedsInteraction, exit_state_, tf::Immediate); - // The reason we have two save states in a row here, is that using the `--stop-before` - // option, one can exit at the point of save_post_artifact_commit_state, and it is allowed - // both to resume and roll back from here. However, once we have started executing the - // `ArtifactCommit_Leave` scripts, it is no longer allowed to roll back, therefore it is - // important to save that as a *separate* state. - s.AddTransition(save_post_artifact_commit_state_, se::Success, save_artifact_commit_leave_state_, tf::Immediate); - s.AddTransition(save_post_artifact_commit_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); + // See `save_before_artifact_install_state_` for an explanation of the following two states. + s.AddTransition(save_before_artifact_commit_state_, se::Success, save_artifact_commit_state_, tf::Immediate); + s.AddTransition(save_before_artifact_commit_state_, se::Failure, rollback_query_state_, tf::Immediate); - s.AddTransition(save_artifact_commit_leave_state_, se::Success, artifact_commit_leave_state_, tf::Immediate); - s.AddTransition(save_artifact_commit_leave_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); + s.AddTransition(save_artifact_commit_state_, se::Success, artifact_commit_enter_state_, tf::Immediate); + s.AddTransition(save_artifact_commit_state_, se::Failure, rollback_query_state_, tf::Immediate); - s.AddTransition(artifact_commit_leave_state_, se::Success, save_cleanup_state_, tf::Immediate); - s.AddTransition(artifact_commit_leave_state_, se::Failure, save_cleanup_state_, tf::Immediate); + s.AddTransition(artifact_commit_enter_state_, se::Success, artifact_commit_state_, tf::Immediate); + s.AddTransition(artifact_commit_enter_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); - s.AddTransition(rollback_query_state_, se::Success, save_artifact_rollback_state_, tf::Immediate); - s.AddTransition(rollback_query_state_, se::NothingToDo, save_artifact_failure_state_, tf::Immediate); - s.AddTransition(rollback_query_state_, se::Failure, save_artifact_failure_state_, tf::Immediate); - s.AddTransition(rollback_query_state_, se::NeedsInteraction, exit_state_, tf::Immediate); + s.AddTransition(artifact_commit_state_, se::Success, save_before_artifact_commit_leave_state_, tf::Immediate); + s.AddTransition(artifact_commit_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); - s.AddTransition(artifact_commit_error_state_, se::Success, rollback_query_state_, tf::Immediate); - s.AddTransition(artifact_commit_error_state_, se::Failure, rollback_query_state_, tf::Immediate); + // See `save_before_artifact_install_state_` for an explanation of the following two states. + s.AddTransition(save_before_artifact_commit_leave_state_, se::Success, save_artifact_commit_leave_state_, tf::Immediate); + s.AddTransition(save_before_artifact_commit_leave_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); - s.AddTransition(save_artifact_rollback_state_, se::Success, artifact_rollback_enter_state_, tf::Immediate); - s.AddTransition(save_artifact_rollback_state_, se::Failure, artifact_rollback_enter_state_, tf::Immediate); + s.AddTransition(save_artifact_commit_leave_state_, se::Success, artifact_commit_leave_state_, tf::Immediate); + s.AddTransition(save_artifact_commit_leave_state_, se::Failure, artifact_commit_error_state_, tf::Immediate); - s.AddTransition(artifact_rollback_enter_state_, se::Success, artifact_rollback_state_, tf::Immediate); - s.AddTransition(artifact_rollback_enter_state_, se::Failure, artifact_rollback_state_, tf::Immediate); + s.AddTransition(artifact_commit_leave_state_, se::Success, save_cleanup_state_, tf::Immediate); + s.AddTransition(artifact_commit_leave_state_, se::Failure, save_cleanup_state_, tf::Immediate); - s.AddTransition(artifact_rollback_state_, se::Success, artifact_rollback_leave_state_, tf::Immediate); - s.AddTransition(artifact_rollback_state_, se::Failure, artifact_rollback_leave_state_, tf::Immediate); + s.AddTransition(rollback_query_state_, se::Success, save_artifact_rollback_state_, tf::Immediate); + s.AddTransition(rollback_query_state_, se::NothingToDo, save_artifact_failure_state_, tf::Immediate); + s.AddTransition(rollback_query_state_, se::Failure, save_artifact_failure_state_, tf::Immediate); + s.AddTransition(rollback_query_state_, se::NeedsInteraction, exit_state_, tf::Immediate); - s.AddTransition(artifact_rollback_leave_state_, se::Success, save_artifact_failure_state_, tf::Immediate); - s.AddTransition(artifact_rollback_leave_state_, se::Failure, save_artifact_failure_state_, tf::Immediate); + s.AddTransition(artifact_commit_error_state_, se::Success, rollback_query_state_, tf::Immediate); + s.AddTransition(artifact_commit_error_state_, se::Failure, rollback_query_state_, tf::Immediate); - s.AddTransition(save_artifact_failure_state_, se::Success, artifact_failure_enter_state_, tf::Immediate); - s.AddTransition(save_artifact_failure_state_, se::Failure, artifact_failure_enter_state_, tf::Immediate); + // Note: States on the error path are supposed to be idempotent, and may execute several + // times if interrupted by a powerloss. Hence they don't need `save_before` states. See + // `save_before_artifact_install_state_` for more context. + s.AddTransition(save_artifact_rollback_state_, se::Success, artifact_rollback_enter_state_, tf::Immediate); + s.AddTransition(save_artifact_rollback_state_, se::Failure, artifact_rollback_enter_state_, tf::Immediate); - s.AddTransition(artifact_failure_enter_state_, se::Success, artifact_failure_state_, tf::Immediate); - s.AddTransition(artifact_failure_enter_state_, se::Failure, artifact_failure_state_, tf::Immediate); + s.AddTransition(artifact_rollback_enter_state_, se::Success, artifact_rollback_state_, tf::Immediate); + s.AddTransition(artifact_rollback_enter_state_, se::Failure, artifact_rollback_state_, tf::Immediate); - s.AddTransition(artifact_failure_state_, se::Success, artifact_failure_leave_state_, tf::Immediate); - s.AddTransition(artifact_failure_state_, se::Failure, artifact_failure_leave_state_, tf::Immediate); + s.AddTransition(artifact_rollback_state_, se::Success, artifact_rollback_leave_state_, tf::Immediate); + s.AddTransition(artifact_rollback_state_, se::Failure, artifact_rollback_leave_state_, tf::Immediate); - s.AddTransition(artifact_failure_leave_state_, se::Success, save_cleanup_state_, tf::Immediate); - s.AddTransition(artifact_failure_leave_state_, se::Failure, save_cleanup_state_, tf::Immediate); + s.AddTransition(artifact_rollback_leave_state_, se::Success, save_artifact_failure_state_, tf::Immediate); + s.AddTransition(artifact_rollback_leave_state_, se::Failure, save_artifact_failure_state_, tf::Immediate); - s.AddTransition(save_cleanup_state_, se::Success, cleanup_state_, tf::Immediate); - s.AddTransition(save_cleanup_state_, se::Failure, cleanup_state_, tf::Immediate); + // See comment for `save_artifact_rollback_state_`. + s.AddTransition(save_artifact_failure_state_, se::Success, artifact_failure_enter_state_, tf::Immediate); + s.AddTransition(save_artifact_failure_state_, se::Failure, artifact_failure_enter_state_, tf::Immediate); - s.AddTransition(cleanup_state_, se::Success, exit_state_, tf::Immediate); - s.AddTransition(cleanup_state_, se::Failure, exit_state_, tf::Immediate); + s.AddTransition(artifact_failure_enter_state_, se::Success, artifact_failure_state_, tf::Immediate); + s.AddTransition(artifact_failure_enter_state_, se::Failure, artifact_failure_state_, tf::Immediate); + + s.AddTransition(artifact_failure_state_, se::Success, artifact_failure_leave_state_, tf::Immediate); + s.AddTransition(artifact_failure_state_, se::Failure, artifact_failure_leave_state_, tf::Immediate); + + s.AddTransition(artifact_failure_leave_state_, se::Success, save_cleanup_state_, tf::Immediate); + s.AddTransition(artifact_failure_leave_state_, se::Failure, save_cleanup_state_, tf::Immediate); + + // See comment for `save_artifact_rollback_state_`. While cleanup is not strictly an error + // state, it is idempotent. + s.AddTransition(save_cleanup_state_, se::Success, cleanup_state_, tf::Immediate); + s.AddTransition(save_cleanup_state_, se::Failure, cleanup_state_, tf::Immediate); + + s.AddTransition(cleanup_state_, se::Success, exit_state_, tf::Immediate); + s.AddTransition(cleanup_state_, se::Failure, exit_state_, tf::Immediate); // clang-format on } @@ -414,20 +437,24 @@ void StateMachine::Run() { } error::Error StateMachine::SetStartStateFromStateData(const string &in_state) { - if (in_state == StateData::kInStateArtifactInstall_Enter) { - start_state_ = &artifact_install_enter_state_; + if (in_state == StateData::kBeforeStateArtifactInstall_Enter) { + start_state_ = &save_before_artifact_install_state_; + } else if (in_state == StateData::kInStateArtifactInstall_Enter) { + start_state_ = &save_artifact_rollback_state_; + } else if (in_state == StateData::kBeforeStateArtifactCommit_Enter) { + start_state_ = &save_before_artifact_commit_state_; } else if (in_state == StateData::kInStateArtifactCommit_Enter) { - start_state_ = &artifact_commit_enter_state_; - } else if (in_state == StateData::kInStatePostArtifactCommit) { - start_state_ = &save_artifact_commit_leave_state_; + start_state_ = &save_artifact_rollback_state_; + } else if (in_state == StateData::kBeforeStateArtifactCommit_Leave) { + start_state_ = &save_before_artifact_commit_leave_state_; } else if (in_state == StateData::kInStateArtifactCommit_Leave) { start_state_ = &artifact_commit_leave_state_; } else if (in_state == StateData::kInStateCleanup) { - start_state_ = &cleanup_state_; + start_state_ = &save_cleanup_state_; } else if (in_state == StateData::kInStateArtifactRollback_Enter) { - start_state_ = &artifact_rollback_enter_state_; + start_state_ = &save_artifact_rollback_state_; } else if (in_state == StateData::kInStateArtifactFailure_Enter) { - start_state_ = &artifact_failure_enter_state_; + start_state_ = &save_artifact_failure_state_; } else { return context::MakeError( context::DatabaseValueError, "Invalid InState in database " + in_state); @@ -443,22 +470,28 @@ error::Error StateMachine::AddStopBeforeState(const string &state) { // Replace transition in state machine in order to exit at given point. if (state == "ArtifactInstall_Enter") { - s.AddTransition(save_artifact_install_state_, se::Success, exit_state_, tf::Immediate); + s.AddTransition( + save_before_artifact_install_state_, se::Success, exit_state_, tf::Immediate); } else if (state == "ArtifactCommit_Enter") { - s.AddTransition(save_artifact_commit_state_, se::Success, exit_state_, tf::Immediate); + s.AddTransition( + save_before_artifact_commit_state_, se::Success, exit_state_, tf::Immediate); } else if (state == "ArtifactCommit_Leave") { - s.AddTransition(save_post_artifact_commit_state_, se::Success, exit_state_, tf::Immediate); + s.AddTransition( + save_before_artifact_commit_leave_state_, se::Success, exit_state_, tf::Immediate); } else if (state == "ArtifactRollback_Enter") { s.AddTransition(save_artifact_rollback_state_, se::Success, exit_state_, tf::Immediate); + s.AddTransition(save_artifact_rollback_state_, se::Failure, exit_state_, tf::Immediate); } else if (state == "ArtifactFailure_Enter") { s.AddTransition(save_artifact_failure_state_, se::Success, exit_state_, tf::Immediate); + s.AddTransition(save_artifact_failure_state_, se::Failure, exit_state_, tf::Immediate); } else if (state == "Cleanup") { s.AddTransition(save_cleanup_state_, se::Success, exit_state_, tf::Immediate); + s.AddTransition(save_cleanup_state_, se::Failure, exit_state_, tf::Immediate); } else if (state != "") { return context::MakeError( @@ -609,7 +642,8 @@ ResultAndError Commit(Context &ctx) { } ctx.state_data = in_progress.value(); - if (ctx.state_data.in_state != StateData::kInStateArtifactCommit_Enter) { + if (ctx.state_data.in_state != StateData::kBeforeStateArtifactCommit_Enter + and ctx.state_data.in_state != StateData::kInStateArtifactCommit_Enter) { return { Result::Failed, context::MakeError( @@ -661,9 +695,11 @@ ResultAndError Rollback(Context &ctx) { } ctx.state_data = in_progress.value(); - if (ctx.state_data.in_state != StateData::kInStateArtifactInstall_Enter + if (ctx.state_data.in_state != StateData::kBeforeStateArtifactInstall_Enter + and ctx.state_data.in_state != StateData::kInStateArtifactInstall_Enter + and ctx.state_data.in_state != StateData::kBeforeStateArtifactCommit_Enter and ctx.state_data.in_state != StateData::kInStateArtifactCommit_Enter - and ctx.state_data.in_state != StateData::kInStatePostArtifactCommit + and ctx.state_data.in_state != StateData::kBeforeStateArtifactCommit_Leave and ctx.state_data.in_state != StateData::kInStateArtifactRollback_Enter) { return { Result::Failed, diff --git a/src/mender-update/standalone/states.cpp b/src/mender-update/standalone/states.cpp index 4cb80e294..9dd51ef6a 100644 --- a/src/mender-update/standalone/states.cpp +++ b/src/mender-update/standalone/states.cpp @@ -69,6 +69,11 @@ void SaveState::OnEnter(Context &ctx, sm::EventPoster &poster) { return; } + OnEnterSaveState(ctx, poster); +} + +void JustSaveState::OnEnterSaveState(Context &ctx, sm::EventPoster &poster) { + // Nothing other than saving, which has already happened. poster.PostEvent(StateEvent::Success); } @@ -362,7 +367,12 @@ void ArtifactInstallState::OnEnter(Context &ctx, sm::EventPoster &po poster.PostEvent(StateEvent::Success); } -void RebootAndRollbackQueryState::OnEnter(Context &ctx, sm::EventPoster &poster) { +RebootAndRollbackQueryState::RebootAndRollbackQueryState() : + SaveState(StateData::kBeforeStateArtifactCommit_Enter) { +} + +void RebootAndRollbackQueryState::OnEnterSaveState( + Context &ctx, sm::EventPoster &poster) { auto reboot = ctx.update_module->NeedsReboot(); if (!reboot) { log::Error("Could not query for reboot: " + reboot.error().String()); diff --git a/src/mender-update/standalone/states.hpp b/src/mender-update/standalone/states.hpp index 597e01f3a..96178efc5 100644 --- a/src/mender-update/standalone/states.hpp +++ b/src/mender-update/standalone/states.hpp @@ -35,12 +35,21 @@ class SaveState : virtual public StateType { SaveState(const string &state) : state_ {state} { } - void OnEnter(Context &ctx, sm::EventPoster &poster) override; + void OnEnter(Context &ctx, sm::EventPoster &poster) override final; + virtual void OnEnterSaveState(Context &ctx, sm::EventPoster &poster) = 0; private: string state_; }; +class JustSaveState : public SaveState { +public: + JustSaveState(const string &state) : + SaveState {state} { + } + void OnEnterSaveState(Context &ctx, sm::EventPoster &poster) override; +}; + class PrepareDownloadState : virtual public StateType { public: void OnEnter(Context &ctx, sm::EventPoster &poster) override; @@ -56,9 +65,10 @@ class ArtifactInstallState : virtual public StateType { void OnEnter(Context &ctx, sm::EventPoster &poster) override; }; -class RebootAndRollbackQueryState : virtual public StateType { +class RebootAndRollbackQueryState : public SaveState { public: - void OnEnter(Context &ctx, sm::EventPoster &poster) override; + RebootAndRollbackQueryState(); + void OnEnterSaveState(Context &ctx, sm::EventPoster &poster) override; }; class ArtifactCommitState : virtual public StateType { diff --git a/tests/src/mender-update/cli/cli_test.cpp b/tests/src/mender-update/cli/cli_test.cpp index d39d08c71..590d1a3ca 100644 --- a/tests/src/mender-update/cli/cli_test.cpp +++ b/tests/src/mender-update/cli/cli_test.cpp @@ -2189,6 +2189,30 @@ Streamed to storage, but not installed/enabled. EXPECT_TRUE(mtesting::FileContainsExactly( path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download +)")); + + { + // Stopping at the same place again should be a no-op. + vector args { + "--datastore", + tmpdir.Path(), + "resume", + "--stop-before", + "ArtifactInstall_Enter", + }; + + mtesting::RedirectStreamOutputs output; + int exit_status = cli::Main( + args, [&tmpdir](context::MenderContext &ctx) { SetTestDir(tmpdir.Path(), ctx); }); + EXPECT_EQ(exit_status, 0) << exit_status; + + EXPECT_EQ(output.GetCout(), ""); + EXPECT_EQ(output.GetCerr(), ""); + } + + EXPECT_TRUE(mtesting::FileContainsExactly( + path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes +Download )")); { @@ -2299,6 +2323,32 @@ Installed and committed. Download ArtifactInstall ArtifactCommit +)")); + + { + // Stopping at the same place again should be a no-op. + vector args { + "--datastore", + tmpdir.Path(), + "resume", + "--stop-before", + "ArtifactCommit_Leave", + }; + + mtesting::RedirectStreamOutputs output; + int exit_status = cli::Main( + args, [&tmpdir](context::MenderContext &ctx) { SetTestDir(tmpdir.Path(), ctx); }); + EXPECT_EQ(exit_status, 0) << exit_status; + + EXPECT_EQ(output.GetCout(), ""); + EXPECT_EQ(output.GetCerr(), ""); + } + + EXPECT_TRUE(mtesting::FileContainsExactly( + path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes +Download +ArtifactInstall +ArtifactCommit )")); { @@ -2396,6 +2446,31 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall +)")); + + { + // Stopping at the same place again should be a no-op. + vector args { + "--datastore", + tmpdir.Path(), + "resume", + "--stop-before", + "ArtifactCommit_Enter", + }; + + mtesting::RedirectStreamOutputs output; + int exit_status = cli::Main( + args, [&tmpdir](context::MenderContext &ctx) { SetTestDir(tmpdir.Path(), ctx); }); + EXPECT_EQ(exit_status, 0) << exit_status; + + EXPECT_EQ(output.GetCout(), ""); + EXPECT_EQ(output.GetCerr(), ""); + } + + EXPECT_TRUE(mtesting::FileContainsExactly( + path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes +Download +ArtifactInstall )")); { @@ -2529,6 +2604,31 @@ Use 'commit' to update, or 'rollback' to roll back the update. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall +)")); + + { + // Stopping at the same place again should be a no-op. + vector args { + "--datastore", + tmpdir.Path(), + "resume", + "--stop-before", + "ArtifactCommit_Enter", + }; + + mtesting::RedirectStreamOutputs output; + int exit_status = cli::Main( + args, [&tmpdir](context::MenderContext &ctx) { SetTestDir(tmpdir.Path(), ctx); }); + EXPECT_EQ(exit_status, 0) << exit_status; + + EXPECT_EQ(output.GetCout(), ""); + EXPECT_EQ(output.GetCerr(), ""); + } + + EXPECT_TRUE(mtesting::FileContainsExactly( + path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes +Download +ArtifactInstall )")); { @@ -2654,6 +2754,31 @@ Installation failed. path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes Download ArtifactInstall +)")); + + { + // Stopping at the same place again should be a no-op. + vector args { + "--datastore", + tmpdir.Path(), + "resume", + "--stop-before", + "ArtifactRollback_Enter", + }; + + mtesting::RedirectStreamOutputs output; + int exit_status = cli::Main( + args, [&tmpdir](context::MenderContext &ctx) { SetTestDir(tmpdir.Path(), ctx); }); + EXPECT_EQ(exit_status, 0) << exit_status; + + EXPECT_EQ(output.GetCout(), ""); + EXPECT_EQ(output.GetCerr(), ""); + } + + EXPECT_TRUE(mtesting::FileContainsExactly( + path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes +Download +ArtifactInstall )")); { @@ -2683,6 +2808,61 @@ ArtifactRollback )")); { + // Stopping at the same place again should be a no-op. + vector args { + "--datastore", + tmpdir.Path(), + "resume", + "--stop-before", + "ArtifactFailure_Enter", + }; + + mtesting::RedirectStreamOutputs output; + int exit_status = cli::Main( + args, [&tmpdir](context::MenderContext &ctx) { SetTestDir(tmpdir.Path(), ctx); }); + EXPECT_EQ(exit_status, 0) << exit_status; + + EXPECT_EQ(output.GetCout(), R"(Rolled back. +)"); + EXPECT_EQ(output.GetCerr(), ""); + } + + EXPECT_TRUE(mtesting::FileContainsExactly( + path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes +Download +ArtifactInstall +ArtifactRollback +)")); + + { + vector args { + "--datastore", + tmpdir.Path(), + "resume", + "--stop-before", + "Cleanup", + }; + + mtesting::RedirectStreamOutputs output; + int exit_status = cli::Main( + args, [&tmpdir](context::MenderContext &ctx) { SetTestDir(tmpdir.Path(), ctx); }); + EXPECT_EQ(exit_status, 0) << exit_status; + + EXPECT_EQ(output.GetCout(), R"(Rolled back. +)"); + EXPECT_EQ(output.GetCerr(), ""); + } + + EXPECT_TRUE(mtesting::FileContainsExactly( + path::Join(tmpdir.Path(), "call.log"), R"(ProvidePayloadFileSizes +Download +ArtifactInstall +ArtifactRollback +ArtifactFailure +)")); + + { + // Stopping at the same place again should be a no-op. vector args { "--datastore", tmpdir.Path(),