feat(defaults): Add default resources to engine/executor container #2514

groszewn · 2020-10-01T22:00:54Z

Defines default requests and limits on the engine/executor container.

Contributes to #2475

Signed-off-by: Nick Groszewski groszewn@gmail.com

What this PR does / why we need it:

Addition of default engine/executor container resources, which helps in clusters with OPA policies defined to require requests and limits be set.

Which issue(s) this PR fixes:

Fixes #2475

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Added default resources to engine/executor container

seldondev · 2020-10-01T22:00:59Z

Hi @groszewn. Thanks for your PR.

I'm waiting for a SeldonIO member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository.

ukclivecox · 2020-10-02T06:54:05Z

@groszewn I'm just wondering if we should be more conservative on the CPU as a value of 1 implies continuous usage in a cluster. Given we are not serverless I am wondering if this could cause issues to existing users that have over-commits for their cluster?

groszewn · 2020-10-02T12:23:24Z

@cliveseldon I think that's a very valid concern. Would it make sense for this value to be configurable via the helm chart install so that cluster admins can specify what meets their needs best and have a lower default (maybe 0.5 or lower)?

ukclivecox · 2020-10-02T12:41:20Z

@groszewn Yes I think that makes sense
Adding things to the helm chart values that appear as env vars is a bit fiddly at present.
You need to

add the envvar to manager kustomize yaml

modify

seldon-core/operator/helm/split_resources.py

Lines 33 to 59 in 490651c

    
           HELM_ENV_SUBST = { 
        
               "AMBASSADOR_ENABLED": "ambassador.enabled", 
        
               "AMBASSADOR_SINGLE_NAMESPACE": "ambassador.singleNamespace", 
        
               "ENGINE_SERVER_GRPC_PORT": "engine.grpc.port", 
        
               "ENGINE_CONTAINER_IMAGE_PULL_POLICY": "engine.image.pullPolicy", 
        
               "ENGINE_LOG_MESSAGES_EXTERNALLY": "engine.logMessagesExternally", 
        
               "ENGINE_SERVER_PORT": "engine.port", 
        
               "ENGINE_PROMETHEUS_PATH": "engine.prometheus.path", 
        
               "ENGINE_CONTAINER_USER": "engine.user", 
        
               "ENGINE_CONTAINER_SERVICE_ACCOUNT_NAME": "engine.serviceAccount.name", 
        
               "ISTIO_ENABLED": "istio.enabled", 
        
               "ISTIO_GATEWAY": "istio.gateway", 
        
               "ISTIO_TLS_MODE": "istio.tlsMode", 
        
               "PREDICTIVE_UNIT_SERVICE_PORT": "predictiveUnit.port", 
        
               "PREDICTIVE_UNIT_DEFAULT_ENV_SECRET_REF_NAME": "predictiveUnit.defaultEnvSecretRefName", 
        
               "PREDICTIVE_UNIT_METRICS_PORT_NAME": "predictiveUnit.metricsPortName", 
        
               "USE_EXECUTOR": "executor.enabled", 
        
               "EXECUTOR_CONTAINER_IMAGE_PULL_POLICY": "executor.image.pullPolicy", 
        
               "EXECUTOR_SERVER_PORT": "executor.port", 
        
               "EXECUTOR_SERVER_METRICS_PORT_NAME": "executor.metricsPortName", 
        
               "EXECUTOR_PROMETHEUS_PATH": "executor.prometheus.path", 
        
               "EXECUTOR_CONTAINER_USER": "executor.user", 
        
               "EXECUTOR_CONTAINER_SERVICE_ACCOUNT_NAME": "executor.serviceAccount.name", 
        
               "MANAGER_CREATE_RESOURCES": "managerCreateResources", 
        
               "EXECUTOR_REQUEST_LOGGER_DEFAULT_ENDPOINT": "executor.requestLogger.defaultEndpoint", 
        
               "DEFAULT_USER_ID": "defaultUserID", 
        
           }

to add new mapping

regenerate the helm chart templates with

seldon-core/operator/helm/Makefile

Line 6 in 490651c

create:
add new var to values.yaml for operator helm chart
modify operator code to use envvar

groszewn · 2020-10-05T11:48:35Z

@cliveseldon updated to make the default resources configurable via the helm chart install.

ukclivecox · 2020-10-05T12:47:25Z

/ok-to-test

seldondev · 2020-10-05T12:49:35Z

Mon Oct 5 12:49:33 UTC 2020
The logs for [pr-build] [1] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/1.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=1

seldondev · 2020-10-05T12:49:36Z

Mon Oct 5 12:49:35 UTC 2020
The logs for [lint] [2] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/2.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=2

groszewn · 2020-10-06T12:54:58Z

@cliveseldon do the test need to be rekicked?

ukclivecox · 2020-10-06T13:07:10Z

/test integration

seldondev · 2020-10-06T13:09:49Z

Tue Oct 6 13:09:43 UTC 2020
The logs for [integration] [3] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/3.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=3

groszewn · 2020-10-07T12:31:53Z

/retest

seldondev · 2020-10-07T12:34:18Z

Wed Oct 7 12:34:13 UTC 2020
The logs for [integration] [4] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/4.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=4

seldondev · 2020-10-07T13:16:08Z

@groszewn: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
integration	`98c4d11`	link	`/test integration`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository. I understand the commands that are listed here.

seldondev · 2020-10-09T13:06:18Z

Fri Oct 9 13:06:16 UTC 2020
The logs for [pr-build] [5] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/5.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=5

seldondev · 2020-10-09T13:06:53Z

Fri Oct 9 13:06:50 UTC 2020
The logs for [lint] [6] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/6.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=6

groszewn · 2020-10-13T16:28:13Z

@cliveseldon seems like the lint stage is hanging. Does this need to be kicked in some way?

ukclivecox · 2020-10-13T16:43:33Z

/retest

axsaucedo · 2020-10-13T16:52:28Z

/test lint

seldondev · 2020-10-13T16:53:57Z

Tue Oct 13 16:53:56 UTC 2020
The logs for [lint] [7] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/7.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=7

groszewn · 2020-10-14T13:58:29Z

@axsaucedo looks like PR checks have passed, do you need to kick the longer-running tests (I'm not sure if I have this capability and don't want to overstep and strain the build system).

Defines default requests and limits on the engine/executor container. Contributes to 2475 Signed-off-by: Nick Groszewski <groszewn@gmail.com>

seldondev · 2020-10-15T14:26:11Z

Thu Oct 15 14:26:10 UTC 2020
The logs for [lint] [9] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/9.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=9

seldondev · 2020-10-15T14:26:20Z

Thu Oct 15 14:26:19 UTC 2020
The logs for [pr-build] [8] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2514/8.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2514 --build=8

axsaucedo · 2020-10-15T15:41:15Z

/approve

axsaucedo · 2020-10-15T15:41:44Z

Thank you @groszewn !

seldondev · 2020-10-15T15:41:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: axsaucedo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [axsaucedo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

seldondev added size/S needs-ok-to-test labels Oct 1, 2020

groszewn force-pushed the default_resources branch from acd341d to d5ff6c7 Compare October 3, 2020 02:53

seldondev added size/XXL and removed size/S labels Oct 3, 2020

groszewn force-pushed the default_resources branch from d5ff6c7 to 98c4d11 Compare October 3, 2020 02:57

seldondev added size/L and removed size/XXL labels Oct 3, 2020

seldondev added ok-to-test and removed needs-ok-to-test labels Oct 5, 2020

groszewn force-pushed the default_resources branch from 98c4d11 to 3e6fe28 Compare October 9, 2020 13:04

feat(defaults): Add default resources to engine container

089181a

Defines default requests and limits on the engine/executor container. Contributes to 2475 Signed-off-by: Nick Groszewski <groszewn@gmail.com>

groszewn force-pushed the default_resources branch from 3e6fe28 to 089181a Compare October 15, 2020 14:24

axsaucedo requested review from ukclivecox and axsaucedo October 15, 2020 15:37

axsaucedo merged commit 3b042a4 into SeldonIO:master Oct 15, 2020

seldondev added the approved label Oct 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(defaults): Add default resources to engine/executor container #2514

feat(defaults): Add default resources to engine/executor container #2514

groszewn commented Oct 1, 2020

seldondev commented Oct 1, 2020

ukclivecox commented Oct 2, 2020

groszewn commented Oct 2, 2020

ukclivecox commented Oct 2, 2020

groszewn commented Oct 5, 2020

ukclivecox commented Oct 5, 2020

seldondev commented Oct 5, 2020

seldondev commented Oct 5, 2020

groszewn commented Oct 6, 2020

ukclivecox commented Oct 6, 2020

seldondev commented Oct 6, 2020

groszewn commented Oct 7, 2020

seldondev commented Oct 7, 2020

seldondev commented Oct 7, 2020

seldondev commented Oct 9, 2020

seldondev commented Oct 9, 2020

groszewn commented Oct 13, 2020

ukclivecox commented Oct 13, 2020

axsaucedo commented Oct 13, 2020

seldondev commented Oct 13, 2020

groszewn commented Oct 14, 2020

seldondev commented Oct 15, 2020

seldondev commented Oct 15, 2020

axsaucedo commented Oct 15, 2020

axsaucedo commented Oct 15, 2020

seldondev commented Oct 15, 2020

feat(defaults): Add default resources to engine/executor container #2514

feat(defaults): Add default resources to engine/executor container #2514

Conversation

groszewn commented Oct 1, 2020

seldondev commented Oct 1, 2020

ukclivecox commented Oct 2, 2020

groszewn commented Oct 2, 2020

ukclivecox commented Oct 2, 2020

groszewn commented Oct 5, 2020

ukclivecox commented Oct 5, 2020

seldondev commented Oct 5, 2020

seldondev commented Oct 5, 2020

groszewn commented Oct 6, 2020

ukclivecox commented Oct 6, 2020

seldondev commented Oct 6, 2020

groszewn commented Oct 7, 2020

seldondev commented Oct 7, 2020

seldondev commented Oct 7, 2020

seldondev commented Oct 9, 2020

seldondev commented Oct 9, 2020

groszewn commented Oct 13, 2020

ukclivecox commented Oct 13, 2020

axsaucedo commented Oct 13, 2020

seldondev commented Oct 13, 2020

groszewn commented Oct 14, 2020

seldondev commented Oct 15, 2020

seldondev commented Oct 15, 2020

axsaucedo commented Oct 15, 2020

axsaucedo commented Oct 15, 2020

seldondev commented Oct 15, 2020