Bug 1443187 - Global build default resource requests do not override empty resource config in bc
Summary: Global build default resource requests do not override empty resource config ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: Build
Version: 3.5.0
Hardware: x86_64
OS: Linux
unspecified
high
Target Milestone: ---
: ---
Assignee: Ben Parees
QA Contact: Mike Fiedler
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-04-18 17:48 UTC by Mike Fiedler
Modified: 2021-04-28 09:36 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Cause: When resources were specified in the build default configuration, the resource values were not applied to the build pods, they were only applied to the build object. Consequence: Builds ran without the default resource limits being applied to them because the pod was created before the build was updated with the default resource limits. Fix: The build resource defaults will be applied to the build pod. Result: Build pods will have the default resource limits applied, if they do not already specify resource limits.
Clone Of:
Environment:
Last Closed: 2017-08-10 05:20:02 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
Build config (1.26 KB, text/plain)
2017-04-18 17:49 UTC, Mike Fiedler
no flags Details
Master logs (120.84 KB, application/x-gzip)
2017-04-19 13:27 UTC, Mike Fiedler
no flags Details
Master at loglevel 5 showing BuildDefaultsConfig (1.55 MB, text/plain)
2017-04-19 14:41 UTC, Mike Fiedler
no flags Details
New master logs with rate limiting disabled (10.99 MB, application/x-gzip)
2017-04-19 15:11 UTC, Mike Fiedler
no flags Details
Master Config (7.13 KB, text/plain)
2017-05-04 03:23 UTC, XiuJuan Wang
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHEA-2017:1716 0 normal SHIPPED_LIVE Red Hat OpenShift Container Platform 3.6 RPM Release Advisory 2017-08-10 09:02:50 UTC

Description Mike Fiedler 2017-04-18 17:48:28 UTC
Description of problem:

When a buildconfig is created from a template, it is created with a resource request/limit of:

  resources: {}

Trying to override this in the global build default section of master-config.yaml with the following (and restarting all masters):

    BuildDefaults:
      configuration:
        apiVersion: v1
        env: []
        kind: BuildDefaultsConfig
        nodeSelector:
          builder: "yes"
          region: "primary"
        resources:
          limits: {}
          requests:
            cpu: "150m"
            memory: "256Mi"


does not work.   When the build runs, the nodeSelector is honored but the resource request is not honored.   The build pod gets a CPU request of 0 and a memory request of 0.

If the request values are specified in the build config, then they are honored and the build pod gets the appropriate request values.

Full bc.yaml and master-config.yaml attached.



Version-Release number of selected component (if applicable): 3.5.5


How reproducible: Always


Steps to Reproduce:
1. Create a new project and a new app from cakephp-mysql-example
2. Edit master-config.yaml on all masters and set build defaults for resources to:
        resources:
          limits: {}
          requests:
            cpu: "150m"
            memory: "256Mi"
3. Restart all masters
4. Start a new build of the build config
5. oc get pods -o wide to display the build node
6. oc describe node <node> to view the requests for the build pod

Actual results:

Requests for the build pod are 0.

If the requests are added to the bc, they will be honored.

Expected results:

Build defaults requests override empty or absent bc requests.

Additional info:

Comment 1 Mike Fiedler 2017-04-18 17:49:05 UTC
Created attachment 1272390 [details]
Build config

Comment 3 Ben Parees 2017-04-19 12:27:31 UTC
can you provide master logs w/ loglevel 5 enabled?

in particular we're looking for this output:
"Initialized build defaults plugin with config <config>"
"Setting default resource limit"
"Setting default resource request"

Comment 4 Mike Fiedler 2017-04-19 13:27:09 UTC
Created attachment 1272617 [details]
Master logs

- set master-controllers and master-api loglevel to 5
- restarted all 3 masters
- collected logs

I don't see the messages you are looking for though.   My build nodeSelector and toleration annotation (added since original master-config attached) are being honored, though.

Comment 5 Mike Fiedler 2017-04-19 14:27:55 UTC
@vlaad took a look at my setup and we compared logfiles he collected at loglevel 5 with mine.   Mine are definitely missing the JSON snipped with the build config.   The only difference we can see is that he had a single master (combined api/controller) whereas mine are split services on 3 masters.

Comment 6 Ben Parees 2017-04-19 14:30:43 UTC
I see this in your log:
Apr 19 09:14:43 svt-m-3 journal: Suppressed 1681 messages from /system.slice/atomic-openshift-master-controllers.service

is it possible systemd is throwing away log messages on us?  Can you log direct to disk instead?

Comment 7 Mike Fiedler 2017-04-19 14:41:17 UTC
I was able to capture a log with the messages.   Attaching.

Apr 19 10:33:55 svt-m-1 atomic-openshift-master-controllers: I0419 10:33:55.828105   11798 admission.go:29] Initialized build defaults plugin with config: api.BuildDefaultsConfig{TypeMeta:unversioned.TypeMeta{Kind:"", APIVersion:""}, GitHTTPProxy:"", GitHTTPSProxy:"", GitNoProxy:"", Env:[]api.EnvVar(nil), SourceStrategyDefaults:(*api.SourceStrategyDefaultsConfig)(nil), ImageLabels:[]api.ImageLabel(nil), NodeSelector:map[string]string{"builder":"yes", "region":"primary"}, Annotations:map[string]string{"scheduler.alpha.kubernetes.io/tolerations":"[{\"key\":\"builder\", \"value\":\"yes\"}]"}, Resources:api.ResourceRequirements{Limits:api.ResourceList(nil), Requests:api.ResourceList{"memory":resource.Quantity{i:resource.int64Amount{value:268435456, scale:0}, d:resource.infDecAmount{Dec:(*inf.Dec)(nil)}, s:"", Format:"BinarySI"}, "cpu":resource.Quantity{i:resource.int64Amount{value:150, scale:-3}, d:resource.infDecAmount{Dec:(*inf.Dec)(nil)}, s:"150m", Format:"DecimalSI"}}}}

Comment 8 Mike Fiedler 2017-04-19 14:41:53 UTC
Created attachment 1272652 [details]
Master at loglevel 5 showing BuildDefaultsConfig

Comment 9 Mike Fiedler 2017-04-19 15:11:06 UTC
Created attachment 1272665 [details]
New master logs with rate limiting disabled

Fresh logs with rsyslog and journald rate limiting disabled.   master3 is the controller and has the BuildDefaultsConfig messages.  I do not see "applying defaults" in any of the logs

Comment 11 Ben Parees 2017-04-19 21:35:39 UTC
thanks I think i see what is happening here.  we are applying the defaults to the build object but we need to also apply them to the pod object at this point in the flow.

Comment 12 Ben Parees 2017-04-19 23:27:21 UTC
PR: https://github.com/openshift/origin/pull/13825

Comment 13 XiuJuan Wang 2017-05-03 07:29:27 UTC
Global build default resources still don't override empty resource config in bc

Tried in below envs, both don't work.
Server https://host-8-174-95.host.centralci.eng.rdu2.redhat.com:8443
openshift v3.6.62
kubernetes v1.6.1+5115d708d7

and openshift origin env devenv-rhel7-6193

Comment 14 Ben Parees 2017-05-03 18:27:28 UTC
> Global build default resources still don't override empty resource config in bc


how did you validate it?  The global build default resource values will not be seen in the BC, nor in the build object.  The only way to confirm they were applied is to look at the resource constraints of the build pod itself, or look at build json definition in the BUILD env variable defined on the build pod.

Comment 15 Mike Fiedler 2017-05-03 22:35:46 UTC
One way to verify this is to trigger a build (or a bunch of builds) and then oc describe the node the build pod is running on.   That should show the cpu and memory requests for the build pod.    You can assign this bz to me for QA if you wish.

Comment 16 XiuJuan Wang 2017-05-04 02:32:47 UTC
I had checked the build pod using
#oc  get po build-pod -o yaml  | grep -i resource  -A 3
The result please see [1]
resources field in the BUILD env variable is empty.

[1] http://pastebin.test.redhat.com/481015

Comment 17 XiuJuan Wang 2017-05-04 02:41:00 UTC
@Mike 
So sweet,thank for your support.
I will assign this to you. Also I will keep an eye on this in case you are busy.

Comment 18 Ben Parees 2017-05-04 02:48:47 UTC
can you provide the full content of your master-config.yaml?  

alternatively can you add some global build default env variables to your config, so we can confirm that the build defaults config is otherwise being picked up and applied?  (again you won't see the build env variables on the build, but you will see them in the build definition within the BUILD env of the build pod)

level 5 logs from the master would also be useful.

Comment 19 Ben Parees 2017-05-04 02:49:59 UTC
XiuJuan, i'd still like to understand why it does not appear to be working in your environment so we can sort out any test configuration issues or doc problems, please see my comment 18 above.

Comment 20 XiuJuan Wang 2017-05-04 03:23:39 UTC
Created attachment 1276092 [details]
Master Config

Comment 21 XiuJuan Wang 2017-05-04 03:25:46 UTC
I set a env TEST=new in the build default config. The env has set in the BUILD env of the build pod, you can refer the attachment.

Comment 22 Ben Parees 2017-05-04 05:28:07 UTC
it looks like there may have been a regression in this area since my other fix, the new issue is described here:
https://github.com/openshift/origin/issues/14039

i'll work on a fix.

Comment 23 Ben Parees 2017-05-04 05:31:14 UTC
PR is here
https://github.com/openshift/origin/pull/14040

Comment 25 XiuJuan Wang 2017-05-31 06:14:46 UTC
Can't reproduce this bug at 

Server https://ip-172-18-6-64.ec2.internal:8443
openshift v3.6.74
kubernetes v1.6.1+5115d708d7

Global build default resources override empty resource config in bc

Comment 29 errata-xmlrpc 2017-08-10 05:20:02 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2017:1716


Note You need to log in before you can comment on or make changes to this bug.