Skip to content

Commit

Permalink
Merge pull request #501 from SeldonIO/473-downgrade-ambassador
Browse files Browse the repository at this point in the history
downgrade ambassador due to grpc unreliability
  • Loading branch information
ukclivecox authored Apr 10, 2019
2 parents d3c1fb1 + d8fc0bd commit a51db9d
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ private String getAmbassadorAnnotation(SeldonDeployment mlDep,String serviceName
final String customRegexHeader = mlDep.getSpec().getAnnotationsOrDefault(Constants.AMBASSADOR_REGEX_HEADER_ANNOTATION, null);

final String restMapping = "---\n"+
"apiVersion: ambassador/v1\n" +
"apiVersion: ambassador/v0\n" +
"kind: Mapping\n" +
"name: seldon_"+mlDep.getMetadata().getName()+"_rest_mapping\n" +
"prefix: /seldon/"+serviceNameExternal+"/\n" +
Expand All @@ -531,7 +531,7 @@ private String getAmbassadorAnnotation(SeldonDeployment mlDep,String serviceName
(StringUtils.isNotEmpty(weight) ? ("weight: "+ weight + "\n") : "") +
(StringUtils.isNotEmpty(shadowing) ? ("shadow: true\n") : "");
final String grpcMapping = "---\n"+
"apiVersion: ambassador/v1\n" +
"apiVersion: ambassador/v0\n" +
"kind: Mapping\n" +
"name: "+mlDep.getMetadata().getName()+"_grpc_mapping\n" +
"grpc: true\n" +
Expand All @@ -547,7 +547,7 @@ private String getAmbassadorAnnotation(SeldonDeployment mlDep,String serviceName
(StringUtils.isNotEmpty(shadowing) ? ("shadow: true\n") : "");

final String restMappingNamespaced = "---\n"+
"apiVersion: ambassador/v1\n" +
"apiVersion: ambassador/v0\n" +
"kind: Mapping\n" +
"name: seldon_"+namespace+"_"+mlDep.getMetadata().getName()+"_rest_mapping\n" +
"prefix: /seldon/"+namespace+"/"+serviceNameExternal+"/\n" +
Expand All @@ -559,7 +559,7 @@ private String getAmbassadorAnnotation(SeldonDeployment mlDep,String serviceName
(StringUtils.isNotEmpty(shadowing) ? ("shadow: true\n") : "");

final String grpcMappingNamespaced = "---\n"+
"apiVersion: ambassador/v1\n" +
"apiVersion: ambassador/v0\n" +
"kind: Mapping\n" +
"name: "+namespace+"_"+mlDep.getMetadata().getName()+"_grpc_mapping\n" +
"grpc: true\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void checkAmbassadorCanary() throws IOException, SeldonDeploymentExceptio
System.out.println(ambassadorConfig);
Assert.assertTrue(ambassadorConfig.indexOf("weight: 25\n")>0);
Assert.assertTrue(ambassadorConfig.indexOf("prefix: /seldon/default/example/\n")>0);
Assert.assertTrue(ambassadorConfig.indexOf("apiVersion: ambassador/v1")==4);
Assert.assertTrue(ambassadorConfig.indexOf("apiVersion: ambassador/v0")==4);
}

@Test
Expand All @@ -61,7 +61,7 @@ public void checkAmbassadorShadow() throws IOException, SeldonDeploymentExceptio
System.out.println(ambassadorConfig);
Assert.assertTrue(ambassadorConfig.indexOf("shadow: true\n")>0);
Assert.assertTrue(ambassadorConfig.indexOf("prefix: /seldon/default/example/\n")>0);
Assert.assertTrue(ambassadorConfig.indexOf("apiVersion: ambassador/v1")==4);
Assert.assertTrue(ambassadorConfig.indexOf("apiVersion: ambassador/v0")==4);
}

@Test
Expand All @@ -79,7 +79,7 @@ public void checkAmbassadorHeader() throws IOException, SeldonDeploymentExceptio
System.out.println(ambassadorConfig);
Assert.assertTrue(ambassadorConfig.indexOf(" location: london\n")>0);
Assert.assertTrue(ambassadorConfig.indexOf("prefix: /seldon/default/example/\n")>0);
Assert.assertTrue(ambassadorConfig.indexOf("apiVersion: ambassador/v1")==4);
Assert.assertTrue(ambassadorConfig.indexOf("apiVersion: ambassador/v0")==4);
}

@Test
Expand All @@ -97,7 +97,7 @@ public void checkAmbassadorCustomConfig() throws IOException, SeldonDeploymentEx
System.out.println(ambassadorConfig);
Assert.assertTrue(ambassadorConfig.indexOf("1234")==0);
Assert.assertFalse(ambassadorConfig.indexOf("prefix: /seldon/default/example/\n")>0);
Assert.assertTrue(ambassadorConfig.indexOf("apiVersion: ambassador/v1")==-1);
Assert.assertTrue(ambassadorConfig.indexOf("apiVersion: ambassador/v0")==-1);
}

}
2 changes: 1 addition & 1 deletion doc/source/graph/ambassador.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ To understand more about the Ambassador configuration for this see [their docs o
The above discussed configurations should cover most cases but there maybe a case where you want to have a very particular Ambassador configuration under your control. You can acheieve this by adding your confguration as an annotation to your Seldon Deployment resource.

* `seldon.io/ambassador-config:<configuration>` : The custom ambassador configuration
* Example: `"seldon.io/ambassador-config":"apiVersion: ambassador/v1\nkind: Mapping\nname: seldon_example_rest_mapping\nprefix: /mycompany/ml/\nservice: production-model-example.seldon:8000\ntimeout_ms: 3000"`
* Example: `"seldon.io/ambassador-config":"apiVersion: ambassador/v0\nkind: Mapping\nname: seldon_example_rest_mapping\nprefix: /mycompany/ml/\nservice: production-model-example.seldon:8000\ntimeout_ms: 3000"`

A worked example for [custom Ambassador config](../examples/ambassador_custom.html) is provided.

2 changes: 1 addition & 1 deletion examples/ambassador/custom/model_custom_ambassador.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"spec": {
"name": "production-model",
"annotations": {
"seldon.io/ambassador-config":"apiVersion: ambassador/v1\nkind: Mapping\nname: seldon_example_rest_mapping\nprefix: /mycompany/ml/\nservice: production-model-example.seldon:8000\ntimeout_ms: 3000"
"seldon.io/ambassador-config":"apiVersion: ambassador/v0\nkind: Mapping\nname: seldon_example_rest_mapping\nprefix: /mycompany/ml/\nservice: production-model-example.seldon:8000\ntimeout_ms: 3000"
},
"predictors": [
{
Expand Down
7 changes: 2 additions & 5 deletions helm-charts/seldon-core/values.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
ambassador:
enabled: false
image:
tag: 0.53.1
tag: 0.50.0
replicaCount: 1
resources:
limits:
Expand All @@ -14,7 +14,7 @@ ambassador:
annotations:
getambassador.io/config: |
---
apiVersion: ambassador/v1
apiVersion: ambassador/v0
kind: Module
name: ambassador
config:
Expand All @@ -27,9 +27,6 @@ ambassador:
rbac:
create: true
namespaced: true
securityContext:
runAsUser: 0
runAsGroup: 0
# scope will be cluster wide unless below is set
# env:
# AMBASSADOR_SINGLE_NAMESPACE: "true"
Expand Down
2 changes: 1 addition & 1 deletion testing/scripts/seldon_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def rest_request_ambassador_auth(deploymentName,namespace,username,password,endp
auth=HTTPBasicAuth(username, password))
return response

@retry(wait_exponential_multiplier=1000, wait_exponential_max=30000, stop_max_attempt_number=7)
@retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, stop_max_attempt_number=5)
def grpc_request_ambassador(deploymentName,namespace,endpoint="localhost:8004",data_size=5,rows=1,data=None):
if data is None:
shape, arr = create_random_data(data_size,rows)
Expand Down
2 changes: 1 addition & 1 deletion testing/scripts/test_bad_graphs.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from k8s_utils import *

def wait_for_status(name):
for attempts in range(3):
for attempts in range(5):
completedProcess = run("kubectl get sdep "+name+" -o json", shell=True, check=True, stdout=subprocess.PIPE)
jStr = completedProcess.stdout
j = json.loads(jStr)
Expand Down

0 comments on commit a51db9d

Please sign in to comment.