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

Deploying seldon models to multiple namespaces #89

Closed
Raab70 opened this issue Feb 13, 2018 · 9 comments
Closed

Deploying seldon models to multiple namespaces #89

Raab70 opened this issue Feb 13, 2018 · 9 comments

Comments

@Raab70
Copy link

Raab70 commented Feb 13, 2018

Currently seldon-core only supports a single prediction model associated with a seldon-core deployment. It would be great to be able to deploy multiple models to a single cluster using namespaces but seldon-core does not recognize models in other namespaces and if I deploy seldon-core to multiple namespaces I get DNS collissions.

Minimal Working Example

Note: This is on GKE

helm install seldon-core --namespace default\
        --name seldon-core \
        --set cluster_manager.rbac=true \
        --set cluster_manager_service_type=LoadBalancer \
        --set apife_service_type=LoadBalancer \
        --repo https://storage.googleapis.com/seldon-charts 
kubectl create namespace ml-deployment
helm install seldon-core --namespace ml-deployment\
        --name seldon-core-namespaced \
        --set cluster_manager.rbac=true \
        --set cluster_manager_service_type=LoadBalancer \
        --set apife_service_type=LoadBalancer \
        --repo https://storage.googleapis.com/seldon-charts 

Behaviour

I've actually seen two different errors, I think it has to do with which node gets the deployment. My guess is that if the node already has the original seldon-core you'll see a port collision such as: Error: release seldon-core-namespaced failed: Service "seldon-apiserver" is invalid: spec.ports[0].nodePort: Invalid value: 30032: provided port is already allocated

The other error I've seen is: Error: release seldon-core-namespaced failed: customresourcedefinitions.apiextensions.k8s.io "seldondeplo yments.machinelearning.seldon.io" already exists

Expected Behaviour

Ideally it would be great if seldon-core supported namespaced API paths out of the box so that multiple models could be deployed to a single cluster. That way I could have a seldondeployment in namespace mymodel and another one in myothermodel and they would both be accessible through the seldon API.

Why not ambassador + ksonnet

I know that one potential solution would be ambassador: https://github.com/SeldonIO/seldon-core/blob/master/notebooks/ksonnet_ambassador_minikube.ipynb

I have evaluated this an alternative and it is not going to be a solution. One issue is the added complexity compared to seldon with helm which is undesirable but not a deal breaker. The deal breaker however is that integrating it into our CI/CD solution is not possible. The whole point is to be able to deploy multiple models meaning that the CI/CD pipeline would have to have access to a central repository or storage for the cluster configuration being modified by ksonnet and it would be way too easy for these to get out of sync, there would have to be some sort of locking which is integrated into the CI/CD pipeline.

@Raab70 Raab70 changed the title Deploying seldon-core to multiple namespaces Deploying seldon mdoels to multiple namespaces Feb 13, 2018
@Raab70 Raab70 changed the title Deploying seldon mdoels to multiple namespaces Deploying seldon models to multiple namespaces Feb 13, 2018
@ukclivecox
Copy link
Contributor

Thanks for the detailed request.

We definitely should fix the port clash and verify multiple namespace deployments works including the correct RBAC settings for the service accounts.

On the issues with ksonnet + ambassador.

  • I think the two could be separated. You could run Ambassador without using ksonnet. We would need to update the Helm chart to allow the APIFE to be optional which should be trivial.
  • You would not need to use ksonnet for the deployment updates you could simply use it to deploy the core components and leave the CI/CD operations to run over the manifests and deploy however you are doing that right now.

Be great to understand what your current CI/CD solution is as we would like to have examples of integrations.

@Raab70
Copy link
Author

Raab70 commented Feb 13, 2018

Right now we are just deploying with a kubectl apply -f deployment.json which a namespace switch.

So I think what you're saying is if you make a switch on the helm chart for an optional APIFE we could helm install seldon-core without the APIFE in a namespace besides default, which should avoid the DNS collisions. Then kubectl apply -f deployment.json --namespace <namespace> And then it should be recognized by ambassador?

@dustinvanbuskirk
Copy link

@cliveseldon We're working together to setup a CI Pipeline using Codefresh CI/CD https://codefresh.io/.

Codefresh has a plugins repository available here: https://github.com/codefresh-io/plugins that can show you some examples of how to use a Codefresh YAML defined pipeline to execute commands against Docker container using a Docker image that is prepared for the integration with a CLI.

For example cf-plugin-helm https://github.com/codefresh-io/cf-plugin-helm is one of the plugins being used on this specific pipeline above.

We have an integration with Kubernetes cluster and pass in the context to the pipeline and we can execute kubectl and helm as part of a build-step and share files in the repository and variables in the pipelines with that step.

Here's some additional documentation on build steps https://docs.codefresh.io/docs/what-is-the-codefresh-yaml

The build step we're held up on currently and hoping to figure out a way around this is the helm install of the seldon-core to support multiple namespaces so we can bring that up in a newly created namespace and not have collisions when another namespace is created by another build.

What are your thoughts on this approach?

@ukclivecox
Copy link
Contributor

@Raab70 Yes - it will work with Ambassador as our operator that creates the svc, pods etc adds Ambassador config in an annotation when it creates the components for a SeldonDeployment, they look like for example:

kubectl get svc mnist-classifier -o yaml

apiVersion: v1
kind: Service
metadata:
  annotations:
    getambassador.io/config: |
      ---
      apiVersion: ambassador/v0
      kind:  Mapping
      name:  seldon_mnist-classifier_rest_mapping
      prefix: /seldon/mnist-classifier/
      service: mnist-classifier:8000
      ---
      apiVersion: ambassador/v0
      kind:  Mapping
      name:  mnist-classifier_grpc_mapping
      grpc: true
      prefix: /seldon.protos.Seldon/
      rewrite: /seldon.protos.Seldon/
      headers:
        seldon: mnist-classifier
      service: mnist-classifier:5001
  creationTimestamp: 2018-02-12T13:57:37Z
  labels:
    seldon-app: mnist-classifier
    seldon-deployment-id: mnist-classifier
  name: mnist-classifier
  namespace: default
 

For Ambassador config and namespaces see https://www.getambassador.io/reference/configuration

@Raab70
Copy link
Author

Raab70 commented Feb 14, 2018

That's great! That may be the solution we end up with but for right now I had trouble configuring it with GKE. Any way we can get an example notebook for that? I wanted to use rbac which was throwing an error (I have rbac configured, probably not a seldon issue, I can dig in) but I also tried using the TLS version of ambassador (https://www.getambassador.io/yaml/ambassador/ambassador-https.yaml) and it was not finding my deployed model.

UPDATE: I have gotten rbac working

@ukclivecox
Copy link
Contributor

Not that #92 is done you can install seldon-core without the APIFE so you can use Ambassador easily, e.g.

helm install ../helm-charts/seldon-core --name seldon-core --set apife.enabled=false

We haven't push a new release to the helm repos yet so you would need to do as above from a clone of seldon-core.

@Raab70
Copy link
Author

Raab70 commented Feb 14, 2018

I tried this but I'm still getting Error: release seldon-core-demomodel failed: customresourcedefinitions.apiextensions.k8s.io "seldondeployments.machinelearning.seldon.io" already exists after using the chart from #92.

My install script:

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
helm install $DIR/helm-charts/seldon-core --name seldon-core \
        --set cluster_manager.rbac=true \
        --set cluster_manager_service_type=LoadBalancer \
        --set apiefe.enabled=false > /dev/null

if [ -z "$(kubectl get namespaces | grep demomodel)" ];then kubectl create namespace demomodel;fi
helm install $DIR/helm-charts/seldon-core --name seldon-core-demomodel --namespace demomodel\
        --set cluster_manager.rbac=true \
        --set cluster_manager_service_type=LoadBalancer \
        --set apife.enabled=false

Am I missing something?

@ukclivecox
Copy link
Contributor

No, I think this is an issue with calling the helm install twice. The CRD needs only to be defined once.

I think the bug is in the CRD definition in that it is not namespaced. I have opened an issue to look at this

#95

@ukclivecox
Copy link
Contributor

This should be fixed now in release 0.1.5

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

3 participants