Skip to content
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

[JENKINS-50664] container cap limit hit due to default labels missing #322

Closed
wants to merge 1 commit into from

Conversation

hrishin
Copy link

@hrishin hrishin commented May 14, 2018

Issue : Setting container cap limit configuration doesn't work correctly without Jenkins restart. The root cause of the issue is label management for slave pods. Once user set the container cap limit configuration for given namespace. Plugin fails to account slave pods/containers based using pods labels. It accounts for all pods/containers from the namespace which are not started by the Kubernetes plugin.
This patch fixes this issue using default slave labels "jenkins"->"slave"

  1. By adding default slave lables to pods/containers
  2. By quering pods/containers with default slave labels

Fixes https://issues.jenkins-ci.org/browse/JENKINS-51286

@hrishin
Copy link
Author

hrishin commented May 16, 2018

@carlossg @iocanel could you please review it?

@piyush-garg
Copy link

@carlossg @iocanel Can you please take a look at this?

@carlossg carlossg changed the title Fix for JENKINS-51286: container cap limit [JENKINS-50664] container cap limit hit due to default labels missing May 22, 2018
@carlossg
Copy link
Contributor

can you check if #325 and #326 fix your issues ?

… missing

Issue : Setting container cap limit configuration doesn't work correctly without Jenkins restart. The root cause of the issue is label management for slave pods. Once user set the container cap limit configuration for given namespace. Plugin fails to account slave pods/containers based using pods labels. It accounts all pods/containers from the namespace which are not started by the Kubernetes plugin.
This patch fixes this issue using default slave labels "jenkins"->"slave"
1) By adding defualt slave lables to pods/containers
2) By quering pods/containers with default slave labels

- added default lables to PodTemplate
- fixed getLables for querying container cap in KubernetesCloud
- updated test cases for PodtemplateBuilder
@hrishin
Copy link
Author

hrishin commented May 23, 2018

@carlossg thanks for updates.

#325 is solving the problem partially. It doesn't add the default labels using PodTemplate. Hence #326 test case is failing.

I have updated the test case and fixed one more issue regarding merging pod labels.

@carlossg
Copy link
Contributor

#325 should fix this

@carlossg carlossg closed this Jun 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants