-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[JENKINS-75563] First draft on how to kill a windows container #1724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[JENKINS-75563] First draft on how to kill a windows container #1724
Conversation
null, | ||
"cmd", | ||
"/Q", | ||
"wmic process where \"CommandLine like '%" + launchCmd + "%'\" call terminate") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not tested yet, provided by ChatGPT. But this should kill any process whose commandline contains launchCmd
So IIRC the difficulty is that we cannot use https://javadoc.jenkins.io/hudson/util/ProcessTree.html#killAll(java.util.Map) since the processes to be killed reside in a different container (and thus process namespace) than the agent. |
null, | ||
"cmd", | ||
"/Q", | ||
"wmic process where \"CommandLine like '%" + launchCmd + "%'\" call terminate") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wmic
is not available by default in newer versions of Windows Server I am afraid.
(Anyway you would want by cookie env var, not just doing some fuzzy match on the command line.)
It is easy to test AFAIK: just run a cluster with a Windows node pool and try unignoring #626 (comment) FTR |
I think you just need to retest whether Also |
Oh, I was under the impression I was limited to CMD, will test if powershell has improved, thanks! |
You would just have to check what is actually available in typical container environments. |
I may have some code to share today end of my day. |
I have been able to craft (with a bit of AI help) a draft of a script that seems to work on usual usages, testing is yet very limited but something that works sometimes is better that something that never works until we can find something that always works. Anyway, I have updated the code here in case anyone wants to take a look while I jump into serious testing using #1727. For the moment tested only with a pipeline like this one which now ends successfully:
|
src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java
Outdated
Show resolved
Hide resolved
try { | ||
String remote = copyWindowsKillScript(workspace).getRemote(); | ||
exitCode = doLaunch( // Will fail if the script is not present, but it was also failing before in all cases | ||
false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For development, may move to true
or introduce a system property in the future
...rces/org/csanchez/jenkins/plugins/kubernetes/pipeline/scripts/kill-processes-with-cookie.ps1
Outdated
Show resolved
Hide resolved
I am not sure what that pipeline proves. The version I saw from a plugin user was doing a rename in the |
...rces/org/csanchez/jenkins/plugins/kubernetes/pipeline/scripts/kill-processes-with-cookie.ps1
Outdated
Show resolved
Hide resolved
Yeah, same with this one, the rename stage fails as the |
try { | ||
$envBlock = [ProcessEnvironmentReader]::ReadEnvironmentBlock($id) | ||
if ($envBlock.Contains("JENKINS_SERVER_COOKIE=$($cookie)")) { | ||
Write-Host "Killing $($_.ProcessName) (ID: $($_.Id)) - JENKINS_SERVER_COOKIE matches" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I remove this? It is not gonna be visible to the end user but may help on local runsa nd debugging
@@ -18,7 +18,11 @@ spec: | |||
''') { | |||
node(POD_LABEL) { | |||
container('shell') { | |||
powershell 'try {Write-Host starting to sleep; Start-Sleep 999999} finally {Write-Host shut down gracefully}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would only work if a user sends a Ctrl-C to the process, Stop-Process
is a hard kill that inmediately kills the process. I have not found a way to do a "soft kill" in windows via powershell and I honestly do not believe is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it would certainly be better for the plugin to emulate the Ctrl-C-like behavior, providing functional parity with the Linux version. If this is simply impossible, then a hard kill is I guess better than the nothing we have now.
What happens if the pod is terminated? (gracefully, e.g. kubectl delete pod
) Does that run any Powershell finally
code? Tricky to test since you would have to look for some effect outside the pod itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought something similar yesterday while trying to sleep, I have found some examples of C# code that seems to emulate the ctrl+c that I want to try. I will also try to test the pod deletion.
Still not able to deal with confirmation dialogs for cmd for example but succesfully killed a ping in my tests
https://issues.jenkins.io/browse/JENKINS-75563
Trying to validate my approach on how we could kill a windows
container
. The problem I have is that AFAIK there is no way to use CMD in windows to get a result similar tokubernetes-plugin/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java
Lines 675 to 676 in 48ea4aa
JENKINS_SERVER_COOKIE
env variable.Instead what I am trying to do is find the process that was originated by the original command in
kubernetes-plugin/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java
Line 381 in 48ea4aa
container
has its own uniqueLauncher
.Note I have not tested anything yet as I want first to know if this approach makes sense at all or I should go in another direction.
(edited by jglick to use permalinks)