make ambassador a dependency #480
Conversation
| @@ -174,3 +174,7 @@ doc/source/_static/cluster-manager | |||
| python/build/ | |||
| python/dist/ | |||
| testing/scripts/proto | |||
| testing/scripts/tensorflow | |||
There was a problem hiding this comment.
Noticed these were generated so put in gitignore and also the clean script in the testing makefile
| @@ -273,7 +273,8 @@ | |||
| "source": [ | |||
| "!helm install ../../../helm-charts/seldon-core --name seldon-core \\\n", | |||
| " --namespace seldon \\\n", | |||
| " --set ambassador.enabled=true" | |||
| " --set ambassador.enabled=true \\\n", | |||
| " --set ambassador.env.AMBASSADOR_SINGLE_NAMESPACE=true" | |||
There was a problem hiding this comment.
I think this is not strictly necessary as otherwise it will be cluster-wide and that would work too. But previously it was namespace-only. In addition to running the tests I also stepped through this notebook to make sure it was ok to make this change. I was able to make all the requests.
There was a problem hiding this comment.
For a little while I wasn't able to see anything in the grafana dashboard but that turned out to be unrelated as I was seeing it with master too and it was fixed by recreating minikube.
| @@ -6,10 +6,17 @@ build_protos: | |||
| python -m grpc.tools.protoc -I. --python_out=. --grpc_python_out=. ./proto/prediction.proto | |||
| cd ../../notebooks && make build_protos | |||
|
|
|||
| #requires mvn, helm, ks, python, s2i and an accessible k8s cluster (if minikube increase mem and cpu) | |||
| #tests will create namespaces, deploy seldon core and create examples | |||
There was a problem hiding this comment.
Pointers to help getting started with running E2E tests
| rm -rf run.log | ||
| rm -rf tensorflow | ||
| rm -rf my-model | ||
| rm -rf ../../wrappers/s2i/python/_python/ |
There was a problem hiding this comment.
These are generated files so I figured they could be included in the clean
| @@ -3,6 +3,8 @@ | |||
| import subprocess | |||
| import os | |||
| import time | |||
| import random | |||
| from retrying import retry | |||
There was a problem hiding this comment.
Use of retry is to handle sporadic connectivity failures. See #473
Replaces #445
The core of this is that change.
As discussed in that thread, there are also changes to improve the reliability of the E2E tests.
The chart doesn't currently allow us to default ambassador to a single namespace and override so have had to change a lot of notebooks. The last commit does this. Will reverse that if the PR to the stable/ambassador chart is merged. We could create a ticket dedicated for keeping an eye on that and reversing these changes.