diff --git a/lib/base/defer.hpp b/lib/base/defer.hpp index 9290c92b9..2a232619a 100644 --- a/lib/base/defer.hpp +++ b/lib/base/defer.hpp @@ -30,13 +30,21 @@ public: inline ~Defer() { - try { - m_Func(); - } catch (...) { - // https://stackoverflow.com/questions/130117/throwing-exceptions-out-of-a-destructor + if (m_Func) { + try { + m_Func(); + } catch (...) { + // https://stackoverflow.com/questions/130117/throwing-exceptions-out-of-a-destructor + } } } + inline + void Cancel() + { + m_Func = nullptr; + } + private: std::function m_Func; }; diff --git a/lib/remote/configpackageutility.cpp b/lib/remote/configpackageutility.cpp index b70009997..e79540147 100644 --- a/lib/remote/configpackageutility.cpp +++ b/lib/remote/configpackageutility.cpp @@ -167,7 +167,8 @@ void ConfigPackageUtility::ActivateStage(const String& packageName, const String WritePackageConfig(packageName); } -void ConfigPackageUtility::TryActivateStageCallback(const ProcessResult& pr, const String& packageName, const String& stageName, bool activate, bool reload) +void ConfigPackageUtility::TryActivateStageCallback(const ProcessResult& pr, const String& packageName, const String& stageName, + bool activate, bool reload, const Shared::Ptr& resetPackageUpdates) { String logFile = GetPackageDir() + "/" + packageName + "/" + stageName + "/startup.log"; std::ofstream fpLog(logFile.CStr(), std::ofstream::out | std::ostream::binary | std::ostream::trunc); @@ -188,8 +189,17 @@ void ConfigPackageUtility::TryActivateStageCallback(const ProcessResult& pr, con ActivateStage(packageName, stageName); } - if (reload) + if (reload) { + /* + * Cancel the deferred callback before going out of scope so that the config stages handler + * flag isn't resetting earlier and allowing other clients to submit further requests while + * Icinga2 is reloading. Otherwise, the ongoing request will be cancelled halfway before the + * operation is completed once the new worker becomes ready. + */ + resetPackageUpdates->Cancel(); + Application::RequestRestart(); + } } } else { Log(LogCritical, "ConfigPackageUtility") @@ -198,7 +208,8 @@ void ConfigPackageUtility::TryActivateStageCallback(const ProcessResult& pr, con } } -void ConfigPackageUtility::AsyncTryActivateStage(const String& packageName, const String& stageName, bool activate, bool reload) +void ConfigPackageUtility::AsyncTryActivateStage(const String& packageName, const String& stageName, bool activate, bool reload, + const Shared::Ptr& resetPackageUpdates) { VERIFY(Application::GetArgC() >= 1); @@ -224,7 +235,9 @@ void ConfigPackageUtility::AsyncTryActivateStage(const String& packageName, cons Process::Ptr process = new Process(Process::PrepareCommand(args)); process->SetTimeout(Application::GetReloadTimeout()); - process->Run([packageName, stageName, activate, reload](const ProcessResult& pr) { TryActivateStageCallback(pr, packageName, stageName, activate, reload); }); + process->Run([packageName, stageName, activate, reload, resetPackageUpdates](const ProcessResult& pr) { + TryActivateStageCallback(pr, packageName, stageName, activate, reload, resetPackageUpdates); + }); } void ConfigPackageUtility::DeleteStage(const String& packageName, const String& stageName) diff --git a/lib/remote/configpackageutility.hpp b/lib/remote/configpackageutility.hpp index 5c16b75ee..240f5919c 100644 --- a/lib/remote/configpackageutility.hpp +++ b/lib/remote/configpackageutility.hpp @@ -8,6 +8,8 @@ #include "base/dictionary.hpp" #include "base/process.hpp" #include "base/string.hpp" +#include "base/defer.hpp" +#include "base/shared.hpp" #include namespace icinga @@ -37,7 +39,8 @@ public: static void SetActiveStage(const String& packageName, const String& stageName); static void SetActiveStageToFile(const String& packageName, const String& stageName); static void ActivateStage(const String& packageName, const String& stageName); - static void AsyncTryActivateStage(const String& packageName, const String& stageName, bool activate, bool reload); + static void AsyncTryActivateStage(const String& packageName, const String& stageName, bool activate, bool reload, + const Shared::Ptr& resetPackageUpdates); static std::vector > GetFiles(const String& packageName, const String& stageName); @@ -59,7 +62,8 @@ private: static void WritePackageConfig(const String& packageName); static void WriteStageConfig(const String& packageName, const String& stageName); - static void TryActivateStageCallback(const ProcessResult& pr, const String& packageName, const String& stageName, bool activate, bool reload); + static void TryActivateStageCallback(const ProcessResult& pr, const String& packageName, const String& stageName, bool activate, + bool reload, const Shared::Ptr& resetPackageUpdates); static bool ValidateFreshName(const String& name); }; diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index 9d7c23064..b5aaadd6c 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -5,12 +5,15 @@ #include "remote/httputility.hpp" #include "remote/filterutility.hpp" #include "base/application.hpp" +#include "base/defer.hpp" #include "base/exception.hpp" using namespace icinga; REGISTER_URLHANDLER("/v1/config/stages", ConfigStagesHandler); +std::atomic ConfigStagesHandler::m_RunningPackageUpdates (false); + bool ConfigStagesHandler::HandleRequest( AsioTlsStream& stream, const ApiUser::Ptr& user, @@ -128,12 +131,19 @@ void ConfigStagesHandler::HandlePost( if (reload && !activate) BOOST_THROW_EXCEPTION(std::invalid_argument("Parameter 'reload' must be false when 'activate' is false.")); + if (m_RunningPackageUpdates.exchange(true)) { + return HttpUtility::SendJsonError(response, params, 423, + "Conflicting request, there is already an ongoing package update in progress. Please try it again later."); + } + + auto resetPackageUpdates (Shared::Make([]() { ConfigStagesHandler::m_RunningPackageUpdates.store(false); })); + std::unique_lock lock(ConfigPackageUtility::GetStaticPackageMutex()); stageName = ConfigPackageUtility::CreateStage(packageName, files); /* validate the config. on success, activate stage and reload */ - ConfigPackageUtility::AsyncTryActivateStage(packageName, stageName, activate, reload); + ConfigPackageUtility::AsyncTryActivateStage(packageName, stageName, activate, reload, resetPackageUpdates); } catch (const std::exception& ex) { return HttpUtility::SendJsonError(response, params, 500, "Stage creation failed.", diff --git a/lib/remote/configstageshandler.hpp b/lib/remote/configstageshandler.hpp index c6d644366..88f248c8f 100644 --- a/lib/remote/configstageshandler.hpp +++ b/lib/remote/configstageshandler.hpp @@ -4,6 +4,7 @@ #define CONFIGSTAGESHANDLER_H #include "remote/httphandler.hpp" +#include namespace icinga { @@ -47,6 +48,7 @@ private: const Dictionary::Ptr& params ); + static std::atomic m_RunningPackageUpdates; }; }