-
Notifications
You must be signed in to change notification settings - Fork 890
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
Added quotes to the Agent command to prevent defaulting to the sleep #768
base: main
Are you sure you want to change the base?
Conversation
…command when the command variable was null. Signed-off-by: papi83dm <[email protected]>
Signed-off-by: papi83dm <[email protected]>
Conflicts: charts/jenkins/CHANGELOG.md Signed-off-by: papi83dm <[email protected]>
Conflicts: charts/jenkins/CHANGELOG.md charts/jenkins/Chart.yaml Signed-off-by: papi83dm <[email protected]>
Signed-off-by: papi83dm <[email protected]>
@timja @torstenwalter How do I submit this PR for review? does it gets submitted automatic ? |
What's the benefit of this? If you are executing nothing the agent will stop before it can execute any CI job which you want to run on it. |
For some reason the Agent for me doesn't run and it dies and and it goes into a loop to create another agent and die. I have to manually go into the pod template in jenkins gui and remove the sleep command. On the job it shows this
Jenkins Logs
k8s event logs
|
Could it be that the image you are using does not have a sleep command? I am not sure what is used as default if this is empty. Just guessing but it could be a |
not sure why the k8s plugins has to default it to sleep when is null |
I am not sure if there was a change in the plugin, but I remember tbe situation where pods being killed if a command was specified which exited right away. It's a bit hard to speculate here without knowing which image you are using and which |
Currently the helm chart doesn’t let you remove the default command when you configure an agent, and this pr does that. Another issue I found is, we limit the amount of concurrence agents that can run and there isn’t an option to limit the instanceCap in the agents |
@torstenwalter this is the image i'm using |
@timja are you aware of any change which defaults to sleep? |
This default seems to exist for quite a while: |
Correct, but shouldn’t a user have the option to override from the helm chart ? |
That's fine with me. I just want to avoid any negative impact where previously the default value was used and it breaks for users if we set it to empty. |
I completely understand, I wonder if this is related to just my eks setup or others are experiencing the same. I tried it with these 4 images and I still have the same behavior.
EKS Version: 1.24 |
What version of Jenkins are you running? those agent images are quite old |
I'm using the latest LTS jenkins/jenkins:2.375.1-lts-alpine |
|
Same error
|
As a workaround I ended up configuring my agents under the agents.podTemplates section as in that part of the code I have full control of the configuration.
|
What this PR does / why we need it
it removes the sleep command from the agent PodTemplate when the command value is null.
By adding quotes to the template, it prevents from adding the sleep command and the command defaults into a blank command.
Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged)Special notes for your reviewer
Checklist