Merge lp:~xnox/upstart/lp1303891 into lp:upstart

Proposed by Dimitri John Ledkov on 2014-04-09
Status: Merged
Merged at revision: 1612
Proposed branch: lp:~xnox/upstart/lp1303891
Merge into: lp:upstart
Diff against target: 295 lines (+258/-3)
2 files modified
util/initctl.c (+27/-3)
util/tests/test_initctl.c (+231/-0)
To merge this branch: bzr merge lp:~xnox/upstart/lp1303891
Reviewer Review Type Date Requested Status
James Hunt 2014-04-09 Approve on 2014-04-09
Review via email: mp+214908@code.launchpad.net

Commit message

Reintroduce previous reload semantics that simply sent SIGHUP to the main process, thus allowing to reload services during precise -> trusty upgrades.

Description of the change

Reintroduce previous reload semantics that simply sent SIGHUP to the main process, thus allowing to reload services during precise -> trusty upgrades.

To post a comment you must log in.
lp:~xnox/upstart/lp1303891 updated on 2014-04-09
1613. By Dimitri John Ledkov on 2014-04-09

* Use fallback path, and only if we received unknown dbus method error
* Rework tests to test fallback path in such a case

James Hunt (jamesodhunt) wrote :

LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'util/initctl.c'
2--- util/initctl.c 2013-10-25 13:49:49 +0000
3+++ util/initctl.c 2014-04-09 12:59:58 +0000
4@@ -1033,9 +1033,33 @@
5
6 job->auto_start = FALSE;
7
8- if (job_reload_sync (NULL, job) < 0)
9- goto error;
10-
11+ if (job_reload_sync (NULL, job) < 0) {
12+ /* well reload_sync call should always work... unless we
13+ * didn't reboot since upgrade and pid1 is still old
14+ * upstart that does not have reload_sync call, fallback
15+ * to sending SIGHUP to main process
16+ */
17+ err = nih_error_get ();
18+ if (strcmp(((NihDBusError *)err)->name, DBUS_ERROR_UNKNOWN_METHOD)) {
19+ nih_error ("%s", err->message);
20+ nih_free (err);
21+ return 1;
22+ }
23+ nih_free (err);
24+
25+ if (job_get_processes_sync (NULL, job, &processes) < 0)
26+ goto error;
27+
28+ if ((! processes[0]) || strcmp (processes[0]->item0, "main")) {
29+ nih_error (_("Not running"));
30+ return 1;
31+ }
32+
33+ if (kill (processes[0]->item1, SIGHUP) < 0) {
34+ nih_error_raise_system ();
35+ goto error;
36+ }
37+ }
38 return 0;
39
40 error:
41
42=== modified file 'util/tests/test_initctl.c'
43--- util/tests/test_initctl.c 2014-03-06 15:21:54 +0000
44+++ util/tests/test_initctl.c 2014-04-09 12:59:58 +0000
45@@ -8462,12 +8462,20 @@
46 FILE * output;
47 FILE * errors;
48 pid_t server_pid;
49+ pid_t proc_pid;
50 DBusMessage * method_call;
51 DBusMessage * reply = NULL;
52 const char * name_value;
53 char ** args_value;
54 int args_elements;
55 const char * str_value;
56+ const char * interface;
57+ const char * property;
58+ DBusMessageIter iter;
59+ DBusMessageIter subiter;
60+ DBusMessageIter arrayiter;
61+ DBusMessageIter structiter;
62+ int32_t int32_value;
63 NihCommand command;
64 char * args[4];
65 int ret = 0;
66@@ -9139,6 +9147,229 @@
67 }
68
69
70+ /* Check that if an error is received from the Reload call,
71+ * the fallback path is used to query main pid and SIGHUP
72+ * that.
73+ */
74+ TEST_FEATURE ("with error reply to Reload");
75+ TEST_ALLOC_FAIL {
76+ TEST_CHILD (proc_pid) {
77+ pause ();
78+ }
79+
80+ TEST_CHILD (server_pid) {
81+ /* Expect the GetJobByName method call on the
82+ * manager object, make sure the job name is passed
83+ * and reply with a path.
84+ */
85+ TEST_DBUS_MESSAGE (server_conn, method_call);
86+
87+ TEST_TRUE (dbus_message_is_method_call (method_call,
88+ DBUS_INTERFACE_UPSTART,
89+ "GetJobByName"));
90+
91+ TEST_EQ_STR (dbus_message_get_path (method_call),
92+ DBUS_PATH_UPSTART);
93+
94+ TEST_TRUE (dbus_message_get_args (method_call, NULL,
95+ DBUS_TYPE_STRING, &name_value,
96+ DBUS_TYPE_INVALID));
97+
98+ TEST_EQ_STR (name_value, "test");
99+
100+ TEST_ALLOC_SAFE {
101+ reply = dbus_message_new_method_return (method_call);
102+
103+ str_value = DBUS_PATH_UPSTART "/jobs/test";
104+
105+ dbus_message_append_args (reply,
106+ DBUS_TYPE_OBJECT_PATH, &str_value,
107+ DBUS_TYPE_INVALID);
108+ }
109+
110+ dbus_connection_send (server_conn, reply, NULL);
111+ dbus_connection_flush (server_conn);
112+
113+ dbus_message_unref (method_call);
114+ dbus_message_unref (reply);
115+
116+ /* Expect the GetInstance method call on the
117+ * job object, make sure the environment args are
118+ * passed and reply with a path.
119+ */
120+ TEST_DBUS_MESSAGE (server_conn, method_call);
121+
122+ TEST_TRUE (dbus_message_is_method_call (method_call,
123+ DBUS_INTERFACE_UPSTART_JOB,
124+ "GetInstance"));
125+
126+ TEST_EQ_STR (dbus_message_get_path (method_call),
127+ DBUS_PATH_UPSTART "/jobs/test");
128+
129+ TEST_TRUE (dbus_message_get_args (method_call, NULL,
130+ DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &args_value, &args_elements,
131+ DBUS_TYPE_INVALID));
132+
133+ TEST_EQ (args_elements, 0);
134+ dbus_free_string_array (args_value);
135+
136+ TEST_ALLOC_SAFE {
137+ reply = dbus_message_new_method_return (method_call);
138+
139+ str_value = DBUS_PATH_UPSTART "/jobs/test/_";
140+
141+ dbus_message_append_args (reply,
142+ DBUS_TYPE_OBJECT_PATH, &str_value,
143+ DBUS_TYPE_INVALID);
144+ }
145+
146+ dbus_connection_send (server_conn, reply, NULL);
147+ dbus_connection_flush (server_conn);
148+
149+ dbus_message_unref (method_call);
150+ dbus_message_unref (reply);
151+
152+ /* Expect the Reload method call against job
153+ * instance and reply with an error.
154+ */
155+ TEST_DBUS_MESSAGE (server_conn, method_call);
156+
157+ TEST_TRUE (dbus_message_is_method_call (method_call,
158+ DBUS_INTERFACE_UPSTART_INSTANCE,
159+ "Reload"));
160+
161+ TEST_EQ_STR (dbus_message_get_path (method_call),
162+ DBUS_PATH_UPSTART "/jobs/test/_");
163+
164+ TEST_TRUE (dbus_message_get_args (method_call, NULL,
165+ DBUS_TYPE_INVALID));
166+
167+ TEST_ALLOC_SAFE {
168+ reply = dbus_message_new_error (method_call,
169+ DBUS_ERROR_UNKNOWN_METHOD,
170+ "Unknown method");
171+ }
172+
173+ dbus_connection_send (server_conn, reply, NULL);
174+ dbus_connection_flush (server_conn);
175+
176+ dbus_message_unref (method_call);
177+ dbus_message_unref (reply);
178+
179+ /* Expect the Get call for the processes, reply with
180+ * a main process pid.
181+ */
182+ TEST_DBUS_MESSAGE (server_conn, method_call);
183+
184+ TEST_TRUE (dbus_message_is_method_call (method_call,
185+ DBUS_INTERFACE_PROPERTIES,
186+ "Get"));
187+ TEST_EQ_STR (dbus_message_get_path (method_call),
188+ DBUS_PATH_UPSTART "/jobs/test/_");
189+
190+ TEST_TRUE (dbus_message_get_args (method_call, NULL,
191+ DBUS_TYPE_STRING, &interface,
192+ DBUS_TYPE_STRING, &property,
193+ DBUS_TYPE_INVALID));
194+
195+ TEST_ALLOC_SAFE {
196+ reply = dbus_message_new_method_return (method_call);
197+
198+ dbus_message_iter_init_append (reply, &iter);
199+
200+ dbus_message_iter_open_container (&iter, DBUS_TYPE_VARIANT,
201+ (DBUS_TYPE_ARRAY_AS_STRING
202+ DBUS_STRUCT_BEGIN_CHAR_AS_STRING
203+ DBUS_TYPE_STRING_AS_STRING
204+ DBUS_TYPE_INT32_AS_STRING
205+ DBUS_STRUCT_END_CHAR_AS_STRING),
206+ &subiter);
207+
208+ dbus_message_iter_open_container (&subiter, DBUS_TYPE_ARRAY,
209+ (DBUS_STRUCT_BEGIN_CHAR_AS_STRING
210+ DBUS_TYPE_STRING_AS_STRING
211+ DBUS_TYPE_INT32_AS_STRING
212+ DBUS_STRUCT_END_CHAR_AS_STRING),
213+ &arrayiter);
214+
215+ dbus_message_iter_open_container (&arrayiter, DBUS_TYPE_STRUCT,
216+ NULL,
217+ &structiter);
218+
219+ str_value = "main";
220+ dbus_message_iter_append_basic (&structiter, DBUS_TYPE_STRING,
221+ &str_value);
222+
223+ int32_value = proc_pid;
224+ dbus_message_iter_append_basic (&structiter, DBUS_TYPE_INT32,
225+ &int32_value);
226+
227+ dbus_message_iter_close_container (&arrayiter, &structiter);
228+
229+ dbus_message_iter_close_container (&subiter, &arrayiter);
230+
231+ dbus_message_iter_close_container (&iter, &subiter);
232+ }
233+
234+ dbus_connection_send (server_conn, reply, NULL);
235+ dbus_connection_flush (server_conn);
236+
237+ dbus_message_unref (method_call);
238+ dbus_message_unref (reply);
239+
240+ TEST_DBUS_CLOSE (server_conn);
241+
242+ dbus_shutdown ();
243+
244+ exit (0);
245+ }
246+
247+ memset (&command, 0, sizeof command);
248+
249+ args[0] = "test";
250+ args[1] = NULL;
251+
252+ TEST_DIVERT_STDOUT (output) {
253+ TEST_DIVERT_STDERR (errors) {
254+ ret = reload_action (&command, args);
255+ }
256+ }
257+ rewind (output);
258+ rewind (errors);
259+
260+ if (test_alloc_failed
261+ && (ret != 0)) {
262+ TEST_FILE_END (output);
263+ TEST_FILE_RESET (output);
264+
265+ TEST_FILE_EQ (errors, "test: Cannot allocate memory\n");
266+ TEST_FILE_END (errors);
267+ TEST_FILE_RESET (errors);
268+
269+ kill (server_pid, SIGTERM);
270+ waitpid (server_pid, NULL, 0);
271+ continue;
272+ }
273+
274+ TEST_EQ (ret, 0);
275+
276+ TEST_FILE_END (output);
277+ TEST_FILE_RESET (output);
278+
279+ TEST_FILE_END (errors);
280+ TEST_FILE_RESET (errors);
281+
282+ waitpid (server_pid, &status, 0);
283+ TEST_TRUE (WIFEXITED (status));
284+ TEST_EQ (WEXITSTATUS (status), 0);
285+
286+ kill (proc_pid, SIGTERM);
287+ waitpid (proc_pid, NULL, 0);
288+ continue;
289+
290+ }
291+
292+
293 /* Check that a missing argument results in an error being output
294 * to stderr along with a suggestion of help.
295 */

Subscribers

People subscribed via source and target branches