Merge lp:~ted/ubuntu-app-launch/reset-units into lp:ubuntu-app-launch

Proposed by Ted Gould
Status: Merged
Approved by: dobey
Approved revision: 410
Merged at revision: 286
Proposed branch: lp:~ted/ubuntu-app-launch/reset-units
Merge into: lp:ubuntu-app-launch
Diff against target: 212 lines (+146/-0)
4 files modified
libubuntu-app-launch/jobs-systemd.cpp (+47/-0)
libubuntu-app-launch/jobs-systemd.h (+2/-0)
tests/jobs-systemd.cpp (+24/-0)
tests/systemd-mock.h (+73/-0)
To merge this branch: bzr merge lp:~ted/ubuntu-app-launch/reset-units
Reviewer Review Type Date Requested Status
dobey (community) Approve
unity-api-1-bot continuous-integration Approve
Review via email: mp+316351@code.launchpad.net

This proposal supersedes a proposal from 2017-02-02.

Commit message

Reset failed units so they can be tried again

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:410
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/204/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1602
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1609
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1387
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1387/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1387
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1387/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1387
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1387/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1387
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1387/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1387
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1387/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1387
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1387/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/204/rebuild

review: Approve (continuous-integration)
Revision history for this message
dobey (dobey) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libubuntu-app-launch/jobs-systemd.cpp'
2--- libubuntu-app-launch/jobs-systemd.cpp 2017-02-02 15:09:24 +0000
3+++ libubuntu-app-launch/jobs-systemd.cpp 2017-02-04 04:10:31 +0000
4@@ -1342,6 +1342,9 @@
5 }
6 g_variant_dict_clear(&dict);
7
8+ /* Reset the failure bit on the unit */
9+ manager->resetUnit(unitinfo);
10+
11 /* Oh, we might want to do something now */
12 auto reason{Registry::FailureType::CRASH};
13 if (g_strcmp0(value, "exit-code") == 0)
14@@ -1368,6 +1371,50 @@
15 return sig_appFailed;
16 }
17
18+/** Requests that systemd reset a unit that has been marked as
19+ failed so that we can continue to work with it. This includes
20+ starting it anew, which can fail if it is left in the failed
21+ state. */
22+void SystemD::resetUnit(const UnitInfo& info) const
23+{
24+ auto registry = registry_.lock();
25+ auto unitname = unitName(info);
26+ auto bus = userbus_;
27+ auto cancel = registry->impl->thread.getCancellable();
28+
29+ registry->impl->thread.executeOnThread([bus, unitname, cancel] {
30+ g_dbus_connection_call(bus.get(), /* user bus */
31+ SYSTEMD_DBUS_ADDRESS, /* bus name */
32+ SYSTEMD_DBUS_PATH_MANAGER, /* path */
33+ SYSTEMD_DBUS_IFACE_MANAGER, /* interface */
34+ "ResetFailedUnit", /* method */
35+ g_variant_new("(s)", /* params */
36+ unitname.c_str()), /* param: specify unit */
37+ nullptr, /* ret type */
38+ G_DBUS_CALL_FLAGS_NONE, /* flags */
39+ -1, /* timeout */
40+ cancel.get(), /* cancellable */
41+ [](GObject* obj, GAsyncResult* res, gpointer user_data) {
42+ GError* error{nullptr};
43+ GVariant* callt = g_dbus_connection_call_finish(G_DBUS_CONNECTION(obj), res, &error);
44+
45+ if (error != nullptr)
46+ {
47+ if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
48+ {
49+ g_warning("Unable to reset failed unit: %s", error->message);
50+ }
51+ g_error_free(error);
52+ return;
53+ }
54+
55+ g_clear_pointer(&callt, g_variant_unref);
56+ g_debug("Reset Failed Unit");
57+ },
58+ nullptr);
59+ });
60+}
61+
62 } // namespace manager
63 } // namespace jobs
64 } // namespace app_launch
65
66=== modified file 'libubuntu-app-launch/jobs-systemd.h'
67--- libubuntu-app-launch/jobs-systemd.h 2017-01-27 18:12:35 +0000
68+++ libubuntu-app-launch/jobs-systemd.h 2017-02-04 04:10:31 +0000
69@@ -128,6 +128,8 @@
70
71 static std::vector<std::string> parseExec(std::list<std::pair<std::string, std::string>>& env);
72 static void application_start_cb(GObject* obj, GAsyncResult* res, gpointer user_data);
73+
74+ void resetUnit(const UnitInfo& info) const;
75 };
76
77 } // namespace manager
78
79=== modified file 'tests/jobs-systemd.cpp'
80--- tests/jobs-systemd.cpp 2017-01-28 05:27:48 +0000
81+++ tests/jobs-systemd.cpp 2017-02-04 04:10:31 +0000
82@@ -360,3 +360,27 @@
83
84 EXPECT_EQ(multipleAppID(), removeunit.get_future().get());
85 }
86+
87+TEST_F(JobsSystemd, UnitFailure)
88+{
89+ auto manager = std::make_shared<ubuntu::app_launch::jobs::manager::SystemD>(registry);
90+ registry->impl->jobs = manager;
91+
92+ ubuntu::app_launch::AppID failedappid;
93+ manager->appFailed().connect([&](const std::shared_ptr<ubuntu::app_launch::Application> &app,
94+ const std::shared_ptr<ubuntu::app_launch::Application::Instance> &inst,
95+ ubuntu::app_launch::Registry::FailureType type) { failedappid = app->appId(); });
96+
97+ systemd->managerEmitFailed({defaultJobName(), std::string{multipleAppID()}, "1234567890", 1, {}});
98+
99+ EXPECT_EVENTUALLY_EQ(multipleAppID(), failedappid);
100+
101+ std::list<std::string> resets;
102+ EXPECT_EVENTUALLY_FUNC_LT(0u, std::function<unsigned int()>([&]() {
103+ resets = systemd->resetCalls();
104+ return resets.size();
105+ }));
106+
107+ EXPECT_EQ(SystemdMock::instanceName({defaultJobName(), std::string{multipleAppID()}, "1234567890", 1, {}}),
108+ *resets.begin());
109+}
110
111=== modified file 'tests/systemd-mock.h'
112--- tests/systemd-mock.h 2017-01-28 05:27:48 +0000
113+++ tests/systemd-mock.h 2017-02-04 04:10:31 +0000
114@@ -140,6 +140,11 @@
115 "ret = '/'", &error);
116 throwError(error);
117
118+ dbus_test_dbus_mock_object_add_method(mock, managerobj, "ResetFailedUnit", G_VARIANT_TYPE_STRING,
119+ nullptr, /* ret type */
120+ "", &error);
121+ throwError(error);
122+
123 for (auto& instance : instances)
124 {
125 auto obj = dbus_test_dbus_mock_get_object(mock, instancePath(instance).c_str(),
126@@ -148,6 +153,9 @@
127 dbus_test_dbus_mock_object_add_property(mock, obj, "MainPID", G_VARIANT_TYPE_UINT32,
128 g_variant_new_uint32(instance.primaryPid), &error);
129 throwError(error);
130+ dbus_test_dbus_mock_object_add_property(mock, obj, "Result", G_VARIANT_TYPE_STRING,
131+ g_variant_new_string("success"), &error);
132+ throwError(error);
133
134 /* Control Group */
135 auto dir = g_build_filename(controlGroupPath.c_str(), instancePath(instance).c_str(), nullptr);
136@@ -432,6 +440,46 @@
137 return retval;
138 }
139
140+ std::list<std::string> resetCalls()
141+ {
142+ guint len = 0;
143+ GError* error = nullptr;
144+
145+ auto calls = dbus_test_dbus_mock_object_get_method_calls(mock, /* mock */
146+ managerobj, /* manager */
147+ "ResetFailedUnit", /* function */
148+ &len, /* number */
149+ &error /* error */
150+ );
151+
152+ if (error != nullptr)
153+ {
154+ g_warning("Unable to get 'ResetFailedUnit' calls from systemd mock: %s", error->message);
155+ g_error_free(error);
156+ throw std::runtime_error{"Mock disfunctional"};
157+ }
158+
159+ std::list<std::string> retval;
160+
161+ for (unsigned int i = 0; i < len; i++)
162+ {
163+ auto& call = calls[i];
164+ gchar* name = nullptr;
165+
166+ g_variant_get(call.params, "(&s)", &name);
167+
168+ if (name == nullptr)
169+ {
170+ g_warning("Invalid 'name' on 'ResetFailedUnit' call");
171+ continue;
172+ }
173+
174+ retval.emplace_back(name);
175+ }
176+
177+ return retval;
178+ }
179+
180 void managerClear()
181 {
182 GError* error = nullptr;
183@@ -478,4 +526,29 @@
184 throw std::runtime_error{"Mock disfunctional"};
185 }
186 }
187+
188+ void managerEmitFailed(const Instance& inst)
189+ {
190+ auto instobj =
191+ std::find_if(insts.begin(), insts.end(), [inst](const std::pair<Instance, DbusTestDbusMockObject*>& item) {
192+ return item.first.job == inst.job && item.first.appid == inst.appid &&
193+ item.first.instanceid == inst.instanceid;
194+ });
195+
196+ if (instobj == insts.end())
197+ {
198+ throw std::runtime_error{"Unable to find instance"};
199+ }
200+
201+ GError* error = nullptr;
202+ dbus_test_dbus_mock_object_update_property(mock, instobj->second, "Result", g_variant_new_string("fail"),
203+ &error);
204+
205+ if (error != nullptr)
206+ {
207+ g_warning("Unable to set result to 'fail': %s", error->message);
208+ g_error_free(error);
209+ throw std::runtime_error{"Mock disfunctional"};
210+ }
211+ }
212 };

Subscribers

People subscribed via source and target branches