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

ENGINE_CONTAINER_SERVICE_ACCOUNT_NAME and EXECUTOR_CONTAINER_SERVICE_ACCOUNT_NAME is picking a default value #1508

Closed
RudraprakashR opened this issue Mar 5, 2020 · 14 comments

Comments

@RudraprakashR
Copy link

RudraprakashR commented Mar 5, 2020

Hi,
I have installed kubeflow on-prem and trying to run iris seldon serving example.

In my cluster containers running on root mode is not allowed by default, hence differnt psp/service is need to be paased if pod is running with container with root .

Due to this behvaiour, when i run serving, I am getting an error as "Error: container has runAsNonRoot and image will run as root"
I have tried to solve this issue by setting ENGINE_CONTAINER_SERVICE_ACCOUNT_NAME & EXECUTOR_CONTAINER_SERVICE_ACCOUNT_NAME to differnet service account which allows root container, but always default is taken in serving container.

Even i tried look in to the code, code seems correct
https://github.com/SeldonIO/seldon-core/blob/v1.0.2/operator/controllers/seldondeployment_engine.go#L269-L273

can you pls check whether I am checking wrong modules?

here is my seldon.yaml template(even i tried setting serviceAccountName in SeldonDeployment)

[root@bcmt-vm-cnfi-2873-control-02 rudra]# cat seldon.yaml
apiVersion: machinelearning.seldon.io/v1alpha2
kind: SeldonDeployment
metadata:
  name: xgboost
spec:
  name: iris
  predictors:
  - graph:
      children: []
      implementation: XGBOOST_SERVER
      serviceAccountName: default-editor
      modelUri: pvc://xgboostlocal
      name: classifier
    name: default
    replicas: 1
@ukclivecox
Copy link
Contributor

Would you not need to set engine.user if you want the engine container to run as nonRoot.

See https://docs.seldon.io/projects/seldon-core/en/latest/reference/helm.html#helm-chart-configuration

@RudraprakashR
Copy link
Author

even that is set to 8888(default provided in kustomization)
ENGINE_CONTAINER_USER: 8888

@RudraprakashR
Copy link
Author

To be precise, issue is coming for init Containers(issue should have been solved if serviceAccountName is set to serviceaccount name which i had passed)

classifier-model-initializer:
    Container ID:
    Image:         gcr.io/kfserving/storage-initializer:0.2.1
    Image ID:
    Port:          <none>
    Host Port:     <none>
    Args:
      /mnt/pvc/
      /mnt/models
    State:          Waiting
      Reason:       CreateContainerConfigError

@ryandawsonuk
Copy link
Contributor

Looks like we only allow for runAsUser to get set on the engine/executor container and not on the initContainer. This is where it's set for the executor:

To support this we'd also need to set it on the initContainer:

initContainer := &corev1.Container{

I guess the same environment variable could be used, assuming you want the engine/executor and initContainer to have the same runAsUser. Otherwise we'd need to add a new env var specifically for the initContainer.

@RudraprakashR would you be interested in submitting a pull request for this?

@RudraprakashR
Copy link
Author

@ryandawsonuk Thanks for reply. I would like to submit but before that I wanted to understand why serviceAccountName is coming as default instead of the name i specified in env variables(env variables mentioned above).

@ryandawsonuk
Copy link
Contributor

ryandawsonuk commented Mar 5, 2020

The serviceAccountName specified at the graph level is specifically for the initContainer. There's an example of how to set that at https://docs.seldon.io/projects/seldon-core/en/latest/servers/overview.html

That actually doesn't attach the serviceAccount directly to the Pod. Instead it iterates through Secrets for the ServiceAccount and attaches those.

The ENGINE_CONTAINER_SERVICE_ACCOUNT_NAME/EXECUTOR_CONTAINER_SERVICE_ACCOUNT_NAME is for the Pod. That is set on the seldon core installation.

Do you mean that you've set the ENGINE_CONTAINER_SERVICE_ACCOUNT_NAME on the seldon core operator installation via its helm chart and you're not seeing the serviceAccount on the Pods for the SeldonDeployments?

@RudraprakashR
Copy link
Author

Do you mean that you've set the ENGINE_CONTAINER_SERVICE_ACCOUNT_NAME on the seldon core operator installation via its helm chart and you're not seeing the serviceAccount on the Pods for the SeldonDeployments?

yes, i have set in operator but not seeing on SeldonDeployments pods..
I am always seeing default

@ryandawsonuk
Copy link
Contributor

Hmm, I can see the code that looks that service account name up and where it is called to set it on the Pod. I guess it would be necessary to try it with latest and master and if that's not working then debug/log what is happening at those points in the code.

@axsaucedo axsaucedo added this to the 1.2 milestone Mar 15, 2020
@ukclivecox ukclivecox removed this from the 1.2 milestone Apr 23, 2020
@ukclivecox
Copy link
Contributor

Can you check in 1.1.0 where the runAsUser if set is provided at the Pod level so call conatiners will run as this user.

@Subreptivus
Copy link

@cliveseldon I don't think that this issue is solved. It's clear that you're talking about different things.
@RudraprakashR was talking about ServiceAccount, not about user from which container should be running.
runAsUser is working fine with v1.1.0, thank you very much for that, because I need that too. But I need to set ServiceAccount as well, rather then using default. I've checked places that @ryandawsonuk indicated, and it looks like it should be working fine...
I've set ENGINE_CONTAINER_SERVICE_ACCOUNT_NAME and EXECUTOR_CONTAINER_SERVICE_ACCOUNT_NAME on the operator, yet it still hasn't been passed to Deployment/Pod level.

@MarshHawk
Copy link
Contributor

... I've set ENGINE_CONTAINER_SERVICE_ACCOUNT_NAME and EXECUTOR_CONTAINER_SERVICE_ACCOUNT_NAME on the operator, yet it still hasn't been passed to Deployment/Pod level.

Is there a solution for this issue?

@ukclivecox ukclivecox reopened this Feb 9, 2022
@ukclivecox
Copy link
Contributor

Here is the code reference

@MarshHawk
Copy link
Contributor

Ok, looking at the code chain it shows that getSvcOrchSvcAccountName is only called when seldon.io/engine-separate-pod: "true" annotation is set correctly in sdep... I don't know if that is intentional or not... Though, it seems it might be because EXECUTOR_CONTAINER_SERVICE_ACCOUNT_NAME is now the lone variable for setting the default service account(?)... verified with version 1.12.0-dev

@ukclivecox
Copy link
Contributor

You can add your own serviceAccountName to the PodSpec.
e.g.

apiVersion: machinelearning.seldon.io/v1
kind: SeldonDeployment
metadata:
  name: seldon-model
spec:
  name: test-deployment
  predictors:
  - componentSpecs:
    - spec:
        serviceAccountName: default
        containers:
        - image: seldonio/mock_classifier_rest:1.3
          name: classifier
          resources:
            requests:
              memory: 1Gi
              cpu: 1
            limits:
              memory: 1Gi
              cpu: 2
    graph:
      children: []
      endpoint:
        type: REST
      name: classifier
      type: MODEL
    name: example
    replicas: 1

If you have 1 componentSpec the executor will be added as a container to it. This is why the service account name setting is only used when the executor is placed in a separate Deployment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants