Skip to content

Commit 41d4e4c

Browse files
qiong-zhouqiong-zhou
authored
[Fix-17753][Task-Plugin] Fix workflow killed not success when shell is sh (#17776)
1 parent 4f693a5 commit 41d4e4c

File tree

2 files changed

+71
-40
lines changed
  • dolphinscheduler-task-plugin/dolphinscheduler-task-api/src

2 files changed

+71
-40
lines changed

‎dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtils.java‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ private ProcessUtils() {
9999
*/
100100
private static final Pattern PID_PATTERN = Pattern.compile("\\s+");
101101

102+
private static final String SIGINT = "2";
103+
private static final String SIGTERM = "15";
104+
private static final String SIGKILL = "9";
105+
102106
/**
103107
* Terminate the task process, support multi-level signal processing and fallback strategy
104108
* @param request Task execution context
@@ -116,27 +120,28 @@ public static boolean kill(@NonNull TaskExecutionContext request) {
116120
// Get all child processes
117121
List<Integer> pidList = getPidList(processId);
118122

119-
// 1. Try to terminate gracefully (SIGINT)
120-
boolean gracefulKillSuccess = sendKillSignal("SIGINT", pidList, request.getTenantCode());
123+
// 1. Try to terminate gracefully `kill -2`
124+
boolean gracefulKillSuccess = sendKillSignal(SIGINT, pidList, request.getTenantCode());
121125
if (gracefulKillSuccess) {
122-
log.info("Successfully killed process tree using SIGINT, processId: {}", processId);
126+
log.info("Successfully killed process tree by SIGINT, processId: {}", processId);
123127
return true;
124128
}
125129

126-
// 2. Try to terminate forcefully (SIGTERM)
127-
boolean termKillSuccess = sendKillSignal("SIGTERM", pidList, request.getTenantCode());
130+
// 2. Try to terminate gracefully `kill -15`
131+
boolean termKillSuccess = sendKillSignal(SIGTERM, pidList, request.getTenantCode());
128132
if (termKillSuccess) {
129-
log.info("Successfully killed process tree using SIGTERM, processId: {}", processId);
133+
log.info("Successfully killed process tree by SIGTERM, processId: {}", processId);
130134
return true;
131135
}
132136

133137
// 3. As a last resort, use `kill -9`
134-
log.warn("SIGINT & SIGTERM failed, using SIGKILL as a last resort for processId: {}", processId);
135-
boolean forceKillSuccess = sendKillSignal("SIGKILL", pidList, request.getTenantCode());
138+
log.warn("Killing process by SIGINT & SIGTERM failed, using SIGKILL as a last resort for processId: {}",
139+
processId);
140+
boolean forceKillSuccess = sendKillSignal(SIGKILL, pidList, request.getTenantCode());
136141
if (forceKillSuccess) {
137-
log.info("Successfully sent SIGKILL signal to process tree, processId: {}", processId);
142+
log.info("Successfully killed process tree by SIGKILL, processId: {}", processId);
138143
} else {
139-
log.error("Error sending SIGKILL signal to process tree, processId: {}", processId);
144+
log.error("Error killing process tree by SIGKILL, processId: {}", processId);
140145
}
141146
return forceKillSuccess;
142147

@@ -170,7 +175,7 @@ private static boolean sendKillSignal(String signal, List<Integer> pidList, Stri
170175

171176
try {
172177
// 1. Send the kill signal
173-
String killCmd = String.format("kill -s %s %s", signal, pids);
178+
String killCmd = String.format("kill -%s %s", signal, pids);
174179
killCmd = OSUtils.getSudoCmd(tenantCode, killCmd);
175180
log.info("Sending {} to process group: {}, command: {}", signal, pids, killCmd);
176181
OSUtils.exeCmd(killCmd);

‎dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/test/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtilsTest.java‎

Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,16 @@ void testKillProcessSuccessWithNoAlivePids() {
122122
Mockito.when(taskRequest.getTenantCode()).thenReturn("testTenant");
123123

124124
// Mock getPidsStr
125-
mockedOSUtils.when(() -> OSUtils.exeCmd(Mockito.matches(".*pstree.*12345")))
126-
.thenReturn("sudo(12345)---86.sh(1234)");
125+
String pstreeCmd;
126+
String pstreeOutput;
127+
if (SystemUtils.IS_OS_MAC) {
128+
pstreeCmd = "pstree -sp 12345";
129+
pstreeOutput = "-+= 12345 sudo -+- 1234 86.sh";
130+
} else {
131+
pstreeCmd = "pstree -p 12345";
132+
pstreeOutput = "sudo(12345)---86.sh(1234)";
133+
}
134+
mockedOSUtils.when(() -> OSUtils.exeCmd(pstreeCmd)).thenReturn(pstreeOutput);
127135

128136
// Mock kill -0
129137
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -0.*")))
@@ -138,9 +146,9 @@ void testKillProcessSuccessWithNoAlivePids() {
138146
Assertions.assertTrue(result);
139147

140148
// Verify SIGINT, SIGTERM, SIGKILL never called
141-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGINT 12345"), Mockito.never());
142-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGTERM 12345"), Mockito.never());
143-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGKILL 12345"), Mockito.never());
149+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -2 12345 1234"), Mockito.never());
150+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -15 12345 1234"), Mockito.never());
151+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -9 12345 1234"), Mockito.never());
144152
}
145153

146154
@Test
@@ -151,17 +159,26 @@ void testKillProcessSuccessWithSigInt() {
151159
Mockito.when(taskRequest.getTenantCode()).thenReturn("testTenant");
152160

153161
// Mock getPidsStr
154-
mockedOSUtils.when(() -> OSUtils.exeCmd(Mockito.matches(".*pstree.*12345")))
155-
.thenReturn("sudo(12345)---86.sh(1234)");
162+
String pstreeCmd;
163+
String pstreeOutput;
164+
if (SystemUtils.IS_OS_MAC) {
165+
pstreeCmd = "pstree -sp 12345";
166+
pstreeOutput = "-+= 12345 sudo -+- 1234 86.sh";
167+
} else {
168+
pstreeCmd = "pstree -p 12345";
169+
pstreeOutput = "sudo(12345)---86.sh(1234)";
170+
}
171+
mockedOSUtils.when(() -> OSUtils.exeCmd(pstreeCmd)).thenReturn(pstreeOutput);
156172

157173
// Mock SIGINT command
158-
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -s SIGINT.*")))
159-
.thenReturn("kill -s SIGINT 12345");
160-
mockedOSUtils.when(() -> OSUtils.exeCmd("kill -s SIGINT 12345")).thenReturn("");
174+
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -2.*")))
175+
.thenReturn("kill -2 12345 1234");
176+
mockedOSUtils.when(() -> OSUtils.exeCmd("kill -2 12345 1234")).thenReturn("");
161177

162178
// Mock kill -0
163179
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -0.*")))
164-
.thenReturn("kill -0 12345");
180+
.thenReturn("kill -0 12345")
181+
.thenReturn("kill -0 1234");
165182
// Mock the static method OSUtils.exeCmd that matches "kill -0" command
166183
mockedOSUtils.when(() -> OSUtils.exeCmd(Mockito.matches(".*kill -0.*")))
167184
.thenReturn("") // First invocation succeeds (process is alive)
@@ -176,10 +193,10 @@ void testKillProcessSuccessWithSigInt() {
176193
Assertions.assertTrue(result);
177194

178195
// Verify SIGINT was called
179-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGINT 12345"), Mockito.times(1));
196+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -2 12345 1234"), Mockito.times(1));
180197
// Verify SIGTERM,SIGKILL was never called
181-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGTERM 12345"), Mockito.never());
182-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGKILL 12345"), Mockito.never());
198+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -15 12345 1234"), Mockito.never());
199+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -9 12345 1234"), Mockito.never());
183200
}
184201

185202
@Test
@@ -190,27 +207,36 @@ void testKillProcessFail() {
190207
Mockito.when(taskRequest.getTenantCode()).thenReturn("testTenant");
191208

192209
// Mock getPidsStr
193-
mockedOSUtils.when(() -> OSUtils.exeCmd(Mockito.matches(".*pstree.*12345")))
194-
.thenReturn("sudo(12345)---86.sh(1234)");
210+
String pstreeCmd;
211+
String pstreeOutput;
212+
if (SystemUtils.IS_OS_MAC) {
213+
pstreeCmd = "pstree -sp 12345";
214+
pstreeOutput = "-+= 12345 sudo -+- 1234 86.sh";
215+
} else {
216+
pstreeCmd = "pstree -p 12345";
217+
pstreeOutput = "sudo(12345)---86.sh(1234)";
218+
}
219+
mockedOSUtils.when(() -> OSUtils.exeCmd(pstreeCmd)).thenReturn(pstreeOutput);
195220

196221
// Mock SIGINT command
197-
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -s SIGINT.*")))
198-
.thenReturn("kill -s SIGINT 12345");
199-
mockedOSUtils.when(() -> OSUtils.exeCmd("kill -s SIGINT 12345")).thenReturn("");
222+
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -2.*")))
223+
.thenReturn("kill -2 12345 1234");
224+
mockedOSUtils.when(() -> OSUtils.exeCmd("kill -2 12345 1234")).thenReturn("");
200225

201226
// Mock SIGTERM command
202-
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -s SIGTERM.*")))
203-
.thenReturn("kill -s SIGTERM 12345");
204-
mockedOSUtils.when(() -> OSUtils.exeCmd("kill -s SIGTERM 12345")).thenReturn("");
227+
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -15.*")))
228+
.thenReturn("kill -15 12345 1234");
229+
mockedOSUtils.when(() -> OSUtils.exeCmd("kill -15 12345 1234")).thenReturn("");
205230

206231
// Mock SIGKILL command
207-
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -s SIGKILL.*")))
208-
.thenReturn("kill -s SIGKILL 12345");
209-
mockedOSUtils.when(() -> OSUtils.exeCmd("kill -s SIGKILL 12345")).thenReturn("");
232+
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -9.*")))
233+
.thenReturn("kill -9 12345 1234");
234+
mockedOSUtils.when(() -> OSUtils.exeCmd("kill -9 12345 1234")).thenReturn("");
210235

211236
// Mock kill -0
212237
mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), Mockito.matches("kill -0.*")))
213-
.thenReturn("kill -0 12345");
238+
.thenReturn("kill -0 12345")
239+
.thenReturn("kill -0 1234");
214240
mockedOSUtils.when(() -> OSUtils.exeCmd(Mockito.matches(".*kill -0.*"))).thenReturn("");
215241

216242
// Act
@@ -220,9 +246,9 @@ void testKillProcessFail() {
220246
Assertions.assertFalse(result);
221247

222248
// Verify SIGINT, SIGTERM, SIGKILL was called
223-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGINT 12345"), Mockito.times(1));
224-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGTERM 12345"), Mockito.times(1));
225-
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGKILL 12345"), Mockito.times(1));
249+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -2 12345 1234"), Mockito.times(1));
250+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -15 12345 1234"), Mockito.times(1));
251+
mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -9 12345 1234"), Mockito.times(1));
226252
}
227253

228254
@Test

0 commit comments

Comments
 (0)