diff --git a/.gitignore b/.gitignore index 7fbb9d7ee8..38fc0f3247 100644 --- a/.gitignore +++ b/.gitignore @@ -192,7 +192,10 @@ doc/source/_static/cluster-manager python/build/ python/dist/ testing/scripts/proto + +python/.coverage + testing/scripts/tensorflow testing/scripts/run.log testing/scripts/my-model/ -wrappers/s2i/python/_python/ \ No newline at end of file +wrappers/s2i/python/_python/ diff --git a/api-frontend/src/main/java/io/seldon/apife/deployments/DeploymentStore.java b/api-frontend/src/main/java/io/seldon/apife/deployments/DeploymentStore.java index c0da1f76d1..f0e37ae96f 100644 --- a/api-frontend/src/main/java/io/seldon/apife/deployments/DeploymentStore.java +++ b/api-frontend/src/main/java/io/seldon/apife/deployments/DeploymentStore.java @@ -60,7 +60,10 @@ private void init() throws Exception{ public SeldonDeployment getDeployment(String clientId) { - return deploymentStore.get(clientId); + if (clientId != null) + return deploymentStore.get(clientId); + else + return null; } @Override diff --git a/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateImpl.java b/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateImpl.java index fe8f1df7a5..192cad27a0 100644 --- a/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateImpl.java +++ b/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateImpl.java @@ -105,7 +105,7 @@ public void updateStatus(String mlDepName, String version, String depName, Integ { logger.info(String.format("UPDATE %s : %s %d %d",mlDepName,depName,replicas,replicasAvailable)); SeldonDeployment mlDep = crdHandler.getSeldonDeployment(mlDepName,version, namespace); - if (mlDep != null) + if (mlDep != null && !(mlDep.getStatus().hasState() && mlDep.getStatus().getState().equals(Constants.STATE_FAILED))) { SeldonDeployment.Builder mlBuilder = SeldonDeployment.newBuilder(mlDep); diff --git a/cluster-manager/src/test/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateTest.java b/cluster-manager/src/test/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateTest.java index f4e70e6372..d9d81eb218 100644 --- a/cluster-manager/src/test/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateTest.java +++ b/cluster-manager/src/test/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateTest.java @@ -6,6 +6,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.never; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -54,15 +55,33 @@ public void updateAvailableTest() throws IOException supdate.updateStatus(selDepName, version, "test-deployment-fx-market-predictor-8e1d76f", 1, 1, namespace); verify(mockCrdHandler,times(1)).getSeldonDeployment(eq(selDepName), eq(version), eq(namespace)); - ArgumentCaptor argument = ArgumentCaptor.forClass(SeldonDeployment.class); - verify(mockCrdHandler,times(1)).updateSeldonDeploymentStatus(argument.capture()); + verify(mockCrdHandler,times(1)).updateSeldonDeploymentStatus(any(SeldonDeployment.class)); + + } + + @Test + public void updateAvailableOnFailedTest() throws IOException + { + createMocks(); - SeldonDeployment sDepUpdated = argument.getAllValues().get(0); - Assert.assertEquals(1, sDepUpdated.getStatus().getPredictorStatusCount()); - Assert.assertEquals(1, sDepUpdated.getStatus().getPredictorStatus(0).getReplicasAvailable()); - Assert.assertEquals("Available", sDepUpdated.getStatus().getState()); + String jsonStr = readFile("src/test/resources/model_failed.json",StandardCharsets.UTF_8); + SeldonDeployment sDep = SeldonDeploymentUtils.jsonToSeldonDeployment(jsonStr); + + when(mockCrdHandler.getSeldonDeployment(any(String.class), any(String.class), any(String.class))).thenReturn(sDep); + + SeldonDeploymentStatusUpdate supdate = new SeldonDeploymentStatusUpdateImpl(mockCrdHandler, mockSeldonDeploymentController, props); + + final String selDepName = "SeldonDep1"; + final String version = "v1alpha1"; + final String namespace = "seldon"; + + supdate.updateStatus(selDepName, version, "test-deployment-fx-market-predictor-8e1d76f", 1, 1, namespace); + + verify(mockCrdHandler,times(1)).getSeldonDeployment(eq(selDepName), eq(version), eq(namespace)); + verify(mockCrdHandler,never()).updateSeldonDeploymentStatus(any(SeldonDeployment.class)); } + @Test public void twoUpdatesTest() throws IOException { @@ -126,5 +145,7 @@ public void removeTest() throws IOException Assert.assertEquals("", sDepUpdated.getStatus().getState()); } + + } diff --git a/cluster-manager/src/test/resources/model_failed.json b/cluster-manager/src/test/resources/model_failed.json new file mode 100644 index 0000000000..0c0ed309ac --- /dev/null +++ b/cluster-manager/src/test/resources/model_failed.json @@ -0,0 +1,61 @@ +{ + "apiVersion": "machinelearning.seldon.io/v1alpha3", + "kind": "SeldonDeployment", + "metadata": { + "creationTimestamp": "2019-04-04T14:33:08Z", + "generation": 1, + "labels": { + "app": "seldon" + }, + "name": "seldon-model", + "namespace": "seldon", + "resourceVersion": "6700", + "selfLink": "/apis/machinelearning.seldon.io/v1alpha3/namespaces/seldon/seldondeployments/seldon-model", + "uid": "9101adcf-56e6-11e9-865f-ec5f8735c09f" + }, + "spec": { + "name": "test-deployment", + "oauth_key": "oauth-key", + "oauth_secret": "oauth-secret", + "predictors": [ + { + "componentSpecs": [ + { + "spec": { + "containers": [ + { + "imagePullPolicy": "IfNotPresent", + "image": "seldonio/mock_classifier:1.0", + "name": "classifier", + "resources": { + "requests": { + "memory": "1Mi" + } + } + } + ], + "terminationGracePeriodSeconds": 1 + } + } + ], + "graph": { + "children": [], + "endpoint": { + "type": "REST" + }, + "name": "classifier", + "type": "MODEL" + }, + "labels": { + "version": "v1" + }, + "name": "example", + "replicas": 1 + } + ] + }, + "status": { + "description": "Cannot find field: imagename in message k8s.io.api.core.v1.Container", + "state": "Failed" + } +} diff --git a/python/.coverage b/python/.coverage deleted file mode 100644 index 55b0da67e9..0000000000 Binary files a/python/.coverage and /dev/null differ diff --git a/python/seldon_core/microservice.py b/python/seldon_core/microservice.py index c6577e07ee..28d82e57cc 100644 --- a/python/seldon_core/microservice.py +++ b/python/seldon_core/microservice.py @@ -11,6 +11,7 @@ from seldon_core.flask_utils import ANNOTATIONS_FILE import seldon_core.wrapper as seldon_microservice from typing import Dict, Callable +from seldon_core.flask_utils import SeldonMicroserviceException logger = logging.getLogger(__name__) @@ -70,7 +71,16 @@ def parse_parameters(parameters: Dict) -> Dict: if type_ == "BOOL": parsed_parameters[name] = bool(strtobool(value)) else: - parsed_parameters[name] = type_dict[type_](value) + try: + parsed_parameters[name] = type_dict[type_](value) + except ValueError: + raise SeldonMicroserviceException( + "Bad model parameter: " + name + " with value " + value + " can't be parsed as a " + type_, + reason="MICROSERVICE_BAD_PARAMETER") + except KeyError: + raise SeldonMicroserviceException( + "Bad model parameter type: " + type_ + " valid are INT, FLOAT, DOUBLE, STRING, BOOL", + reason="MICROSERVICE_BAD_PARAMETER") return parsed_parameters @@ -167,6 +177,7 @@ def main(): log_level_num = getattr(logging, args.log_level.upper(), None) if not isinstance(log_level_num, int): raise ValueError('Invalid log level: %s', args.log_level) + logger.setLevel(log_level_num) logger.debug("Log level set to %s:%s", args.log_level, log_level_num) @@ -185,6 +196,9 @@ def main(): # set log level for the imported microservice type seldon_microservice.logger.setLevel(log_level_num) + logging.getLogger().setLevel(log_level_num) + for handler in logger.handlers: + handler.setLevel(log_level_num) port = int(os.environ.get(SERVICE_PORT_ENV_NAME, DEFAULT_PORT)) diff --git a/python/tests/test_microservice.py b/python/tests/test_microservice.py index 4a2d60cbdb..14f016305a 100644 --- a/python/tests/test_microservice.py +++ b/python/tests/test_microservice.py @@ -8,9 +8,12 @@ import pytest from seldon_core.proto import prediction_pb2 from seldon_core.proto import prediction_pb2_grpc +import seldon_core.microservice as microservice +from seldon_core.flask_utils import SeldonMicroserviceException import grpc import numpy as np import signal +import unittest @contextmanager def start_microservice(app_location,tracing=False,grpc=False,envs={}): @@ -116,6 +119,7 @@ def test_model_template_app_grpc(tracing): feedback = prediction_pb2.Feedback(request=request,reward=1.0) response = stub.SendFeedback(request=request) + def test_model_template_app_tracing_config(): envs = {"JAEGER_CONFIG_PATH":join(dirname(__file__), "tracing_config/tracing.yaml")} with start_microservice(join(dirname(__file__), "model-template-app"),tracing=True,envs=envs): @@ -133,3 +137,17 @@ def test_model_template_app_tracing_config(): "http://127.0.0.1:5000/send-feedback", params="json=%s" % data) response.raise_for_status() assert response.json() == {'data': {'ndarray': []}, 'meta': {}} + + +def test_model_template_bad_params(): + + params = [join(dirname(__file__), "model-template-app"),"seldon-core-microservice","REST","--parameters",'[{"type":"FLOAT","name":"foo","value":"abc"}]'] + with unittest.mock.patch('sys.argv',params): + with pytest.raises(SeldonMicroserviceException): + microservice.main() + +def test_model_template_bad_params_type(): + params = [join(dirname(__file__), "model-template-app"),"seldon-core-microservice","REST","--parameters",'[{"type":"FOO","name":"foo","value":"abc"}]'] + with unittest.mock.patch('sys.argv',params): + with pytest.raises(SeldonMicroserviceException): + microservice.main() \ No newline at end of file