From aa6e1db3a146e4e4918eee2f5a2e352147e919ab Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Fri, 23 Nov 2018 20:58:41 -0800 Subject: [PATCH 01/20] Update Basic example README.md --- examples/basic_query/README.md | 123 +++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 6 deletions(-) diff --git a/examples/basic_query/README.md b/examples/basic_query/README.md index e67b0ee17..cfa90f249 100644 --- a/examples/basic_query/README.md +++ b/examples/basic_query/README.md @@ -7,12 +7,123 @@ The examples in this directory assume you have the `clipper_admin` pip package i pip install clipper_admin ``` We recommend using [Anaconda](https://www.continuum.io/downloads) -to install Python packages. +to install Python packages. # Running the example query -1. Start Clipper locally - + With Docker `cd /docker && docker-compose up -d query_frontend` - + Without Docker `/bin/start_clipper.sh` -2. Run the example: `python example_client.py` -3. Connect a container: `cd /containers/python && CLIPPER_MODEL_NAME=example_model CLIPPER_MODEL_VERSION=1 CLIPPER_INPUT_TYPE=doubles python noop_container.py` +Run the example: `python example_client.py` +Note that in the version 0.3, Python3.7 is not supported. + +## Possible Errors +Sometimes, because of version issues, you can see this error logs from docker log. +``` +$ docker logs 439ba722d79a +Starting Python Closure container +Connecting to Clipper with default port: 7000 +Traceback (most recent call last): + File "/container/python_closure_container.py", line 56, in + rpc_service.get_input_type()) + File "/container/python_closure_container.py", line 28, in __init__ + self.predict_func = load_predict_func(predict_path) + File "/container/python_closure_container.py", line 17, in load_predict_func + return cloudpickle.load(serialized_func_file) + File "/usr/local/lib/python3.6/site-packages/cloudpickle/cloudpickle.py", line 1060, in _make_skel_func + base_globals['__builtins__'] = __builtins__ +TypeError: 'str' object does not support item assignment +``` + +In this case, you can fix this issue by installing cloudpickle dependency. +`pip install -U cloudpickle==0.5.3` + +## Model Function +```python +def feature_sum(xs): + return [str(sum(x)) for x in xs] + ``` + +Define a simple model that just returns the sum of each feature vector. Note that the prediction function takes a list of feature vectors as input and returns a list of strings. + +## Basic Example Code Explanation +Let's import necessary classes for applications and models registration. Our model in the example will be a Python function. +```python +from clipper_admin import ClipperConnection, DockerContainerManager +from clipper_admin.deployers import python as python_deployer +``` + +We first create a connection to a clipper cluster. Note that we should define a container manager when we initiate a connection. +In this example, we will use a Docker container manager. +```python +clipper_conn = ClipperConnection(DockerContainerManager()) + +""" +DockerContainerManager uses Docker-Compose to orchestrate the Clipper cluster. +If you want to use Kubernetes for orchestration, use KubernetesContainerManager instead. +You can also create your own ContainerManager by inheriting container_manager class. +""" +``` + +Now, let's create a Clipper cluster. start_clipper() function creates containers for a Clipper cluster. +It includes Query Frontend, Prometheus metric server, Frontend management, and Redis. + +```python +clipper_conn.start_clipper() +``` + +Once we start a Clipper cluster, we can see the log that our cluster is running. + +``` +18-11-23:16:56:41 INFO [docker_container_manager.py:151] [default-cluster] Starting managed Redis instance in Docker +18-11-23:16:59:28 INFO [docker_container_manager.py:229] [default-cluster] Metric Configuration Saved at /private/var/folders/b7/gfcqcp6n1qv63rkfpkn_0qnjcxxskx/T/tmp2l4pspy4.yml +18-11-23:16:59:28 INFO [clipper_admin.py:138] [default-cluster] Clipper is running +``` + +Once Clipper cluster is started, we should register applications and models. In our case, our model will be a python function, `feature_sum`, we created above. +We will use a python deployer to deploy our application and model. +Check out the link to learn what is model deployers and what model deployers Clipper currently has. http://docs.clipper.ai/en/v0.3.0/model_deployers.html +```python +python_deployer.create_endpoint(clipper_conn, "simple-example", "doubles", 46, feature_sum) +``` + +python_deployer (clipper_admin.deployers.python) registers application, deploys a model, and link the model to the app. (It can be changed in the later update. It is written when the verion is 0.3) +If you want to create your own Dockerfile, don't forget that you should register the application and connect the model to the application. +Also, from the following link, see the explanation about `Applications` to understand the concept of Clipper applications and why you need to connect your model to application. +http://clipper.ai/tutorials/basic_concepts/ + +```python +clipper_conn.register_application(name, input_type, default_output, + slo_micros) +deploy_python_closure(clipper_conn, name, version, input_type, func, + base_image, labels, registry, num_replicas, + batch_size, pkgs_to_install) + +clipper_conn.link_model_to_app(name, name) +``` + +We can see these logs when the registration is succesfully done. + +``` +18-11-23:16:59:28 INFO [clipper_admin.py:215] [default-cluster] Application simple-example was successfully registered +18-11-23:16:59:28 INFO [deployer_utils.py:41] Saving function to /var/folders/b7/gfcqcp6n1qv63rkfpkn_0qnjcxxskx/T/tmp_810jx8yclipper +18-11-23:16:59:28 INFO [deployer_utils.py:51] Serialized and supplied predict function +18-11-23:16:59:28 INFO [python.py:192] Python closure saved +18-11-23:16:59:28 INFO [python.py:206] Using Python 3.6 base image +18-11-23:16:59:28 INFO [clipper_admin.py:467] [default-cluster] Building model Docker image with model data from /var/folders/b7/gfcqcp6n1qv63rkfpkn_0qnjcxxskx/T/tmp_810jx8yclipper +18-11-23:16:59:43 INFO [clipper_admin.py:472] [default-cluster] Step 1/2 : FROM clipper/python36-closure-container:develop +18-11-23:16:59:43 INFO [clipper_admin.py:472] [default-cluster] ---> 0fac6e6e8242 +18-11-23:16:59:43 INFO [clipper_admin.py:472] [default-cluster] Step 2/2 : COPY /var/folders/b7/gfcqcp6n1qv63rkfpkn_0qnjcxxskx/T/tmp_810jx8yclipper /model/ +18-11-23:16:59:43 INFO [clipper_admin.py:472] [default-cluster] ---> 4a2709ceaa4f +18-11-23:16:59:43 INFO [clipper_admin.py:472] [default-cluster] Successfully built 4a2709ceaa4f +18-11-23:16:59:43 INFO [clipper_admin.py:472] [default-cluster] Successfully tagged default-cluster-simple-example:1 +18-11-23:16:59:43 INFO [clipper_admin.py:474] [default-cluster] Pushing model Docker image to default-cluster-simple-example:1 +18-11-23:16:59:46 INFO [docker_container_manager.py:353] [default-cluster] Found 0 replicas for simple-example:1. Adding 1 +18-11-23:16:59:53 INFO [clipper_admin.py:651] [default-cluster] Successfully registered model simple-example:1 +18-11-23:16:59:53 INFO [clipper_admin.py:569] [default-cluster] Done deploying model simple-example:1. +18-11-23:16:59:53 INFO [clipper_admin.py:277] [default-cluster] Model simple-example is now linked to application simple-example +``` + +Now that you’ve deployed your first model, you can start requesting predictions with your favorite REST client at the endpoint that Clipper created for your application: +`http://localhost:1337//predict` +To check this out, try out this command. +``` +curl -X POST --header "Content-Type:application/json" -d '{"input": [1.1, 2.2, 3.3]}' 127.0.0.1:1337/simple-example/predict +``` From a05443a0af3acb6973d53c3c784c9bc08c8d7835 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Mon, 4 Feb 2019 16:23:43 -0800 Subject: [PATCH 02/20] Added a basic fluentd support --- .../clipper_admin/container_manager.py | 4 ++- .../docker/docker_container_manager.py | 33 ++++++++++++++++- .../docker/docker_logging_utils.py | 36 +++++++++++++++++++ .../docker/docker_metric_utils.py | 6 ++++ dockerfiles/FluentdLoggingContainerDockerfile | 1 + logging/README.md | 2 ++ 6 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 clipper_admin/clipper_admin/docker/docker_logging_utils.py create mode 100644 dockerfiles/FluentdLoggingContainerDockerfile create mode 100644 logging/README.md diff --git a/clipper_admin/clipper_admin/container_manager.py b/clipper_admin/clipper_admin/container_manager.py index b03687adb..cc06602c1 100644 --- a/clipper_admin/clipper_admin/container_manager.py +++ b/clipper_admin/clipper_admin/container_manager.py @@ -8,6 +8,7 @@ CLIPPER_INTERNAL_RPC_PORT = 7000 CLIPPER_INTERNAL_METRIC_PORT = 1390 CLIPPER_INTERNAL_REDIS_PORT = 6379 +CLIPPER_INTERNAL_FLUENTD_PORT = 24224 CLIPPER_DOCKER_LABEL = "ai.clipper.container.label" CLIPPER_NAME_LABEL = "ai.clipper.name" @@ -22,7 +23,8 @@ 'query_rest': 'ai.clipper.query_frontend.query.port', 'query_rpc': 'ai.clipper.query_frontend.rpc.port', 'management': 'ai.clipper.management.port', - 'metric': 'ai.clipper.metric.port' + 'metric': 'ai.clipper.metric.port', + 'fluentd': 'ai.clipper.fluentd.port' } CLIPPER_METRIC_CONFIG_LABEL = 'ai.clipper.metric.config' diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 2b6ddf827..759002b73 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -16,19 +16,22 @@ CLIPPER_QUERY_FRONTEND_CONTAINER_LABEL, CLIPPER_MGMT_FRONTEND_CONTAINER_LABEL, CLIPPER_INTERNAL_RPC_PORT, CLIPPER_INTERNAL_QUERY_PORT, CLIPPER_INTERNAL_MANAGEMENT_PORT, - CLIPPER_INTERNAL_METRIC_PORT, CLIPPER_INTERNAL_REDIS_PORT, + CLIPPER_INTERNAL_METRIC_PORT, CLIPPER_INTERNAL_REDIS_PORT, CLIPPER_INTERNAL_FLUENTD_PORT, CLIPPER_DOCKER_PORT_LABELS, CLIPPER_METRIC_CONFIG_LABEL, ClusterAdapter) from ..exceptions import ClipperException from requests.exceptions import ConnectionError from .docker_metric_utils import * +from .docker_logging_utils import run_fluentd_image logger = logging.getLogger(__name__) class DockerContainerManager(ContainerManager): + # SANG-TODO Add SQLITE support def __init__(self, cluster_name="default-cluster", docker_ip_address="localhost", + fluentd_port=24224, clipper_query_port=1337, clipper_management_port=1338, clipper_rpc_port=7000, @@ -47,6 +50,8 @@ def __init__(self, The public hostname or IP address at which the Clipper Docker containers can be accessed via their exposed ports. This should almost always be "localhost". Only change if you know what you're doing! + fluentd_port : int, optional + The port on which the fluentd logging driver should listen to centralize logs. clipper_query_port : int, optional The port on which the query frontend should listen for incoming prediction requests. clipper_management_port : int, optional @@ -70,6 +75,8 @@ def __init__(self, self.cluster_name = cluster_name self.cluster_identifier = cluster_name # For logging purpose self.public_hostname = docker_ip_address + # SANG-TODO Add SQLITE support + self.fluentd_port = fluentd_port self.clipper_query_port = clipper_query_port self.clipper_management_port = clipper_management_port self.clipper_rpc_port = clipper_rpc_port @@ -110,6 +117,7 @@ def __init__(self, 'cluster_name': self.cluster_identifier }) + #SANG-TODO Add sqlite support def start_clipper(self, query_frontend_image, mgmt_frontend_image, @@ -147,6 +155,17 @@ def start_clipper(self, "Please use ClipperConnection.connect() to connect to it.". format(self.cluster_name)) + # SQLite Logging DB + + # Fluentd for Logging Centralization + self.fluentd_port = find_unbound_port(self.fluentd_port) + fluentd_labels = self.common_labels.copy() + fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str( + self.fluentd_port) + run_fluentd_image(self.docker_client, fluentd_labels, + self.fluend_port, self.extra_container_kwargs) + + # Redis for cluster configuration if not self.external_redis: self.logger.info("Starting managed Redis instance in Docker") self.redis_port = find_unbound_port(self.redis_port) @@ -156,6 +175,9 @@ def start_clipper(self, redis_container = self.docker_client.containers.run( 'redis:alpine', "redis-server --port %s" % CLIPPER_INTERNAL_REDIS_PORT, + log_config={ + "type": "fluentd" + }, name="redis-{}".format(random.randint( 0, 100000)), # generate a random name ports={ @@ -165,6 +187,7 @@ def start_clipper(self, **self.extra_container_kwargs) self.redis_ip = redis_container.name + # frontend management mgmt_cmd = "--redis_ip={redis_ip} --redis_port={redis_port}".format( redis_ip=self.redis_ip, redis_port=CLIPPER_INTERNAL_REDIS_PORT) self.clipper_management_port = find_unbound_port( @@ -176,6 +199,10 @@ def start_clipper(self, self.docker_client.containers.run( mgmt_frontend_image, mgmt_cmd, + # SANG-TODO log_config shouldn't be always included + log_config={ + "type": "fluentd" + }, name="mgmt_frontend-{}".format(random.randint( 0, 100000)), # generate a random name ports={ @@ -185,6 +212,7 @@ def start_clipper(self, labels=mgmt_labels, **self.extra_container_kwargs) + # query frontend query_cmd = ("--redis_ip={redis_ip} --redis_port={redis_port} " "--prediction_cache_size={cache_size}").format( redis_ip=self.redis_ip, @@ -204,6 +232,9 @@ def start_clipper(self, self.docker_client.containers.run( query_frontend_image, query_cmd, + log_config={ + "type": "fluentd" + }, name=query_name, ports={ '%s/tcp' % CLIPPER_INTERNAL_QUERY_PORT: diff --git a/clipper_admin/clipper_admin/docker/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/docker_logging_utils.py new file mode 100644 index 000000000..d18851c6c --- /dev/null +++ b/clipper_admin/clipper_admin/docker/docker_logging_utils.py @@ -0,0 +1,36 @@ +import random + +FLUENTD_VERSION = 'v1.3-debian-1' + + +def run_fluentd_image(docker_client, fluentd_labels, fluend_port, extra_container_kwargs): + + fluentd_cmd = '' # There's no special fluentd cmd for now + fluentd_container_id = random.randint(0, 100000) + fluentd_name = "fluentd-{}".format(fluentd_container_id) + fluentd_img = 'fluent/fluentd:{version}'.format(version=FLUENTD_VERSION) + + docker_client.containers.run( + fluentd_img, + fluentd_cmd, + name=fluentd_name, + port={ + '%s/tcp' % fluend_port: fluend_port, + '%s/udp' % fluend_port: fluend_port + }, + labels=fluentd_labels, + detach=True, + **extra_container_kwargs) + +class FluentdConfig: + """ + Follow a builder pattern to contruct a fluentd configuration file, fluentd.conf + """ + def __init__(self): + self.conf = { + + } + + def build(self): + pass + diff --git a/clipper_admin/clipper_admin/docker/docker_metric_utils.py b/clipper_admin/clipper_admin/docker/docker_metric_utils.py index e528edb44..5d6422934 100644 --- a/clipper_admin/clipper_admin/docker/docker_metric_utils.py +++ b/clipper_admin/clipper_admin/docker/docker_metric_utils.py @@ -38,6 +38,9 @@ def run_query_frontend_metric_image(name, docker_client, query_name, docker_client.containers.run( frontend_exporter_image, query_frontend_metric_cmd, + log_config={ + "type": "fluentd" + }, name=name, labels=query_frontend_metric_labels, **extra_container_kwargs) @@ -96,6 +99,9 @@ def run_metric_image(docker_client, common_labels, prometheus_port, metric_cmd, name="metric_frontend-{}".format(random.randint(0, 100000)), ports={'9090/tcp': prometheus_port}, + log_config={ + "type": "fluentd" + }, volumes={ prom_config_path: { 'bind': '/etc/prometheus/prometheus.yml', diff --git a/dockerfiles/FluentdLoggingContainerDockerfile b/dockerfiles/FluentdLoggingContainerDockerfile new file mode 100644 index 000000000..94ced68b7 --- /dev/null +++ b/dockerfiles/FluentdLoggingContainerDockerfile @@ -0,0 +1 @@ +# Currently, we are using a default Fluentd image from Dockerhub. Needs to be customized by Clipper \ No newline at end of file diff --git a/logging/README.md b/logging/README.md new file mode 100644 index 000000000..926751485 --- /dev/null +++ b/logging/README.md @@ -0,0 +1,2 @@ +# Clipper Logging Architecture with Fluentd + From d7bd73abe6160ec8c1d945dd2ee418e50dad8d30 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Sun, 10 Feb 2019 14:56:52 -0800 Subject: [PATCH 03/20] log_config option added --- clipper_admin/clipper_admin/clipper_admin.py | 1 + .../docker/docker_container_manager.py | 41 +++++++++++-------- .../docker/docker_logging_utils.py | 7 +++- .../docker/docker_metric_utils.py | 8 +--- 4 files changed, 31 insertions(+), 26 deletions(-) diff --git a/clipper_admin/clipper_admin/clipper_admin.py b/clipper_admin/clipper_admin/clipper_admin.py index 3a4291778..10cf1e260 100644 --- a/clipper_admin/clipper_admin/clipper_admin.py +++ b/clipper_admin/clipper_admin/clipper_admin.py @@ -132,6 +132,7 @@ def start_clipper(self, host=self.cm.get_admin_addr()) for name, url in [('query frontend', query_frontend_url), ('management frontend', mgmt_frontend_url)]: + print('{}: {}'.format(name, url)) r = requests.get(url, timeout=5) if r.status_code != requests.codes.ok: raise RequestException( diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 759002b73..8c746d530 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -16,7 +16,7 @@ CLIPPER_QUERY_FRONTEND_CONTAINER_LABEL, CLIPPER_MGMT_FRONTEND_CONTAINER_LABEL, CLIPPER_INTERNAL_RPC_PORT, CLIPPER_INTERNAL_QUERY_PORT, CLIPPER_INTERNAL_MANAGEMENT_PORT, - CLIPPER_INTERNAL_METRIC_PORT, CLIPPER_INTERNAL_REDIS_PORT, CLIPPER_INTERNAL_FLUENTD_PORT, + CLIPPER_INTERNAL_METRIC_PORT, CLIPPER_INTERNAL_REDIS_PORT, CLIPPER_DOCKER_PORT_LABELS, CLIPPER_METRIC_CONFIG_LABEL, ClusterAdapter) from ..exceptions import ClipperException from requests.exceptions import ConnectionError @@ -31,6 +31,7 @@ class DockerContainerManager(ContainerManager): def __init__(self, cluster_name="default-cluster", docker_ip_address="localhost", + centralize_log=False, fluentd_port=24224, clipper_query_port=1337, clipper_management_port=1338, @@ -50,6 +51,8 @@ def __init__(self, The public hostname or IP address at which the Clipper Docker containers can be accessed via their exposed ports. This should almost always be "localhost". Only change if you know what you're doing! + centralize_log: bool, optional + If it is True, Clipper sets up Fluentd and DB (Currently SQlite) to centralize logs fluentd_port : int, optional The port on which the fluentd logging driver should listen to centralize logs. clipper_query_port : int, optional @@ -111,6 +114,11 @@ def __init__(self, "detach": True, } + # Logging centralization args. Currently, fluentd is used to centralize logs + self.centralize_log = centralize_log + self.log_config = {'type': 'fluentd'} if centralize_log \ + else {'type': 'json-file'} + self.extra_container_kwargs.update(container_args) self.logger = ClusterAdapter(logger, { @@ -155,15 +163,16 @@ def start_clipper(self, "Please use ClipperConnection.connect() to connect to it.". format(self.cluster_name)) - # SQLite Logging DB + if self.centralize_log: + # SQLite Logging DB - # Fluentd for Logging Centralization - self.fluentd_port = find_unbound_port(self.fluentd_port) - fluentd_labels = self.common_labels.copy() - fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str( - self.fluentd_port) - run_fluentd_image(self.docker_client, fluentd_labels, - self.fluend_port, self.extra_container_kwargs) + # Fluentd for Logging Centralization + self.fluentd_port = find_unbound_port(self.fluentd_port) + fluentd_labels = self.common_labels.copy() + fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str( + self.fluentd_port) + run_fluentd_image(self.docker_client, fluentd_labels, + self.fluend_port, self.extra_container_kwargs) # Redis for cluster configuration if not self.external_redis: @@ -175,9 +184,7 @@ def start_clipper(self, redis_container = self.docker_client.containers.run( 'redis:alpine', "redis-server --port %s" % CLIPPER_INTERNAL_REDIS_PORT, - log_config={ - "type": "fluentd" - }, + #log_config=self.log_config, name="redis-{}".format(random.randint( 0, 100000)), # generate a random name ports={ @@ -200,9 +207,7 @@ def start_clipper(self, mgmt_frontend_image, mgmt_cmd, # SANG-TODO log_config shouldn't be always included - log_config={ - "type": "fluentd" - }, + #log_config=self.log_config, name="mgmt_frontend-{}".format(random.randint( 0, 100000)), # generate a random name ports={ @@ -232,9 +237,7 @@ def start_clipper(self, self.docker_client.containers.run( query_frontend_image, query_cmd, - log_config={ - "type": "fluentd" - }, + #log_config=self.log_config, name=query_name, ports={ '%s/tcp' % CLIPPER_INTERNAL_QUERY_PORT: @@ -298,6 +301,7 @@ def connect(self): 'query_rpc']] self.prometheus_port = all_labels[CLIPPER_DOCKER_PORT_LABELS['metric']] self.prom_config_path = all_labels[CLIPPER_METRIC_CONFIG_LABEL] + # SANG-TODO Add a Fluentd Support def deploy_model(self, name, version, input_type, image, num_replicas=1): # Parameters @@ -356,6 +360,7 @@ def _add_replica(self, name, version, input_type, image): model_container_name = model_container_label + '-{}'.format( random.randint(0, 100000)) + # TODO SANG - Add fluentd support self.docker_client.containers.run( image, name=model_container_name, diff --git a/clipper_admin/clipper_admin/docker/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/docker_logging_utils.py index d18851c6c..312c69684 100644 --- a/clipper_admin/clipper_admin/docker/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/docker_logging_utils.py @@ -1,5 +1,7 @@ import random +from ..container_manager import CLIPPER_INTERNAL_FLUENTD_PORT + FLUENTD_VERSION = 'v1.3-debian-1' @@ -15,13 +17,14 @@ def run_fluentd_image(docker_client, fluentd_labels, fluend_port, extra_containe fluentd_cmd, name=fluentd_name, port={ - '%s/tcp' % fluend_port: fluend_port, - '%s/udp' % fluend_port: fluend_port + '%s/tcp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port, + '%s/udp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port }, labels=fluentd_labels, detach=True, **extra_container_kwargs) + class FluentdConfig: """ Follow a builder pattern to contruct a fluentd configuration file, fluentd.conf diff --git a/clipper_admin/clipper_admin/docker/docker_metric_utils.py b/clipper_admin/clipper_admin/docker/docker_metric_utils.py index 5d6422934..9dd034a0d 100644 --- a/clipper_admin/clipper_admin/docker/docker_metric_utils.py +++ b/clipper_admin/clipper_admin/docker/docker_metric_utils.py @@ -38,9 +38,7 @@ def run_query_frontend_metric_image(name, docker_client, query_name, docker_client.containers.run( frontend_exporter_image, query_frontend_metric_cmd, - log_config={ - "type": "fluentd" - }, + #log_config=log_config, name=name, labels=query_frontend_metric_labels, **extra_container_kwargs) @@ -99,9 +97,7 @@ def run_metric_image(docker_client, common_labels, prometheus_port, metric_cmd, name="metric_frontend-{}".format(random.randint(0, 100000)), ports={'9090/tcp': prometheus_port}, - log_config={ - "type": "fluentd" - }, + #log_config=log_config, volumes={ prom_config_path: { 'bind': '/etc/prometheus/prometheus.yml', From f0dae98e50c98911f8927f6c68754a2e3459d574 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Tue, 12 Feb 2019 15:27:47 -0800 Subject: [PATCH 04/20] fluentd instance is now running --- .../docker/docker_container_manager.py | 21 ++++++++++--------- .../docker/docker_logging_utils.py | 3 +-- .../docker/docker_metric_utils.py | 8 +++---- examples/basic_query/example_client.py | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 8c746d530..86382d7f2 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -31,7 +31,7 @@ class DockerContainerManager(ContainerManager): def __init__(self, cluster_name="default-cluster", docker_ip_address="localhost", - centralize_log=False, + use_centralized_log=False, fluentd_port=24224, clipper_query_port=1337, clipper_management_port=1338, @@ -51,7 +51,7 @@ def __init__(self, The public hostname or IP address at which the Clipper Docker containers can be accessed via their exposed ports. This should almost always be "localhost". Only change if you know what you're doing! - centralize_log: bool, optional + use_centralized_log: bool, optional If it is True, Clipper sets up Fluentd and DB (Currently SQlite) to centralize logs fluentd_port : int, optional The port on which the fluentd logging driver should listen to centralize logs. @@ -115,8 +115,8 @@ def __init__(self, } # Logging centralization args. Currently, fluentd is used to centralize logs - self.centralize_log = centralize_log - self.log_config = {'type': 'fluentd'} if centralize_log \ + self.centralize_log = use_centralized_log + self.log_config = {'type': 'fluentd'} if use_centralized_log \ else {'type': 'json-file'} self.extra_container_kwargs.update(container_args) @@ -167,12 +167,13 @@ def start_clipper(self, # SQLite Logging DB # Fluentd for Logging Centralization + self.logger.info("Starting Fluentd instance in Docker") self.fluentd_port = find_unbound_port(self.fluentd_port) fluentd_labels = self.common_labels.copy() fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str( self.fluentd_port) run_fluentd_image(self.docker_client, fluentd_labels, - self.fluend_port, self.extra_container_kwargs) + self.fluentd_port, self.extra_container_kwargs) # Redis for cluster configuration if not self.external_redis: @@ -184,7 +185,7 @@ def start_clipper(self, redis_container = self.docker_client.containers.run( 'redis:alpine', "redis-server --port %s" % CLIPPER_INTERNAL_REDIS_PORT, - #log_config=self.log_config, + log_config=self.log_config, name="redis-{}".format(random.randint( 0, 100000)), # generate a random name ports={ @@ -207,7 +208,7 @@ def start_clipper(self, mgmt_frontend_image, mgmt_cmd, # SANG-TODO log_config shouldn't be always included - #log_config=self.log_config, + log_config=self.log_config, name="mgmt_frontend-{}".format(random.randint( 0, 100000)), # generate a random name ports={ @@ -237,7 +238,7 @@ def start_clipper(self, self.docker_client.containers.run( query_frontend_image, query_cmd, - #log_config=self.log_config, + log_config=self.log_config, name=query_name, ports={ '%s/tcp' % CLIPPER_INTERNAL_QUERY_PORT: @@ -253,7 +254,7 @@ def start_clipper(self, run_query_frontend_metric_image( query_frontend_metric_name, self.docker_client, query_name, frontend_exporter_image, self.common_labels, - self.extra_container_kwargs) + self.log_config, self.extra_container_kwargs) self.prom_config_path = tempfile.NamedTemporaryFile( 'w', suffix='.yml', delete=False).name @@ -271,7 +272,7 @@ def start_clipper(self, metric_labels[CLIPPER_METRIC_CONFIG_LABEL] = self.prom_config_path run_metric_image(self.docker_client, metric_labels, self.prometheus_port, self.prom_config_path, - self.extra_container_kwargs) + self.log_config, self.extra_container_kwargs) self.connect() diff --git a/clipper_admin/clipper_admin/docker/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/docker_logging_utils.py index 312c69684..c292bf4ec 100644 --- a/clipper_admin/clipper_admin/docker/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/docker_logging_utils.py @@ -16,12 +16,11 @@ def run_fluentd_image(docker_client, fluentd_labels, fluend_port, extra_containe fluentd_img, fluentd_cmd, name=fluentd_name, - port={ + ports={ '%s/tcp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port, '%s/udp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port }, labels=fluentd_labels, - detach=True, **extra_container_kwargs) diff --git a/clipper_admin/clipper_admin/docker/docker_metric_utils.py b/clipper_admin/clipper_admin/docker/docker_metric_utils.py index 9dd034a0d..69144ae0f 100644 --- a/clipper_admin/clipper_admin/docker/docker_metric_utils.py +++ b/clipper_admin/clipper_admin/docker/docker_metric_utils.py @@ -20,7 +20,7 @@ def get_prometheus_base_config(): def run_query_frontend_metric_image(name, docker_client, query_name, frontend_exporter_image, common_labels, - extra_container_kwargs): + log_config, extra_container_kwargs): """ Use docker_client to run a frontend-exporter image. :param name: Name to pass in, need to be unique. @@ -38,7 +38,7 @@ def run_query_frontend_metric_image(name, docker_client, query_name, docker_client.containers.run( frontend_exporter_image, query_frontend_metric_cmd, - #log_config=log_config, + log_config=log_config, name=name, labels=query_frontend_metric_labels, **extra_container_kwargs) @@ -73,7 +73,7 @@ def setup_metric_config(query_frontend_metric_name, prom_config_path, def run_metric_image(docker_client, common_labels, prometheus_port, - prom_config_path, extra_container_kwargs): + prom_config_path, log_config, extra_container_kwargs): """ Run the prometheus image. :param docker_client: The docker client object @@ -97,7 +97,7 @@ def run_metric_image(docker_client, common_labels, prometheus_port, metric_cmd, name="metric_frontend-{}".format(random.randint(0, 100000)), ports={'9090/tcp': prometheus_port}, - #log_config=log_config, + log_config=log_config, volumes={ prom_config_path: { 'bind': '/etc/prometheus/prometheus.yml', diff --git a/examples/basic_query/example_client.py b/examples/basic_query/example_client.py index 797b70d6a..a5a8c6b59 100644 --- a/examples/basic_query/example_client.py +++ b/examples/basic_query/example_client.py @@ -40,7 +40,7 @@ def signal_handler(signal, frame): if __name__ == '__main__': signal.signal(signal.SIGINT, signal_handler) - clipper_conn = ClipperConnection(DockerContainerManager()) + clipper_conn = ClipperConnection(DockerContainerManager(use_centralized_log=True)) clipper_conn.start_clipper() python_deployer.create_endpoint(clipper_conn, "simple-example", "doubles", feature_sum) From 00c2d9143cfb6da7762586c2b8fbfa6ac73846f8 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Sun, 10 Mar 2019 14:20:13 -0700 Subject: [PATCH 05/20] Now we can mount a fluentd conf file to a docker. --- .../clipper_admin/container_manager.py | 1 + .../clipper_admin/docker/clipper_fluentd.conf | 8 ++ .../docker/docker_container_manager.py | 50 ++++++--- .../docker/docker_logging_utils.py | 106 ++++++++++++++++-- examples/basic_query/example_client.py | 4 +- logging/README.md | 2 +- 6 files changed, 140 insertions(+), 31 deletions(-) create mode 100644 clipper_admin/clipper_admin/docker/clipper_fluentd.conf diff --git a/clipper_admin/clipper_admin/container_manager.py b/clipper_admin/clipper_admin/container_manager.py index cc06602c1..366b8509e 100644 --- a/clipper_admin/clipper_admin/container_manager.py +++ b/clipper_admin/clipper_admin/container_manager.py @@ -27,6 +27,7 @@ 'fluentd': 'ai.clipper.fluentd.port' } CLIPPER_METRIC_CONFIG_LABEL = 'ai.clipper.metric.config' +CLIPPER_FLUENTD_CONFIG_LABEL = 'ai.clipper.fluentd.config' # NOTE: we use '_' as the delimiter because kubernetes allows the use # '_' in labels but not in deployment names. We force model names and diff --git a/clipper_admin/clipper_admin/docker/clipper_fluentd.conf b/clipper_admin/clipper_admin/docker/clipper_fluentd.conf new file mode 100644 index 000000000..c7ee7ded7 --- /dev/null +++ b/clipper_admin/clipper_admin/docker/clipper_fluentd.conf @@ -0,0 +1,8 @@ + + @type forward + port 24224 + + + + @type stdout + \ No newline at end of file diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 86382d7f2..6211e8104 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -17,17 +17,18 @@ CLIPPER_MGMT_FRONTEND_CONTAINER_LABEL, CLIPPER_INTERNAL_RPC_PORT, CLIPPER_INTERNAL_QUERY_PORT, CLIPPER_INTERNAL_MANAGEMENT_PORT, CLIPPER_INTERNAL_METRIC_PORT, CLIPPER_INTERNAL_REDIS_PORT, - CLIPPER_DOCKER_PORT_LABELS, CLIPPER_METRIC_CONFIG_LABEL, ClusterAdapter) + CLIPPER_DOCKER_PORT_LABELS, CLIPPER_METRIC_CONFIG_LABEL, ClusterAdapter, + CLIPPER_FLUENTD_CONFIG_LABEL) from ..exceptions import ClipperException from requests.exceptions import ConnectionError from .docker_metric_utils import * -from .docker_logging_utils import run_fluentd_image +from .docker_logging_utils import run_fluentd_image, FluentdConfig logger = logging.getLogger(__name__) class DockerContainerManager(ContainerManager): - # SANG-TODO Add SQLITE support + # Logging-TODO Add SQLITE support def __init__(self, cluster_name="default-cluster", docker_ip_address="localhost", @@ -78,7 +79,7 @@ def __init__(self, self.cluster_name = cluster_name self.cluster_identifier = cluster_name # For logging purpose self.public_hostname = docker_ip_address - # SANG-TODO Add SQLITE support + # Logging-TODO Add SQLITE support self.fluentd_port = fluentd_port self.clipper_query_port = clipper_query_port self.clipper_management_port = clipper_management_port @@ -118,6 +119,7 @@ def __init__(self, self.centralize_log = use_centralized_log self.log_config = {'type': 'fluentd'} if use_centralized_log \ else {'type': 'json-file'} + self.fluentd_conf_path = None self.extra_container_kwargs.update(container_args) @@ -125,7 +127,7 @@ def __init__(self, 'cluster_name': self.cluster_identifier }) - #SANG-TODO Add sqlite support + #Logging-TODO Add sqlite support def start_clipper(self, query_frontend_image, mgmt_frontend_image, @@ -164,16 +166,9 @@ def start_clipper(self, format(self.cluster_name)) if self.centralize_log: - # SQLite Logging DB - + # Logging-TODO Initialize SQLite Logging DB # Fluentd for Logging Centralization - self.logger.info("Starting Fluentd instance in Docker") - self.fluentd_port = find_unbound_port(self.fluentd_port) - fluentd_labels = self.common_labels.copy() - fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str( - self.fluentd_port) - run_fluentd_image(self.docker_client, fluentd_labels, - self.fluentd_port, self.extra_container_kwargs) + self.start_fluentd() # Redis for cluster configuration if not self.external_redis: @@ -207,7 +202,6 @@ def start_clipper(self, self.docker_client.containers.run( mgmt_frontend_image, mgmt_cmd, - # SANG-TODO log_config shouldn't be always included log_config=self.log_config, name="mgmt_frontend-{}".format(random.randint( 0, 100000)), # generate a random name @@ -302,7 +296,11 @@ def connect(self): 'query_rpc']] self.prometheus_port = all_labels[CLIPPER_DOCKER_PORT_LABELS['metric']] self.prom_config_path = all_labels[CLIPPER_METRIC_CONFIG_LABEL] - # SANG-TODO Add a Fluentd Support + + if self.centralize_log: + self.fluentd_conf_path = all_labels[CLIPPER_FLUENTD_CONFIG_LABEL] + self.fluentd_port = all_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] + # Logging-TODO Add a Sqlite support def deploy_model(self, name, version, input_type, image, num_replicas=1): # Parameters @@ -361,7 +359,7 @@ def _add_replica(self, name, version, input_type, image): model_container_name = model_container_label + '-{}'.format( random.randint(0, 100000)) - # TODO SANG - Add fluentd support + self.docker_client.containers.run( image, name=model_container_name, @@ -493,6 +491,24 @@ def get_metric_addr(self): return "{host}:{port}".format( host=self.public_hostname, port=self.prometheus_port) + def start_fluentd(self): + self.logger.info("Starting Fluentd instance in Docker cluster {cluster_name}" + .format(cluster_name=self.cluster_name)) + self.fluentd_port = find_unbound_port(self.fluentd_port) + fluentd_labels = self.common_labels.copy() + + self.fluentd_conf_path = FluentdConfig().build() + fluentd_labels[CLIPPER_FLUENTD_CONFIG_LABEL] = self.fluentd_conf_path + fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str(self.fluentd_port) + + self.logger.info( + "Fluentd Configuration Saved at {path}. " + "It will be mounted at {mounted_path} inside container" + .format(path=self.fluentd_conf_path, mounted_path=FluentdConfig.get_conf_path_within_docker())) + + run_fluentd_image(self.docker_client, fluentd_labels, + self.fluentd_port, self.fluentd_conf_path, self.extra_container_kwargs) + def find_unbound_port(start=None, increment=False, diff --git a/clipper_admin/clipper_admin/docker/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/docker_logging_utils.py index c292bf4ec..73ec91d25 100644 --- a/clipper_admin/clipper_admin/docker/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/docker_logging_utils.py @@ -1,38 +1,122 @@ import random +import tempfile +import os from ..container_manager import CLIPPER_INTERNAL_FLUENTD_PORT -FLUENTD_VERSION = 'v1.3-debian-1' +FLUENTD_VERSION = 'v1.3-debian-1' # TODO needs to be update to receive env variable like prometheus +FLUENTD_CONF_PATH_IN_DOCKER = '/fluentd/etc/fluent.conf' +FLUENTD_DEFAULT_CONF_PATH = '{current_dir}/clipper_fluentd.conf' \ + .format(current_dir=os.path.dirname(os.path.abspath(__file__))) -def run_fluentd_image(docker_client, fluentd_labels, fluend_port, extra_container_kwargs): - fluentd_cmd = '' # There's no special fluentd cmd for now +def run_fluentd_image(docker_client, fluentd_labels, fluend_port, fluentd_conf_path, extra_container_kwargs): + fluentd_cmd = [] # No cmd is required. fluentd_container_id = random.randint(0, 100000) - fluentd_name = "fluentd-{}".format(fluentd_container_id) - fluentd_img = 'fluent/fluentd:{version}'.format(version=FLUENTD_VERSION) + fluentd_name = "fluentd-{}".format(fluentd_container_id) + fluentd_img = 'fluent/fluentd:{version}'.format(version=FLUENTD_VERSION) docker_client.containers.run( fluentd_img, - fluentd_cmd, + command=fluentd_cmd, name=fluentd_name, ports={ '%s/tcp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port, '%s/udp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port }, + volumes={ + fluentd_conf_path: { + 'bind': '{conf_path_within_docker}'.format(conf_path_within_docker=FLUENTD_CONF_PATH_IN_DOCKER), + 'mode': 'rw' + } + }, labels=fluentd_labels, **extra_container_kwargs) class FluentdConfig: """ - Follow a builder pattern to contruct a fluentd configuration file, fluentd.conf + Class to build a fluentd config file. + + EX) FluentdConfig() # will build an initial conf file with initial configuration. + .set_forward_address(OTHER_FLUENTD_ADDRESS, OTHER_FLUENTD_PORT) + .set_directory(DIR_NAME) + .set_file_name(FILE_NAME) + .build() """ - def __init__(self): - self.conf = { + def __init__(self, customized_conf_file=False): + """ + @param customized_conf_file: Decide whether or not to provide customized configuration file. + If false, it will use clipper default conf file + """ + self.conf = self.get_base_config() + self._file_path = self.build_temp_file() + + def set_forward_address(self, address, port): + """ + Set the port number and address of external fluentd instance (internal is the clipper fluentd) + in which centralized logs will be forwarded + + @param port: port number to forward logs + """ + raise NotImplementedError("set_forward_address is not implemented yet. It will be coming soon.") - } + def set_directory(self, dir): + raise NotImplementedError("set_directory is not implemented yet. It will be coming soon.") + + def set_file_name(self, name): + raise NotImplementedError("set_file_name is not implemented yet. It will be coming soon.") + + def provide_customized_file(self, file_path): + """Provide a customized fluentd conf file.""" + raise NotImplementedError("provide_customized_file is not implemented yet. It will be coming soon.") def build(self): - pass + """ + Build a fluentd configuration file and return the path of it. + fluentd_default_conf_path will be stored in clipper_admin/docker folder + and used to write the initial conf file. + + Build should be called only once to build an initial conf file. + Developers can customize conf file written in the self.file_path using defined interfaces. + + TODO: Interfaces for modifying fluentd config file. + TODO: Interface for providing customized fluentd file + instead of building a file using default fluentd conf file. + + @return: Path of fluentd config file that will be sync with Docker container. + """ + if self._file_path is None \ + or not os.path.isfile(self._file_path): + self._file_path = self.build_temp_file() + + # Logging-TODO Currently, it copies the default conf from clipper_fluentd.conf. + # We need a way to customize it. + with open(FLUENTD_DEFAULT_CONF_PATH, 'r') as default_conf_file: + with open(self._file_path, 'w') as fleutnd_conf: + for line in default_conf_file: + fleutnd_conf.write(line) + + return self._file_path + + @property + def get_temp_file_path(self): + return self._file_path + + @staticmethod + def get_base_config(self): + return {} + + @staticmethod + def get_conf_path_within_docker(): + return FLUENTD_CONF_PATH_IN_DOCKER + + @staticmethod + def build_temp_file(): + file_path = tempfile.NamedTemporaryFile( + 'w', suffix='.conf', delete=False).name + file_path = os.path.realpath( + file_path) # resolve symlink + return file_path diff --git a/examples/basic_query/example_client.py b/examples/basic_query/example_client.py index a5a8c6b59..b84e01a20 100644 --- a/examples/basic_query/example_client.py +++ b/examples/basic_query/example_client.py @@ -34,7 +34,7 @@ def feature_sum(xs): def signal_handler(signal, frame): print("Stopping Clipper...") clipper_conn = ClipperConnection(DockerContainerManager()) - clipper_conn.stop_all() + clipper_conn.stop_all(graceful=False) sys.exit(0) @@ -58,6 +58,6 @@ def signal_handler(signal, frame): batch=True) else: predict(clipper_conn.get_query_addr(), np.random.random(200)) - time.sleep(0.2) + time.sleep(2) except Exception as e: clipper_conn.stop_all() \ No newline at end of file diff --git a/logging/README.md b/logging/README.md index 926751485..63c31bf67 100644 --- a/logging/README.md +++ b/logging/README.md @@ -1,2 +1,2 @@ -# Clipper Logging Architecture with Fluentd +# Clipper Logging with Fluentd From 3b5b1cacec6f2ac4f7b44ab8a92a2b3be9d3407d Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Sun, 10 Mar 2019 22:18:21 -0700 Subject: [PATCH 06/20] Changed pydocstring style --- clipper_admin/clipper_admin/docker/docker_logging_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clipper_admin/clipper_admin/docker/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/docker_logging_utils.py index 73ec91d25..ad2de261c 100644 --- a/clipper_admin/clipper_admin/docker/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/docker_logging_utils.py @@ -47,7 +47,7 @@ class FluentdConfig: """ def __init__(self, customized_conf_file=False): """ - @param customized_conf_file: Decide whether or not to provide customized configuration file. + :param customized_conf_file: Decide whether or not to provide customized configuration file. If false, it will use clipper default conf file """ self.conf = self.get_base_config() @@ -58,7 +58,7 @@ def set_forward_address(self, address, port): Set the port number and address of external fluentd instance (internal is the clipper fluentd) in which centralized logs will be forwarded - @param port: port number to forward logs + :param port: port number to forward logs """ raise NotImplementedError("set_forward_address is not implemented yet. It will be coming soon.") @@ -85,7 +85,7 @@ def build(self): TODO: Interface for providing customized fluentd file instead of building a file using default fluentd conf file. - @return: Path of fluentd config file that will be sync with Docker container. + :return: Path of fluentd config file that will be sync with Docker container. """ if self._file_path is None \ or not os.path.isfile(self._file_path): From b255c4b5f1dc1acfb5449302372fa12beffd6ca8 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Mon, 18 Mar 2019 19:57:44 -0700 Subject: [PATCH 07/20] Cleaned up some styles --- .../docker/docker_container_manager.py | 2 +- .../docker/docker_logging_utils.py | 23 +++++++++---------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 6211e8104..70fb4d8d6 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -504,7 +504,7 @@ def start_fluentd(self): self.logger.info( "Fluentd Configuration Saved at {path}. " "It will be mounted at {mounted_path} inside container" - .format(path=self.fluentd_conf_path, mounted_path=FluentdConfig.get_conf_path_within_docker())) + .format(path=self.fluentd_conf_path, mounted_path=FluentdConfig.conf_path_within_docker)) run_fluentd_image(self.docker_client, fluentd_labels, self.fluentd_port, self.fluentd_conf_path, self.extra_container_kwargs) diff --git a/clipper_admin/clipper_admin/docker/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/docker_logging_utils.py index ad2de261c..f1181adb9 100644 --- a/clipper_admin/clipper_admin/docker/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/docker_logging_utils.py @@ -50,7 +50,7 @@ def __init__(self, customized_conf_file=False): :param customized_conf_file: Decide whether or not to provide customized configuration file. If false, it will use clipper default conf file """ - self.conf = self.get_base_config() + self.conf = self.base_config self._file_path = self.build_temp_file() def set_forward_address(self, address, port): @@ -82,16 +82,15 @@ def build(self): Developers can customize conf file written in the self.file_path using defined interfaces. TODO: Interfaces for modifying fluentd config file. - TODO: Interface for providing customized fluentd file - instead of building a file using default fluentd conf file. + TODO: Interfaces for providing customized fluentd file - :return: Path of fluentd config file that will be sync with Docker container. + :return: Path of fluentd config file in which Fluentd container mounts on. """ if self._file_path is None \ or not os.path.isfile(self._file_path): self._file_path = self.build_temp_file() - # Logging-TODO Currently, it copies the default conf from clipper_fluentd.conf. + # Logging-TODO: Currently, it copies the default conf from clipper_fluentd.conf. # We need a way to customize it. with open(FLUENTD_DEFAULT_CONF_PATH, 'r') as default_conf_file: with open(self._file_path, 'w') as fleutnd_conf: @@ -101,17 +100,17 @@ def build(self): return self._file_path @property - def get_temp_file_path(self): + def temp_file_path(self): return self._file_path - @staticmethod - def get_base_config(self): - return {} - - @staticmethod - def get_conf_path_within_docker(): + @property + def conf_path_within_docker(self): return FLUENTD_CONF_PATH_IN_DOCKER + @property + def base_config(self): + return {} + @staticmethod def build_temp_file(): file_path = tempfile.NamedTemporaryFile( From 90d480a68ddc90c6a5e61e8f4d2ea92c21c24b6d Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Thu, 21 Mar 2019 00:46:46 -0700 Subject: [PATCH 08/20] Created an integration test --- .../docker/docker_container_manager.py | 66 ++++----- .../clipper_admin/docker/logging}/README.md | 0 .../clipper_admin/docker/logging/__init__.py | 0 .../docker/{ => logging}/clipper_fluentd.conf | 0 .../{ => logging}/docker_logging_utils.py | 65 ++++++-- examples/basic_query/example_client.py | 4 +- integration-tests/clipper_admin_tests.py | 29 ++-- .../clipper_fluentd_logging_docker.py | 139 ++++++++++++++++++ integration-tests/test_utils.py | 8 +- 9 files changed, 243 insertions(+), 68 deletions(-) rename {logging => clipper_admin/clipper_admin/docker/logging}/README.md (100%) create mode 100644 clipper_admin/clipper_admin/docker/logging/__init__.py rename clipper_admin/clipper_admin/docker/{ => logging}/clipper_fluentd.conf (100%) rename clipper_admin/clipper_admin/docker/{ => logging}/docker_logging_utils.py (66%) create mode 100644 integration-tests/clipper_fluentd_logging_docker.py diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 70fb4d8d6..10bda7552 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -5,24 +5,24 @@ import docker import logging import os -import sys -import random import time -import json import tempfile from ..container_manager import ( create_model_container_label, parse_model_container_label, ContainerManager, CLIPPER_DOCKER_LABEL, CLIPPER_MODEL_CONTAINER_LABEL, CLIPPER_QUERY_FRONTEND_CONTAINER_LABEL, CLIPPER_MGMT_FRONTEND_CONTAINER_LABEL, CLIPPER_INTERNAL_RPC_PORT, - CLIPPER_INTERNAL_QUERY_PORT, CLIPPER_INTERNAL_MANAGEMENT_PORT, + CLIPPER_INTERNAL_MANAGEMENT_PORT, CLIPPER_INTERNAL_METRIC_PORT, CLIPPER_INTERNAL_REDIS_PORT, CLIPPER_DOCKER_PORT_LABELS, CLIPPER_METRIC_CONFIG_LABEL, ClusterAdapter, CLIPPER_FLUENTD_CONFIG_LABEL) -from ..exceptions import ClipperException from requests.exceptions import ConnectionError from .docker_metric_utils import * -from .docker_logging_utils import run_fluentd_image, FluentdConfig +from clipper_admin.docker.logging.docker_logging_utils import ( + run_fluentd_image, FluentdConfig, + get_logs_from_containers, get_centralized_logs, + get_fluentd_log_config +) logger = logging.getLogger(__name__) @@ -117,8 +117,11 @@ def __init__(self, # Logging centralization args. Currently, fluentd is used to centralize logs self.centralize_log = use_centralized_log - self.log_config = {'type': 'fluentd'} if use_centralized_log \ - else {'type': 'json-file'} + if use_centralized_log: + self.log_config = get_fluentd_log_config() + else: + self.log_config= {'type': 'json-file'} + self.fluentd_conf_path = None self.extra_container_kwargs.update(container_args) @@ -127,7 +130,6 @@ def __init__(self, 'cluster_name': self.cluster_identifier }) - #Logging-TODO Add sqlite support def start_clipper(self, query_frontend_image, mgmt_frontend_image, @@ -297,9 +299,11 @@ def connect(self): self.prometheus_port = all_labels[CLIPPER_DOCKER_PORT_LABELS['metric']] self.prom_config_path = all_labels[CLIPPER_METRIC_CONFIG_LABEL] - if self.centralize_log: + if CLIPPER_DOCKER_PORT_LABELS['fluentd'] in all_labels: + self.centralize_log= True self.fluentd_conf_path = all_labels[CLIPPER_FLUENTD_CONFIG_LABEL] self.fluentd_port = all_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] + self.log_config = get_fluentd_log_config() # Logging-TODO Add a Sqlite support def deploy_model(self, name, version, input_type, image, num_replicas=1): @@ -365,6 +369,7 @@ def _add_replica(self, name, version, input_type, image): name=model_container_name, environment=env_vars, labels=labels, + log_config=self.log_config, **self.extra_container_kwargs) # Metric Section @@ -416,30 +421,10 @@ def set_num_replicas(self, name, version, input_type, image, num_replicas): self.prometheus_port) def get_logs(self, logging_dir): - containers = self.docker_client.containers.list( - filters={ - "label": - "{key}={val}".format( - key=CLIPPER_DOCKER_LABEL, val=self.cluster_name) - }) - logging_dir = os.path.abspath(os.path.expanduser(logging_dir)) - - log_files = [] - if not os.path.exists(logging_dir): - os.makedirs(logging_dir) - self.logger.info("Created logging directory: %s" % logging_dir) - for c in containers: - log_file_name = "image_{image}:container_{id}.log".format( - image=c.image.short_id, id=c.short_id) - log_file = os.path.join(logging_dir, log_file_name) - if sys.version_info < (3, 0): - with open(log_file, "w") as lf: - lf.write(c.logs(stdout=True, stderr=True)) - else: - with open(log_file, "wb") as lf: - lf.write(c.logs(stdout=True, stderr=True)) - log_files.append(log_file) - return log_files + if self.centralize_log: + get_centralized_logs(logging_dir) + else: + get_logs_from_containers(self, logging_dir) def stop_models(self, models): containers = self.docker_client.containers.list( @@ -491,23 +476,28 @@ def get_metric_addr(self): return "{host}:{port}".format( host=self.public_hostname, port=self.prometheus_port) + def get_fluentd_addr(self): + return "{host}:{port}".format( + host='98.207.50.3', port=self.fluentd_port + ) + def start_fluentd(self): self.logger.info("Starting Fluentd instance in Docker cluster {cluster_name}" .format(cluster_name=self.cluster_name)) self.fluentd_port = find_unbound_port(self.fluentd_port) - fluentd_labels = self.common_labels.copy() + self.fluentd_conf_path = FluentdConfig().build(self.fluentd_port) - self.fluentd_conf_path = FluentdConfig().build() + fluentd_labels = self.common_labels.copy() fluentd_labels[CLIPPER_FLUENTD_CONFIG_LABEL] = self.fluentd_conf_path fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str(self.fluentd_port) self.logger.info( "Fluentd Configuration Saved at {path}. " "It will be mounted at {mounted_path} inside container" - .format(path=self.fluentd_conf_path, mounted_path=FluentdConfig.conf_path_within_docker)) + .format(path=self.fluentd_conf_path, mounted_path=str(FluentdConfig.get_conf_path_within_docker()))) run_fluentd_image(self.docker_client, fluentd_labels, - self.fluentd_port, self.fluentd_conf_path, self.extra_container_kwargs) + self.fluentd_port, self.fluentd_conf_path, self.extra_container_kwargs) def find_unbound_port(start=None, diff --git a/logging/README.md b/clipper_admin/clipper_admin/docker/logging/README.md similarity index 100% rename from logging/README.md rename to clipper_admin/clipper_admin/docker/logging/README.md diff --git a/clipper_admin/clipper_admin/docker/logging/__init__.py b/clipper_admin/clipper_admin/docker/logging/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/clipper_admin/clipper_admin/docker/clipper_fluentd.conf b/clipper_admin/clipper_admin/docker/logging/clipper_fluentd.conf similarity index 100% rename from clipper_admin/clipper_admin/docker/clipper_fluentd.conf rename to clipper_admin/clipper_admin/docker/logging/clipper_fluentd.conf diff --git a/clipper_admin/clipper_admin/docker/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py similarity index 66% rename from clipper_admin/clipper_admin/docker/docker_logging_utils.py rename to clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py index f1181adb9..7cb2da856 100644 --- a/clipper_admin/clipper_admin/docker/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py @@ -1,8 +1,9 @@ import random import tempfile import os +import sys -from ..container_manager import CLIPPER_INTERNAL_FLUENTD_PORT +from clipper_admin.container_manager import CLIPPER_INTERNAL_FLUENTD_PORT, CLIPPER_DOCKER_LABEL FLUENTD_VERSION = 'v1.3-debian-1' # TODO needs to be update to receive env variable like prometheus @@ -13,8 +14,7 @@ def run_fluentd_image(docker_client, fluentd_labels, fluend_port, fluentd_conf_path, extra_container_kwargs): fluentd_cmd = [] # No cmd is required. - fluentd_container_id = random.randint(0, 100000) - fluentd_name = "fluentd-{}".format(fluentd_container_id) + fluentd_name = "fluentd-{}".format(random.randint(0, 100000)) fluentd_img = 'fluent/fluentd:{version}'.format(version=FLUENTD_VERSION) docker_client.containers.run( @@ -22,8 +22,8 @@ def run_fluentd_image(docker_client, fluentd_labels, fluend_port, fluentd_conf_p command=fluentd_cmd, name=fluentd_name, ports={ - '%s/tcp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port, - '%s/udp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port + '%s/tcp' % fluend_port : CLIPPER_INTERNAL_FLUENTD_PORT, + '%s/udp' % fluend_port : CLIPPER_INTERNAL_FLUENTD_PORT }, volumes={ fluentd_conf_path: { @@ -35,6 +35,43 @@ def run_fluentd_image(docker_client, fluentd_labels, fluend_port, fluentd_conf_p **extra_container_kwargs) +def get_fluentd_log_config(): + return { + 'type': 'fluentd', + } + + +def get_centralized_logs(logging_dir): + raise NotImplementedError("Centralized log collection is not implemented yet. It is currently in beta.") + + +def get_logs_from_containers(docker_container_manager, logging_dir): + containers = docker_container_manager.docker_client.containers.list( + filters={ + "label": + "{key}={val}".format( + key=CLIPPER_DOCKER_LABEL, val=docker_container_manager.cluster_name) + }) + logging_dir = os.path.abspath(os.path.expanduser(logging_dir)) + + log_files = [] + if not os.path.exists(logging_dir): + os.makedirs(logging_dir) + docker_container_manager.logger.info("Created logging directory: %s" % logging_dir) + for c in containers: + log_file_name = "image_{image}:container_{id}.log".format( + image=c.image.short_id, id=c.short_id) + log_file = os.path.join(logging_dir, log_file_name) + if sys.version_info < (3, 0): + with open(log_file, "w") as lf: + lf.write(c.logs(stdout=True, stderr=True)) + else: + with open(log_file, "wb") as lf: + lf.write(c.logs(stdout=True, stderr=True)) + log_files.append(log_file) + return log_files + + class FluentdConfig: """ Class to build a fluentd config file. @@ -72,7 +109,7 @@ def provide_customized_file(self, file_path): """Provide a customized fluentd conf file.""" raise NotImplementedError("provide_customized_file is not implemented yet. It will be coming soon.") - def build(self): + def build(self, fluentd_port): """ Build a fluentd configuration file and return the path of it. fluentd_default_conf_path will be stored in clipper_admin/docker folder @@ -93,9 +130,13 @@ def build(self): # Logging-TODO: Currently, it copies the default conf from clipper_fluentd.conf. # We need a way to customize it. with open(FLUENTD_DEFAULT_CONF_PATH, 'r') as default_conf_file: - with open(self._file_path, 'w') as fleutnd_conf: + with open(self._file_path, 'w') as fluetnd_conf: for line in default_conf_file: - fleutnd_conf.write(line) + # port number in a conf file should be the same as container manager's port number + if 'port' in line: + fluetnd_conf.write(' port {}\n'.format(fluentd_port)) + else: + fluetnd_conf.write(line) return self._file_path @@ -103,14 +144,14 @@ def build(self): def temp_file_path(self): return self._file_path - @property - def conf_path_within_docker(self): - return FLUENTD_CONF_PATH_IN_DOCKER - @property def base_config(self): return {} + @staticmethod + def get_conf_path_within_docker(): + return FLUENTD_CONF_PATH_IN_DOCKER + @staticmethod def build_temp_file(): file_path = tempfile.NamedTemporaryFile( diff --git a/examples/basic_query/example_client.py b/examples/basic_query/example_client.py index b84e01a20..7324820ba 100644 --- a/examples/basic_query/example_client.py +++ b/examples/basic_query/example_client.py @@ -34,13 +34,13 @@ def feature_sum(xs): def signal_handler(signal, frame): print("Stopping Clipper...") clipper_conn = ClipperConnection(DockerContainerManager()) - clipper_conn.stop_all(graceful=False) + clipper_conn.stop_all() sys.exit(0) if __name__ == '__main__': signal.signal(signal.SIGINT, signal_handler) - clipper_conn = ClipperConnection(DockerContainerManager(use_centralized_log=True)) + clipper_conn = ClipperConnection(DockerContainerManager()) clipper_conn.start_clipper() python_deployer.create_endpoint(clipper_conn, "simple-example", "doubles", feature_sum) diff --git a/integration-tests/clipper_admin_tests.py b/integration-tests/clipper_admin_tests.py index 23dbbe4a0..637eb7033 100644 --- a/integration-tests/clipper_admin_tests.py +++ b/integration-tests/clipper_admin_tests.py @@ -40,7 +40,7 @@ class ClipperManagerTestCaseShort(unittest.TestCase): def setUp(self): new_name = "admin-test-cluster-{}".format(random.randint(0, 5000)) self.clipper_conn = create_docker_connection( - cleanup=False, start_clipper=True, new_name=new_name) + cleanup=False, start_clipper=True, new_name=new_name, use_centralized_log=False) self.name = new_name def tearDown(self): @@ -177,18 +177,19 @@ def test_model_version_sets_correctly(self): self.assertTrue(models_list_contains_correct_version) def test_get_logs_creates_log_files(self): - if not os.path.exists(cl.CLIPPER_TEMP_DIR): - os.makedirs(cl.CLIPPER_TEMP_DIR) - tmp_log_dir = tempfile.mkdtemp(dir=cl.CLIPPER_TEMP_DIR) - log_file_names = self.clipper_conn.get_clipper_logs( - logging_dir=tmp_log_dir) - self.assertIsNotNone(log_file_names) - self.assertGreaterEqual(len(log_file_names), 1) - for file_name in log_file_names: - self.assertTrue(os.path.isfile(file_name)) - - # Remove temp files - shutil.rmtree(tmp_log_dir) + if not self.clipper_conn.cm.centralize_log: + if not os.path.exists(cl.CLIPPER_TEMP_DIR): + os.makedirs(cl.CLIPPER_TEMP_DIR) + tmp_log_dir = tempfile.mkdtemp(dir=cl.CLIPPER_TEMP_DIR) + log_file_names = self.clipper_conn.get_clipper_logs( + logging_dir=tmp_log_dir) + self.assertIsNotNone(log_file_names) + self.assertGreaterEqual(len(log_file_names), 1) + for file_name in log_file_names: + self.assertTrue(os.path.isfile(file_name)) + + # Remove temp files + shutil.rmtree(tmp_log_dir) def test_inspect_instance_returns_json_dict(self): metrics = self.clipper_conn.inspect_instance() @@ -833,7 +834,7 @@ def test_remove_inactive_container(self): 'test_test_predict_function', 'test_build_model_with_custom_packages', 'test_delete_application_correct', - 'test_query_specific_model_version', + 'test_query_specific_model_version' ] LONG_TEST_ORDERING = [ diff --git a/integration-tests/clipper_fluentd_logging_docker.py b/integration-tests/clipper_fluentd_logging_docker.py new file mode 100644 index 000000000..c396dcbdd --- /dev/null +++ b/integration-tests/clipper_fluentd_logging_docker.py @@ -0,0 +1,139 @@ +from __future__ import print_function + +import json +import logging +import os +import subprocess +import sys +import time +import random + +import numpy as np +import requests +import yaml +from test_utils import log_clipper_state, create_docker_connection, get_docker_client + +cur_dir = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.abspath("%s/../clipper_admin" % cur_dir)) +from clipper_admin.deployers import python as python_deployer +from clipper_admin.container_manager import CLIPPER_DOCKER_LABEL + + +def predict(addr, x): + url = "http://%s/simple-example/predict" % addr + req_json = json.dumps({'input': list(x)}) + headers = {'Content-type': 'application/json'} + r = requests.post(url, headers=headers, data=req_json) + + +def feature_sum(xs): + return [str(sum(x)) for x in xs] + + +def setup(clipper_conn): + docker_client = get_docker_client() + containers = docker_client.containers.list( + filters={ + 'label': [ + '{key}={val}'.format( + key=CLIPPER_DOCKER_LABEL, val=clipper_conn.cm.cluster_name) + ] + }) + + fluentd_container = None + for c in containers: + if 'fluentd-' in c.name: + fluentd_container = c + + if fluentd_container is None: + raise AssertionError("Fluentd has not been running") + + return fluentd_container + + +def check_fluentd_has_correct_logs(clipper_conn): + fluentd_container = setup(clipper_conn) + fluentd_logs = str(fluentd_container.logs()) + + if not check_query_frontend_logs(fluentd_logs): + raise AssertionError("Query Frontend log is not found") + if not check_metric_frontend_logs(fluentd_logs): + raise AssertionError("Metric Frontend log is not found") + if not check_redis_logs(fluentd_logs): + raise AssertionError("Redis log is not found") + +def check_fluentd_has_correct_model_logs(clipper_conn, model_name): + fluentd_container = setup(clipper_conn) + fluentd_logs = str(fluentd_container.logs()) + + if not check_model_logs(fluentd_logs, model_name): + raise AssertionError("{model_name} log is not found".format(model_name=model_name)) + + +def check_query_frontend_logs(fluentd_logs): + return '"container_name":"/query_frontend' in fluentd_logs + + +def check_metric_frontend_logs(fluentd_logs): + return '"container_name":"/mgmt_frontend' in fluentd_logs + + +def check_redis_logs(fluentd_logs): + return '"container_name":"/redis' in fluentd_logs + + +def check_model_logs(fluentd_logs, model_name): + return '"container_name":"/{}',format(model_name) in fluentd_logs + + +def log_docker_ps(clipper_conn): + container_runing = clipper_conn.cm.docker_client.containers.list() + logger.info('Current docker status') + for cont in container_runing: + logger.info('Name {}, Image {}, Status {}, Label {}'.format( + cont.name, cont.image, cont.status, cont.labels)) + + +if __name__ == '__main__': + logging.basicConfig( + format= + '%(asctime)s %(levelname)-8s [%(filename)s:%(lineno)d] %(message)s', + datefmt='%y-%m-%d:%H:%M:%S', + level=0) + + logger = logging.getLogger(__name__) + + logger.info("Start Fluentd Test (0/2): Running 2 Replicas") + + cluster_name = "fluentd-test-{}".format(random.randint(0, 50000)) + clipper_conn = create_docker_connection( + cleanup=False, start_clipper=True, new_name=cluster_name, use_centralized_log=True) + python_deployer.create_endpoint( + clipper_conn, "simple-example", "doubles", feature_sum, num_replicas=2) + time.sleep(2) + + try: + logger.info("Test 1: Checking if fluentd has correct logs") + check_fluentd_has_correct_logs(clipper_conn) + logger.info("Fluentd Test (1/2): Test 1 passed") + + logger.info( + "Making 100 predictions using two model container; Should takes 25 seconds." + ) + for _ in range(100): + predict(clipper_conn.get_query_addr(), np.random.random(200)) + time.sleep(0.2) + + logger.info("Test 2: Checking if fluentd has correct model logs") + check_fluentd_has_correct_model_logs(clipper_conn, 'simple-example') + logger.info("Fluentd Test (2/2): Test 2 passed") + + create_docker_connection( + cleanup=True, start_clipper=False, cleanup_name=cluster_name) + except Exception as e: + logger.info("Test failed") + #log_docker_ps(clipper_conn) + logger.error(e) + #log_clipper_state(clipper_conn) + clipper_conn.stop_all(graceful=False) + sys.exit(1) diff --git a/integration-tests/test_utils.py b/integration-tests/test_utils.py index ae63af1a3..e9e0d2f35 100644 --- a/integration-tests/test_utils.py +++ b/integration-tests/test_utils.py @@ -71,7 +71,8 @@ def find_unbound_port(): def create_docker_connection(cleanup=False, start_clipper=False, cleanup_name='default-cluster', - new_name='default-cluster'): + new_name='default-cluster', + use_centralized_log=False): logger.info("Creating DockerContainerManager") cl = None assert cleanup or start_clipper, "You must set at least one of {cleanup, start_clipper} to be true." @@ -99,7 +100,10 @@ def create_docker_connection(cleanup=False, clipper_query_port=find_unbound_port(), clipper_management_port=find_unbound_port(), clipper_rpc_port=find_unbound_port(), - redis_port=find_unbound_port()) + fluentd_port=find_unbound_port(), + redis_port=find_unbound_port(), + use_centralized_log=use_centralized_log + ) cl = ClipperConnection(cm) try: logger.info("Starting Clipper") From bb6a990e7ec1e8ff764ea8092c0680840c69f260 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Thu, 21 Mar 2019 01:10:55 -0700 Subject: [PATCH 09/20] Cleaned up some part of code --- .../clipper_admin/docker/docker_container_manager.py | 5 ----- .../docker/logging/docker_logging_utils.py | 11 +---------- integration-tests/clipper_admin_tests.py | 2 +- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 10bda7552..b8f7f78b5 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -476,11 +476,6 @@ def get_metric_addr(self): return "{host}:{port}".format( host=self.public_hostname, port=self.prometheus_port) - def get_fluentd_addr(self): - return "{host}:{port}".format( - host='98.207.50.3', port=self.fluentd_port - ) - def start_fluentd(self): self.logger.info("Starting Fluentd instance in Docker cluster {cluster_name}" .format(cluster_name=self.cluster_name)) diff --git a/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py index 7cb2da856..a5588bba9 100644 --- a/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py @@ -99,16 +99,6 @@ def set_forward_address(self, address, port): """ raise NotImplementedError("set_forward_address is not implemented yet. It will be coming soon.") - def set_directory(self, dir): - raise NotImplementedError("set_directory is not implemented yet. It will be coming soon.") - - def set_file_name(self, name): - raise NotImplementedError("set_file_name is not implemented yet. It will be coming soon.") - - def provide_customized_file(self, file_path): - """Provide a customized fluentd conf file.""" - raise NotImplementedError("provide_customized_file is not implemented yet. It will be coming soon.") - def build(self, fluentd_port): """ Build a fluentd configuration file and return the path of it. @@ -121,6 +111,7 @@ def build(self, fluentd_port): TODO: Interfaces for modifying fluentd config file. TODO: Interfaces for providing customized fluentd file + :param fluentd_port: External fluentd port in which fluentd with this conf file listens to :return: Path of fluentd config file in which Fluentd container mounts on. """ if self._file_path is None \ diff --git a/integration-tests/clipper_admin_tests.py b/integration-tests/clipper_admin_tests.py index 637eb7033..5719cdaaa 100644 --- a/integration-tests/clipper_admin_tests.py +++ b/integration-tests/clipper_admin_tests.py @@ -40,7 +40,7 @@ class ClipperManagerTestCaseShort(unittest.TestCase): def setUp(self): new_name = "admin-test-cluster-{}".format(random.randint(0, 5000)) self.clipper_conn = create_docker_connection( - cleanup=False, start_clipper=True, new_name=new_name, use_centralized_log=False) + cleanup=False, start_clipper=True, new_name=new_name, use_centralized_log=True) self.name = new_name def tearDown(self): From 09fc0897ddd691395e5cb92b95ebd854237535c1 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Thu, 21 Mar 2019 11:54:50 -0700 Subject: [PATCH 10/20] Refactor done --- bin/shipyard/clipper_test.cfg.py | 1 + .../docker/docker_container_manager.py | 94 ++++++----- .../clipper_admin/docker/logging/README.md | 50 ++++++ .../docker/logging/docker_logging_utils.py | 120 +------------ .../{ => fluentd}/clipper_fluentd.conf | 0 .../docker/logging/fluentd/fluentd.py | 157 ++++++++++++++++++ examples/basic_query/example_client.py | 4 +- integration-tests/clipper_admin_tests.py | 2 +- .../clipper_fluentd_logging_docker.py | 87 ++++++++-- 9 files changed, 339 insertions(+), 176 deletions(-) rename clipper_admin/clipper_admin/docker/logging/{ => fluentd}/clipper_fluentd.conf (100%) create mode 100644 clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py diff --git a/bin/shipyard/clipper_test.cfg.py b/bin/shipyard/clipper_test.cfg.py index cd889e36d..288365bd5 100644 --- a/bin/shipyard/clipper_test.cfg.py +++ b/bin/shipyard/clipper_test.cfg.py @@ -14,6 +14,7 @@ DOCKER_INTEGRATION_TESTS = { "admin_unit_test": "python /clipper/integration-tests/clipper_admin_tests.py", + "fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", "many_apps_many_models": "python /clipper/integration-tests/many_apps_many_models.py", "pyspark": "python /clipper/integration-tests/deploy_pyspark_models.py", "pyspark_pipeline": "python /clipper/integration-tests/deploy_pyspark_pipeline_models.py", diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index b8f7f78b5..e43aab8a3 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -19,10 +19,10 @@ from requests.exceptions import ConnectionError from .docker_metric_utils import * from clipper_admin.docker.logging.docker_logging_utils import ( - run_fluentd_image, FluentdConfig, - get_logs_from_containers, get_centralized_logs, - get_fluentd_log_config + get_logs_from_containers, + get_default_log_config ) +from clipper_admin.docker.logging.fluentd.fluentd import Fluentd logger = logging.getLogger(__name__) @@ -79,8 +79,6 @@ def __init__(self, self.cluster_name = cluster_name self.cluster_identifier = cluster_name # For logging purpose self.public_hostname = docker_ip_address - # Logging-TODO Add SQLITE support - self.fluentd_port = fluentd_port self.clipper_query_port = clipper_query_port self.clipper_management_port = clipper_management_port self.clipper_rpc_port = clipper_rpc_port @@ -91,6 +89,7 @@ def __init__(self, self.external_redis = True self.redis_port = redis_port self.prometheus_port = prometheus_port + self.centralize_log = use_centralized_log if docker_network is "host": raise ClipperException( "DockerContainerManager does not support running Clipper on the " @@ -115,21 +114,27 @@ def __init__(self, "detach": True, } - # Logging centralization args. Currently, fluentd is used to centralize logs - self.centralize_log = use_centralized_log - if use_centralized_log: - self.log_config = get_fluentd_log_config() - else: - self.log_config= {'type': 'json-file'} - - self.fluentd_conf_path = None - self.extra_container_kwargs.update(container_args) self.logger = ClusterAdapter(logger, { 'cluster_name': self.cluster_identifier }) + # Setting Docker cluster logging. + # Logging-TODO Add SQLITE support + self.logging_system = Fluentd + self.log_config = get_default_log_config() + self.logging_system_instance = None + + if self.centralize_log: + self.logging_system_instance = self.logging_system( + self.logger, + self.cluster_name, + self.docker_client, + port=find_unbound_port(fluentd_port) + ) + self.log_config = self.logging_system.get_log_config() + def start_clipper(self, query_frontend_image, mgmt_frontend_image, @@ -169,8 +174,7 @@ def start_clipper(self, if self.centralize_log: # Logging-TODO Initialize SQLite Logging DB - # Fluentd for Logging Centralization - self.start_fluentd() + self.logging_system_instance.start(self.common_labels, self.extra_container_kwargs) # Redis for cluster configuration if not self.external_redis: @@ -299,11 +303,17 @@ def connect(self): self.prometheus_port = all_labels[CLIPPER_DOCKER_PORT_LABELS['metric']] self.prom_config_path = all_labels[CLIPPER_METRIC_CONFIG_LABEL] - if CLIPPER_DOCKER_PORT_LABELS['fluentd'] in all_labels: + if self._is_valid_logging_state_to_connect(all_labels): self.centralize_log= True - self.fluentd_conf_path = all_labels[CLIPPER_FLUENTD_CONFIG_LABEL] - self.fluentd_port = all_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] - self.log_config = get_fluentd_log_config() + self.logging_system_instance = \ + self.logging_system( + self.logger, + self.cluster_name, + self.docker_client, + port=all_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']], + conf_path=all_labels[CLIPPER_FLUENTD_CONFIG_LABEL] + ) + self.log_config = self.logging_system.get_log_config() # Logging-TODO Add a Sqlite support def deploy_model(self, name, version, input_type, image, num_replicas=1): @@ -422,9 +432,9 @@ def set_num_replicas(self, name, version, input_type, image, num_replicas): def get_logs(self, logging_dir): if self.centralize_log: - get_centralized_logs(logging_dir) + return self.logging_system_instance.get_logs(logging_dir) else: - get_logs_from_containers(self, logging_dir) + return get_logs_from_containers(self, logging_dir) def stop_models(self, models): containers = self.docker_client.containers.list( @@ -464,6 +474,26 @@ def stop_all(self, graceful=True): else: c.kill() + def _is_valid_logging_state_to_connect(self, all_labels): + if self.centralize_log and CLIPPER_DOCKER_PORT_LABELS['fluentd'] not in all_labels: + raise ConnectionError( + "Invalid state detected. " + "log centralization is {log_centralization_state}, " + "but cannot find fluentd instance running." + "Please change your use_centralized_log parameter of DockerContainermanager" + .format(log_centralization_state=self.centralize_log) + ) + elif CLIPPER_DOCKER_PORT_LABELS['fluentd'] in all_labels and not self.centralize_log: + raise ConnectionError( + "Invalid state detected. " + "Fluentd instance is running, " + "but log centralization state is {log_centralization_state}." + "Please change your use_centralized_log parameter of DockerContainermanager" + .format(log_centralization_state=self.centralize_log) + ) + else: + return CLIPPER_DOCKER_PORT_LABELS['fluentd'] in all_labels + def get_admin_addr(self): return "{host}:{port}".format( host=self.public_hostname, port=self.clipper_management_port) @@ -476,24 +506,6 @@ def get_metric_addr(self): return "{host}:{port}".format( host=self.public_hostname, port=self.prometheus_port) - def start_fluentd(self): - self.logger.info("Starting Fluentd instance in Docker cluster {cluster_name}" - .format(cluster_name=self.cluster_name)) - self.fluentd_port = find_unbound_port(self.fluentd_port) - self.fluentd_conf_path = FluentdConfig().build(self.fluentd_port) - - fluentd_labels = self.common_labels.copy() - fluentd_labels[CLIPPER_FLUENTD_CONFIG_LABEL] = self.fluentd_conf_path - fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str(self.fluentd_port) - - self.logger.info( - "Fluentd Configuration Saved at {path}. " - "It will be mounted at {mounted_path} inside container" - .format(path=self.fluentd_conf_path, mounted_path=str(FluentdConfig.get_conf_path_within_docker()))) - - run_fluentd_image(self.docker_client, fluentd_labels, - self.fluentd_port, self.fluentd_conf_path, self.extra_container_kwargs) - def find_unbound_port(start=None, increment=False, @@ -537,3 +549,5 @@ def find_unbound_port(start=None, start += 1 else: start = random.randint(*port_range) + + diff --git a/clipper_admin/clipper_admin/docker/logging/README.md b/clipper_admin/clipper_admin/docker/logging/README.md index 63c31bf67..81f5a8f81 100644 --- a/clipper_admin/clipper_admin/docker/logging/README.md +++ b/clipper_admin/clipper_admin/docker/logging/README.md @@ -1,2 +1,52 @@ # Clipper Logging with Fluentd +## Log Centralization (Beta) +Clipper uses Fluentd (https://www.fluentd.org/) for centralizing logs from Docker containers within Clipper cluster. +It is currently a beta version. It only supports centralizing logs into Fluentd instance for now, but we will add various functinoalities +like monitoring and debugging on the top of it. Please create an issue if you want any functionality. +Also, please don't hesistate to contribute if you add any features. + +## How to guide +Firstly, when you define `DockerContainerManager`, you should set `use_centralized_log` parameter to be `True` + +```python + clipper_conn = ClipperConnection(DockerContainerManager(use_centralized_log=True)) + clipper_conn.start_clipper() +``` + +Once you start up the clipper cluster, you can check fluentd Docker container running. + +```bash +$docker ps +CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES +170000ec75d7 default-cluster-simple-example:1 "/container/containe…" 11 seconds ago Up 10 seconds (healthy) simple-example_1-71538 +5b533ff2fd3a prom/prometheus:v2.1.0 "/bin/prometheus --c…" 13 seconds ago Up 12 seconds 0.0.0.0:9090->9090/tcp metric_frontend-7206 +b71b557a0001 clipper/frontend-exporter:develop "python /usr/src/app…" 14 seconds ago Up 13 seconds query_frontend_exporter-55488 +bc8a7cc31754 clipper/query_frontend:develop "/clipper/release/sr…" 15 seconds ago Up 14 seconds 0.0.0.0:1337->1337/tcp, 0.0.0.0:7000->7000/tcp query_frontend-55488 +d04f33c654fd clipper/management_frontend:develop "/clipper/release/sr…" 15 seconds ago Up 15 seconds 0.0.0.0:1338->1338/tcp mgmt_frontend-60461 +30103e84e2a1 redis:alpine "docker-entrypoint.s…" 16 seconds ago Up 15 seconds 0.0.0.0:30356->6379/tcp redis-82152 +b78c3242c3e7 fluent/fluentd:v1.3-debian-1 "tini -- /bin/entryp…" 17 seconds ago Up 16 seconds 5140/tcp, 0.0.0.0:24224->24224/tcp, 0.0.0.0:24224->24224/udp fluentd-51374 +``` + +You can see centralized logs from fluentd container's stdout. Type + +```bash +$docker logs +``` + +Currently, it just prints out huge amount of logs centralized. It is because this feature is in the beggining phase. +We will add persistent storage for logs and query feature in the upcoming version. + +## How to customize +Currently, we don't recommend customizing a logging feature or using it for production. It is immature and unstable. Some APIs can be drastically changed. +If you still want to use it, you can directly modify fluentd conf file. It is mounted in a temp folder which you can easily find through python interactive shell. + +```python +>>> # Make sure you already ran clipper_conn.clipper_conn.start_clipper() with DockerContainerManager(use_centralized_log=True). Also, it is the python shell. +>>> clipper_conn = ClipperConnection(DockerContainerManager(use_centralized_log=True)) +>>> clipper_conn.connect() +19-03-21:10:36:58 INFO [clipper_admin.py:157] [default-cluster] Successfully connected to Clipper cluster at localhost:1337 +>>> cm = clipper_conn.cm +>>> cm.logging_system_instance.conf_path +# It will show you conf file path mounted on your local machine. +``` \ No newline at end of file diff --git a/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py index a5588bba9..4464af932 100644 --- a/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py @@ -1,44 +1,7 @@ -import random -import tempfile import os import sys -from clipper_admin.container_manager import CLIPPER_INTERNAL_FLUENTD_PORT, CLIPPER_DOCKER_LABEL - - -FLUENTD_VERSION = 'v1.3-debian-1' # TODO needs to be update to receive env variable like prometheus -FLUENTD_CONF_PATH_IN_DOCKER = '/fluentd/etc/fluent.conf' -FLUENTD_DEFAULT_CONF_PATH = '{current_dir}/clipper_fluentd.conf' \ - .format(current_dir=os.path.dirname(os.path.abspath(__file__))) - - -def run_fluentd_image(docker_client, fluentd_labels, fluend_port, fluentd_conf_path, extra_container_kwargs): - fluentd_cmd = [] # No cmd is required. - fluentd_name = "fluentd-{}".format(random.randint(0, 100000)) - fluentd_img = 'fluent/fluentd:{version}'.format(version=FLUENTD_VERSION) - - docker_client.containers.run( - fluentd_img, - command=fluentd_cmd, - name=fluentd_name, - ports={ - '%s/tcp' % fluend_port : CLIPPER_INTERNAL_FLUENTD_PORT, - '%s/udp' % fluend_port : CLIPPER_INTERNAL_FLUENTD_PORT - }, - volumes={ - fluentd_conf_path: { - 'bind': '{conf_path_within_docker}'.format(conf_path_within_docker=FLUENTD_CONF_PATH_IN_DOCKER), - 'mode': 'rw' - } - }, - labels=fluentd_labels, - **extra_container_kwargs) - - -def get_fluentd_log_config(): - return { - 'type': 'fluentd', - } +from clipper_admin.container_manager import CLIPPER_DOCKER_LABEL def get_centralized_logs(logging_dir): @@ -72,82 +35,5 @@ def get_logs_from_containers(docker_container_manager, logging_dir): return log_files -class FluentdConfig: - """ - Class to build a fluentd config file. - - EX) FluentdConfig() # will build an initial conf file with initial configuration. - .set_forward_address(OTHER_FLUENTD_ADDRESS, OTHER_FLUENTD_PORT) - .set_directory(DIR_NAME) - .set_file_name(FILE_NAME) - .build() - """ - def __init__(self, customized_conf_file=False): - """ - :param customized_conf_file: Decide whether or not to provide customized configuration file. - If false, it will use clipper default conf file - """ - self.conf = self.base_config - self._file_path = self.build_temp_file() - - def set_forward_address(self, address, port): - """ - Set the port number and address of external fluentd instance (internal is the clipper fluentd) - in which centralized logs will be forwarded - - :param port: port number to forward logs - """ - raise NotImplementedError("set_forward_address is not implemented yet. It will be coming soon.") - - def build(self, fluentd_port): - """ - Build a fluentd configuration file and return the path of it. - fluentd_default_conf_path will be stored in clipper_admin/docker folder - and used to write the initial conf file. - - Build should be called only once to build an initial conf file. - Developers can customize conf file written in the self.file_path using defined interfaces. - - TODO: Interfaces for modifying fluentd config file. - TODO: Interfaces for providing customized fluentd file - - :param fluentd_port: External fluentd port in which fluentd with this conf file listens to - :return: Path of fluentd config file in which Fluentd container mounts on. - """ - if self._file_path is None \ - or not os.path.isfile(self._file_path): - self._file_path = self.build_temp_file() - - # Logging-TODO: Currently, it copies the default conf from clipper_fluentd.conf. - # We need a way to customize it. - with open(FLUENTD_DEFAULT_CONF_PATH, 'r') as default_conf_file: - with open(self._file_path, 'w') as fluetnd_conf: - for line in default_conf_file: - # port number in a conf file should be the same as container manager's port number - if 'port' in line: - fluetnd_conf.write(' port {}\n'.format(fluentd_port)) - else: - fluetnd_conf.write(line) - - return self._file_path - - @property - def temp_file_path(self): - return self._file_path - - @property - def base_config(self): - return {} - - @staticmethod - def get_conf_path_within_docker(): - return FLUENTD_CONF_PATH_IN_DOCKER - - @staticmethod - def build_temp_file(): - file_path = tempfile.NamedTemporaryFile( - 'w', suffix='.conf', delete=False).name - file_path = os.path.realpath( - file_path) # resolve symlink - return file_path - +def get_default_log_config(): + return {'type': 'json-file'} diff --git a/clipper_admin/clipper_admin/docker/logging/clipper_fluentd.conf b/clipper_admin/clipper_admin/docker/logging/fluentd/clipper_fluentd.conf similarity index 100% rename from clipper_admin/clipper_admin/docker/logging/clipper_fluentd.conf rename to clipper_admin/clipper_admin/docker/logging/fluentd/clipper_fluentd.conf diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py new file mode 100644 index 000000000..d0e55060f --- /dev/null +++ b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py @@ -0,0 +1,157 @@ +import random +import tempfile +import os + +from clipper_admin.container_manager import ( + CLIPPER_INTERNAL_FLUENTD_PORT, CLIPPER_FLUENTD_CONFIG_LABEL, + CLIPPER_DOCKER_PORT_LABELS +) + + +FLUENTD_VERSION = 'v1.3-debian-1' # TODO needs to be update to receive env variable like prometheus +FLUENTD_CONF_PATH_IN_DOCKER = '/fluentd/etc/fluent.conf' +FLUENTD_DEFAULT_CONF_PATH = '{current_dir}/clipper_fluentd.conf' \ + .format(current_dir=os.path.dirname(os.path.abspath(__file__))) + + +class Fluentd(object): + def __init__(self, logger, cluster_name, docker_client, port=24224, conf_path=None): + self.port = port + self.logger = logger + self.docker_client = docker_client + self.cluster_name = cluster_name + self.conf_path = conf_path + + def start(self, clipper_common_labels, extra_container_kwargs): + self.logger.info("Starting Fluentd instance in Docker cluster {cluster_name}" + .format(cluster_name=self.cluster_name)) + + self.fluentd_conf_path = FluentdConfig().build(self.port) + fluentd_labels = self._get_labels(clipper_common_labels) + + self.logger.info( + "Fluentd Configuration Saved at {path}. " + "It will be mounted at {mounted_path} inside container" + .format(path=self.fluentd_conf_path, mounted_path=str(FluentdConfig.get_conf_path_within_docker()))) + + self._run_fluentd_image(self.docker_client, fluentd_labels, + self.port, self.fluentd_conf_path, extra_container_kwargs) + + def get_logs(self, logging_dir): + raise NotImplementedError("Not implemented yet.") + + def _run_fluentd_image(self, docker_client, fluentd_labels, fluend_port, fluentd_conf_path, extra_container_kwargs): + fluentd_cmd = [] # No cmd is required. + fluentd_name = "fluentd-{}".format(random.randint(0, 100000)) + fluentd_img = 'fluent/fluentd:{version}'.format(version=FLUENTD_VERSION) + + docker_client.containers.run( + fluentd_img, + command=fluentd_cmd, + name=fluentd_name, + ports={ + '%s/tcp' % fluend_port: CLIPPER_INTERNAL_FLUENTD_PORT, + '%s/udp' % fluend_port: CLIPPER_INTERNAL_FLUENTD_PORT + }, + volumes={ + fluentd_conf_path: { + 'bind': '{conf_path_within_docker}'.format(conf_path_within_docker=FLUENTD_CONF_PATH_IN_DOCKER), + 'mode': 'rw' + } + }, + labels=fluentd_labels, + **extra_container_kwargs) + + def _get_labels(self, clipper_common_labels): + fluentd_labels = clipper_common_labels.copy() + fluentd_labels[CLIPPER_FLUENTD_CONFIG_LABEL] = self.fluentd_conf_path + fluentd_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']] = str(self.port) + + return fluentd_labels + + @staticmethod + def get_log_config(): + return { + 'type': 'fluentd', + } + + +class FluentdConfig: + """ + Class to build a fluentd config file. + + EX) FluentdConfig() # will build an initial conf file with initial configuration. + .set_forward_address(OTHER_FLUENTD_ADDRESS, OTHER_FLUENTD_PORT) + .set_directory(DIR_NAME) + .set_file_name(FILE_NAME) + .build() + """ + + def __init__(self, customized_conf_file=False): + """ + :param customized_conf_file: Decide whether or not to provide customized configuration file. + If false, it will use clipper default conf file + """ + self.conf = self.base_config + self._file_path = self.build_temp_file() + + def set_forward_address(self, address, port): + """ + Set the port number and address of external fluentd instance (internal is the clipper fluentd) + in which centralized logs will be forwarded + + :param port: port number to forward logs + """ + raise NotImplementedError("set_forward_address is not implemented yet. It will be coming soon.") + + def build(self, fluentd_port): + """ + Build a fluentd configuration file and return the path of it. + fluentd_default_conf_path will be stored in clipper_admin/docker folder + and used to write the initial conf file. + + Build should be called only once to build an initial conf file. + Developers can customize conf file written in the self.file_path using defined interfaces. + + TODO: Interfaces for modifying fluentd config file. + TODO: Interfaces for providing customized fluentd file + + :param fluentd_port: External fluentd port in which fluentd with this conf file listens to + :return: Path of fluentd config file in which Fluentd container mounts on. + """ + if self._file_path is None \ + or not os.path.isfile(self._file_path): + self._file_path = self.build_temp_file() + + # Logging-TODO: Currently, it copies the default conf from clipper_fluentd.conf. + # We need a way to customize it. + with open(FLUENTD_DEFAULT_CONF_PATH, 'r') as default_conf_file: + with open(self._file_path, 'w') as fluetnd_conf: + for line in default_conf_file: + # port number in a conf file should be the same as container manager's port number + if 'port' in line: + fluetnd_conf.write(' port {}\n'.format(fluentd_port)) + else: + fluetnd_conf.write(line) + + return self._file_path + + @property + def temp_file_path(self): + return self._file_path + + @property + def base_config(self): + return {} + + @staticmethod + def get_conf_path_within_docker(): + return FLUENTD_CONF_PATH_IN_DOCKER + + @staticmethod + def build_temp_file(): + file_path = tempfile.NamedTemporaryFile( + 'w', suffix='.conf', delete=False).name + file_path = os.path.realpath( + file_path) # resolve symlink + return file_path \ No newline at end of file diff --git a/examples/basic_query/example_client.py b/examples/basic_query/example_client.py index 7324820ba..8eca6a303 100644 --- a/examples/basic_query/example_client.py +++ b/examples/basic_query/example_client.py @@ -40,11 +40,11 @@ def signal_handler(signal, frame): if __name__ == '__main__': signal.signal(signal.SIGINT, signal_handler) - clipper_conn = ClipperConnection(DockerContainerManager()) + clipper_conn = ClipperConnection(DockerContainerManager(use_centralized_log=False)) clipper_conn.start_clipper() python_deployer.create_endpoint(clipper_conn, "simple-example", "doubles", feature_sum) - time.sleep(2) + time.sleep(0.2) # For batch inputs set this number > 1 batch_size = 1 diff --git a/integration-tests/clipper_admin_tests.py b/integration-tests/clipper_admin_tests.py index 5719cdaaa..637eb7033 100644 --- a/integration-tests/clipper_admin_tests.py +++ b/integration-tests/clipper_admin_tests.py @@ -40,7 +40,7 @@ class ClipperManagerTestCaseShort(unittest.TestCase): def setUp(self): new_name = "admin-test-cluster-{}".format(random.randint(0, 5000)) self.clipper_conn = create_docker_connection( - cleanup=False, start_clipper=True, new_name=new_name, use_centralized_log=True) + cleanup=False, start_clipper=True, new_name=new_name, use_centralized_log=False) self.name = new_name def tearDown(self): diff --git a/integration-tests/clipper_fluentd_logging_docker.py b/integration-tests/clipper_fluentd_logging_docker.py index c396dcbdd..9bdbb5304 100644 --- a/integration-tests/clipper_fluentd_logging_docker.py +++ b/integration-tests/clipper_fluentd_logging_docker.py @@ -3,7 +3,6 @@ import json import logging import os -import subprocess import sys import time import random @@ -31,15 +30,7 @@ def feature_sum(xs): def setup(clipper_conn): - docker_client = get_docker_client() - containers = docker_client.containers.list( - filters={ - 'label': [ - '{key}={val}'.format( - key=CLIPPER_DOCKER_LABEL, val=clipper_conn.cm.cluster_name) - ] - }) - + containers = get_containers(clipper_conn) fluentd_container = None for c in containers: if 'fluentd-' in c.name: @@ -51,7 +42,19 @@ def setup(clipper_conn): return fluentd_container +def get_containers(clipper_conn): + docker_client = get_docker_client() + return docker_client.containers.list( + filters={ + 'label': [ + '{key}={val}'.format( + key=CLIPPER_DOCKER_LABEL, val=clipper_conn.cm.cluster_name) + ] + }) + + def check_fluentd_has_correct_logs(clipper_conn): + # We don't check frontend-exporter because it doesn't have lots of logs and is hard to find. fluentd_container = setup(clipper_conn) fluentd_logs = str(fluentd_container.logs()) @@ -59,6 +62,8 @@ def check_fluentd_has_correct_logs(clipper_conn): raise AssertionError("Query Frontend log is not found") if not check_metric_frontend_logs(fluentd_logs): raise AssertionError("Metric Frontend log is not found") + if not check_management_frontend_logs(fluentd_logs): + raise AssertionError("Management Frontend log is not found") if not check_redis_logs(fluentd_logs): raise AssertionError("Redis log is not found") @@ -75,12 +80,15 @@ def check_query_frontend_logs(fluentd_logs): def check_metric_frontend_logs(fluentd_logs): - return '"container_name":"/mgmt_frontend' in fluentd_logs + return '"container_name":"/metric_frontend' in fluentd_logs def check_redis_logs(fluentd_logs): return '"container_name":"/redis' in fluentd_logs +def check_management_frontend_logs(fluentd_logs): + return '"container_name":"/mgmt_frontend' in fluentd_logs + def check_model_logs(fluentd_logs, model_name): return '"container_name":"/{}',format(model_name) in fluentd_logs @@ -93,6 +101,35 @@ def log_docker_ps(clipper_conn): logger.info('Name {}, Image {}, Status {}, Label {}'.format( cont.name, cont.image, cont.status, cont.labels)) +def all_containers_found(containers): + metric_frontend_found = False + management_frontend_found = False + query_frontend_found = False + fluentd_found = False + redis_found = False + frontend_exporter_found = False + + for c in containers: + if 'metric_frontend' in c.name: + metric_frontend_found = True + elif 'query_frontend_exporter' in c.name: + frontend_exporter_found = True + elif 'query_frontend' in c.name: + query_frontend_found = True + elif 'mgmt_frontend' in c.name: + management_frontend_found = True + elif 'redis' in c.name: + redis_found = True + elif 'fluentd' in c.name: + fluentd_found = True + + return metric_frontend_found and management_frontend_found \ + and query_frontend_found and fluentd_found \ + and redis_found and frontend_exporter_found + + +def get_newline_str(): + return '=================================================================\n' if __name__ == '__main__': logging.basicConfig( @@ -108,14 +145,29 @@ def log_docker_ps(clipper_conn): cluster_name = "fluentd-test-{}".format(random.randint(0, 50000)) clipper_conn = create_docker_connection( cleanup=False, start_clipper=True, new_name=cluster_name, use_centralized_log=True) - python_deployer.create_endpoint( - clipper_conn, "simple-example", "doubles", feature_sum, num_replicas=2) - time.sleep(2) try: + timeout_count = 0 + while True: + containers = get_containers(clipper_conn) + if all_containers_found(containers): + break + timeout_count += 1 + if timeout_count == 5: + raise TimeoutError("Containers haven't been created within 10 seconds") + time.sleep(2) + logger.info("Test setup: All the necessary instances found") + logger.info(get_newline_str()) + logger.info("Test 1: Checking if fluentd has correct logs") check_fluentd_has_correct_logs(clipper_conn) logger.info("Fluentd Test (1/2): Test 1 passed") + logger.info(get_newline_str()) + + logger.info("Test 2: Deploying two models") + python_deployer.create_endpoint( + clipper_conn, "simple-example", "doubles", feature_sum, num_replicas=2) + time.sleep(2) logger.info( "Making 100 predictions using two model container; Should takes 25 seconds." @@ -127,13 +179,16 @@ def log_docker_ps(clipper_conn): logger.info("Test 2: Checking if fluentd has correct model logs") check_fluentd_has_correct_model_logs(clipper_conn, 'simple-example') logger.info("Fluentd Test (2/2): Test 2 passed") + logger.info(get_newline_str()) create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) + + logger.info("Fluentd tests All passed") except Exception as e: logger.info("Test failed") - #log_docker_ps(clipper_conn) + log_docker_ps(clipper_conn) logger.error(e) - #log_clipper_state(clipper_conn) + log_clipper_state(clipper_conn) clipper_conn.stop_all(graceful=False) sys.exit(1) From 0418be70f238ad7fe87c685b0cc1b6a0eb7f60de Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Thu, 21 Mar 2019 17:29:38 -0700 Subject: [PATCH 11/20] Cleaning up. Refactoring tests to unittest style --- clipper_admin/clipper_admin/clipper_admin.py | 1 - .../docker/docker_container_manager.py | 16 +- .../docker/logging/docker_logging_utils.py | 4 - .../docker/logging/fluentd/fluentd.py | 4 + examples/basic_query/example_client.py | 4 +- .../clipper_fluentd_logging_docker.py | 268 ++++++++---------- integration-tests/test_utils.py | 31 ++ 7 files changed, 165 insertions(+), 163 deletions(-) diff --git a/clipper_admin/clipper_admin/clipper_admin.py b/clipper_admin/clipper_admin/clipper_admin.py index 10cf1e260..3a4291778 100644 --- a/clipper_admin/clipper_admin/clipper_admin.py +++ b/clipper_admin/clipper_admin/clipper_admin.py @@ -132,7 +132,6 @@ def start_clipper(self, host=self.cm.get_admin_addr()) for name, url in [('query frontend', query_frontend_url), ('management frontend', mgmt_frontend_url)]: - print('{}: {}'.format(name, url)) r = requests.get(url, timeout=5) if r.status_code != requests.codes.ok: raise RequestException( diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index e43aab8a3..66043783a 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -475,24 +475,24 @@ def stop_all(self, graceful=True): c.kill() def _is_valid_logging_state_to_connect(self, all_labels): - if self.centralize_log and CLIPPER_DOCKER_PORT_LABELS['fluentd'] not in all_labels: - raise ConnectionError( + if self.centralize_log and not self.logging_system.container_is_running(all_labels): + raise ConnectionRefusedError( "Invalid state detected. " "log centralization is {log_centralization_state}, " - "but cannot find fluentd instance running." + "but cannot find fluentd instance running. " "Please change your use_centralized_log parameter of DockerContainermanager" .format(log_centralization_state=self.centralize_log) ) - elif CLIPPER_DOCKER_PORT_LABELS['fluentd'] in all_labels and not self.centralize_log: - raise ConnectionError( + elif self.logging_system.container_is_running(all_labels) and not self.centralize_log: + raise ConnectionRefusedError( "Invalid state detected. " "Fluentd instance is running, " - "but log centralization state is {log_centralization_state}." - "Please change your use_centralized_log parameter of DockerContainermanager" + "but log centralization state is {log_centralization_state}. " + "Please change your use_centralized_log parameter of DockerContainerManager to True" .format(log_centralization_state=self.centralize_log) ) else: - return CLIPPER_DOCKER_PORT_LABELS['fluentd'] in all_labels + return self.logging_system.container_is_running(all_labels) def get_admin_addr(self): return "{host}:{port}".format( diff --git a/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py index 4464af932..5d442090f 100644 --- a/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py @@ -4,10 +4,6 @@ from clipper_admin.container_manager import CLIPPER_DOCKER_LABEL -def get_centralized_logs(logging_dir): - raise NotImplementedError("Centralized log collection is not implemented yet. It is currently in beta.") - - def get_logs_from_containers(docker_container_manager, logging_dir): containers = docker_container_manager.docker_client.containers.list( filters={ diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py index d0e55060f..f4b655972 100644 --- a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py +++ b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py @@ -40,6 +40,10 @@ def start(self, clipper_common_labels, extra_container_kwargs): def get_logs(self, logging_dir): raise NotImplementedError("Not implemented yet.") + @staticmethod + def container_is_running(all_labels): + return CLIPPER_DOCKER_PORT_LABELS['fluentd'] in all_labels + def _run_fluentd_image(self, docker_client, fluentd_labels, fluend_port, fluentd_conf_path, extra_container_kwargs): fluentd_cmd = [] # No cmd is required. fluentd_name = "fluentd-{}".format(random.randint(0, 100000)) diff --git a/examples/basic_query/example_client.py b/examples/basic_query/example_client.py index 8eca6a303..3275187be 100644 --- a/examples/basic_query/example_client.py +++ b/examples/basic_query/example_client.py @@ -44,7 +44,7 @@ def signal_handler(signal, frame): clipper_conn.start_clipper() python_deployer.create_endpoint(clipper_conn, "simple-example", "doubles", feature_sum) - time.sleep(0.2) + time.sleep(2) # For batch inputs set this number > 1 batch_size = 1 @@ -58,6 +58,6 @@ def signal_handler(signal, frame): batch=True) else: predict(clipper_conn.get_query_addr(), np.random.random(200)) - time.sleep(2) + time.sleep(0.2) except Exception as e: clipper_conn.stop_all() \ No newline at end of file diff --git a/integration-tests/clipper_fluentd_logging_docker.py b/integration-tests/clipper_fluentd_logging_docker.py index 9bdbb5304..b7e00a275 100644 --- a/integration-tests/clipper_fluentd_logging_docker.py +++ b/integration-tests/clipper_fluentd_logging_docker.py @@ -6,16 +6,30 @@ import sys import time import random +import unittest import numpy as np import requests -import yaml -from test_utils import log_clipper_state, create_docker_connection, get_docker_client +from test_utils import ( + log_clipper_state, create_docker_connection, + get_docker_client, get_one_container, + get_containers, check_container_logs, + get_new_connection_instance +) cur_dir = os.path.dirname(os.path.abspath(__file__)) sys.path.insert(0, os.path.abspath("%s/../clipper_admin" % cur_dir)) from clipper_admin.deployers import python as python_deployer -from clipper_admin.container_manager import CLIPPER_DOCKER_LABEL + + +CLIPPER_NODES = [ + 'metric_frontend', + 'query_frontend_exporter', + 'query_frontend', + 'mgmt_frontend', + 'redis', + 'fluentd' +] def predict(addr, x): @@ -29,166 +43,124 @@ def feature_sum(xs): return [str(sum(x)) for x in xs] -def setup(clipper_conn): - containers = get_containers(clipper_conn) - fluentd_container = None - for c in containers: - if 'fluentd-' in c.name: - fluentd_container = c - - if fluentd_container is None: - raise AssertionError("Fluentd has not been running") - - return fluentd_container - - -def get_containers(clipper_conn): - docker_client = get_docker_client() - return docker_client.containers.list( - filters={ - 'label': [ - '{key}={val}'.format( - key=CLIPPER_DOCKER_LABEL, val=clipper_conn.cm.cluster_name) - ] - }) - - -def check_fluentd_has_correct_logs(clipper_conn): - # We don't check frontend-exporter because it doesn't have lots of logs and is hard to find. - fluentd_container = setup(clipper_conn) - fluentd_logs = str(fluentd_container.logs()) - - if not check_query_frontend_logs(fluentd_logs): - raise AssertionError("Query Frontend log is not found") - if not check_metric_frontend_logs(fluentd_logs): - raise AssertionError("Metric Frontend log is not found") - if not check_management_frontend_logs(fluentd_logs): - raise AssertionError("Management Frontend log is not found") - if not check_redis_logs(fluentd_logs): - raise AssertionError("Redis log is not found") - -def check_fluentd_has_correct_model_logs(clipper_conn, model_name): - fluentd_container = setup(clipper_conn) - fluentd_logs = str(fluentd_container.logs()) - - if not check_model_logs(fluentd_logs, model_name): - raise AssertionError("{model_name} log is not found".format(model_name=model_name)) - - -def check_query_frontend_logs(fluentd_logs): - return '"container_name":"/query_frontend' in fluentd_logs - - -def check_metric_frontend_logs(fluentd_logs): - return '"container_name":"/metric_frontend' in fluentd_logs - - -def check_redis_logs(fluentd_logs): - return '"container_name":"/redis' in fluentd_logs +class FluentdTest(unittest.TestCase): + def setUp(self): + logging.basicConfig( + format= + '%(asctime)s %(levelname)-8s [%(filename)s:%(lineno)d] %(message)s', + datefmt='%y-%m-%d:%H:%M:%S', + level=0) -def check_management_frontend_logs(fluentd_logs): - return '"container_name":"/mgmt_frontend' in fluentd_logs + self.logger = logging.getLogger(__name__) + self.cluster_name = "fluentd-test-{}".format(random.randint(0, 50000)) + self.start_clipper(self.cluster_name) + def start_clipper(self, cluster_name, use_centralized_log=True): + self.clipper_conn = create_docker_connection( + cleanup=False, start_clipper=True, new_name=cluster_name, use_centralized_log=use_centralized_log) -def check_model_logs(fluentd_logs, model_name): - return '"container_name":"/{}',format(model_name) in fluentd_logs - - -def log_docker_ps(clipper_conn): - container_runing = clipper_conn.cm.docker_client.containers.list() - logger.info('Current docker status') - for cont in container_runing: - logger.info('Name {}, Image {}, Status {}, Label {}'.format( - cont.name, cont.image, cont.status, cont.labels)) - -def all_containers_found(containers): - metric_frontend_found = False - management_frontend_found = False - query_frontend_found = False - fluentd_found = False - redis_found = False - frontend_exporter_found = False - - for c in containers: - if 'metric_frontend' in c.name: - metric_frontend_found = True - elif 'query_frontend_exporter' in c.name: - frontend_exporter_found = True - elif 'query_frontend' in c.name: - query_frontend_found = True - elif 'mgmt_frontend' in c.name: - management_frontend_found = True - elif 'redis' in c.name: - redis_found = True - elif 'fluentd' in c.name: - fluentd_found = True - - return metric_frontend_found and management_frontend_found \ - and query_frontend_found and fluentd_found \ - and redis_found and frontend_exporter_found - - -def get_newline_str(): - return '=================================================================\n' - -if __name__ == '__main__': - logging.basicConfig( - format= - '%(asctime)s %(levelname)-8s [%(filename)s:%(lineno)d] %(message)s', - datefmt='%y-%m-%d:%H:%M:%S', - level=0) - - logger = logging.getLogger(__name__) - - logger.info("Start Fluentd Test (0/2): Running 2 Replicas") - - cluster_name = "fluentd-test-{}".format(random.randint(0, 50000)) - clipper_conn = create_docker_connection( - cleanup=False, start_clipper=True, new_name=cluster_name, use_centralized_log=True) - - try: + # Make sure all the containers are on. timeout_count = 0 while True: - containers = get_containers(clipper_conn) - if all_containers_found(containers): + containers = get_containers(self.clipper_conn) + if self.all_containers_found(containers): break timeout_count += 1 if timeout_count == 5: - raise TimeoutError("Containers haven't been created within 10 seconds") + self.logger.info("Running containers: {}".format(containers)) + raise TimeoutError( + "Containers haven't been created within 10 seconds. " + "It means that every instance haven't been initialized." + ) time.sleep(2) - logger.info("Test setup: All the necessary instances found") - logger.info(get_newline_str()) - logger.info("Test 1: Checking if fluentd has correct logs") - check_fluentd_has_correct_logs(clipper_conn) - logger.info("Fluentd Test (1/2): Test 1 passed") - logger.info(get_newline_str()) - - logger.info("Test 2: Deploying two models") + self.logger.info("All the containers are found") + + def doCleanups(self): + self.clipper_conn = create_docker_connection( + cleanup=True, start_clipper=False, cleanup_name=self.cluster_name) + + def check_fluentd_has_correct_logs(self, clipper_conn): + fluentd_container = get_one_container('fluentd', clipper_conn) + fluentd_logs = str(fluentd_container.logs()) + for node_name in CLIPPER_NODES: + if node_name == 'query_frontend_exporter': + continue # We don't check exporter log because it is uncommon. + self.assertTrue(check_container_logs(fluentd_logs, node_name)) + + def check_fluentd_has_correct_model_logs(self, clipper_conn, model_name): + fluentd_container = get_one_container('fluentd', clipper_conn) + fluentd_logs = str(fluentd_container.logs()) + self.assertTrue(check_container_logs(fluentd_logs, model_name)) + + def all_containers_found(self, containers): + for c in containers: + node_name = c.name.split('-')[0] + if node_name not in CLIPPER_NODES: + return False + + return True + + def test_invalid_clipper_conn_old_connection_use_log_centralization(self): + # Raise a ConnectionError when new connection doesn't use log-centralization, although + # the original connection uses log-centralization. + new_conn = get_new_connection_instance(self.cluster_name, False) + self.assertRaises(ConnectionRefusedError, new_conn.connect) + + def test_invalid_clipper_conn_old_connection_not_use_log_centralization(self): + # Raise a ConnectionError when new connection uses log-centralization, although + # the original connection does not use log-centralization. + # Recreate a cluster with + self.clipper_conn = create_docker_connection( + cleanup=True, start_clipper=False, cleanup_name=self.cluster_name) + self.start_clipper(self.cluster_name, use_centralized_log=False) + new_conn = get_new_connection_instance(self.cluster_name, True) + self.assertRaises(ConnectionRefusedError, new_conn.connect) + + def test_correct_fluentd_connection(self): + new_clipper_conn = get_new_connection_instance(self.cluster_name, use_centralized_log=True) + new_clipper_conn.connect() + + self.assertTrue(new_clipper_conn.cm.centralize_log) + self.assertTrue(new_clipper_conn.cm.log_config == new_clipper_conn.cm.logging_system.get_log_config()) + + old_conn_fluentd = self.clipper_conn.cm.logging_system_instance + new_conn_fluentd = new_clipper_conn.cm.logging_system_instance + + self.assertTrue(old_conn_fluentd.port == new_conn_fluentd.port) + self.assertTrue(old_conn_fluentd.conf_path == new_conn_fluentd.conf_path) + + def test_clipper_with_fluentd(self): + self.check_fluentd_has_correct_logs(self.clipper_conn) + + def test_deployed_models_are_logged(self): + # Deploy models python_deployer.create_endpoint( - clipper_conn, "simple-example", "doubles", feature_sum, num_replicas=2) + self.clipper_conn, "simple-example", "doubles", feature_sum, num_replicas=2) time.sleep(2) - logger.info( + self.logger.info( "Making 100 predictions using two model container; Should takes 25 seconds." ) for _ in range(100): - predict(clipper_conn.get_query_addr(), np.random.random(200)) + predict(self.clipper_conn.get_query_addr(), np.random.random(200)) time.sleep(0.2) - logger.info("Test 2: Checking if fluentd has correct model logs") - check_fluentd_has_correct_model_logs(clipper_conn, 'simple-example') - logger.info("Fluentd Test (2/2): Test 2 passed") - logger.info(get_newline_str()) - - create_docker_connection( - cleanup=True, start_clipper=False, cleanup_name=cluster_name) - - logger.info("Fluentd tests All passed") - except Exception as e: - logger.info("Test failed") - log_docker_ps(clipper_conn) - logger.error(e) - log_clipper_state(clipper_conn) - clipper_conn.stop_all(graceful=False) - sys.exit(1) + self.check_fluentd_has_correct_model_logs(self.clipper_conn, 'simple-example') + + +if __name__ == '__main__': + TEST = [ + 'test_invalid_clipper_conn_old_connection_use_log_centralization', + 'test_invalid_clipper_conn_old_connection_not_use_log_centralization', + 'test_correct_fluentd_connection', + 'test_clipper_with_fluentd', + 'test_deployed_models_are_logged' + ] + suite = unittest.TestSuite() + + for test in TEST: + suite.addTest(FluentdTest(test)) + + result = unittest.TextTestRunner(verbosity=2, failfast=True).run(suite) + sys.exit(not result.wasSuccessful()) diff --git a/integration-tests/test_utils.py b/integration-tests/test_utils.py index e9e0d2f35..782181ea4 100644 --- a/integration-tests/test_utils.py +++ b/integration-tests/test_utils.py @@ -68,6 +68,37 @@ def find_unbound_port(): "randomly generated port %d is bound. Trying again." % port) +def get_new_connection_instance(cluster_name, use_centralized_log): + return ClipperConnection(DockerContainerManager(cluster_name=cluster_name, use_centralized_log=use_centralized_log)) + +def get_containers(clipper_conn): + docker_client = get_docker_client() + return docker_client.containers.list( + filters={ + 'label': [ + '{key}={val}'.format( + key=CLIPPER_DOCKER_LABEL, val=clipper_conn.cm.cluster_name) + ] + }) + + +def get_one_container(container_name, clipper_conn): + containers = get_containers(clipper_conn) + container_to_return = None + for c in containers: + if container_name in c.name: + container_to_return = c + + if container_to_return is None: + raise AssertionError("{} has not been running".format(container_to_return)) + + return container_to_return + + +def check_container_logs(logs, name): + return '"container_name":"/{}',format(name) in logs + + def create_docker_connection(cleanup=False, start_clipper=False, cleanup_name='default-cluster', From 7bf1f1b4e99ba13f98350f1abf6e377c2c2720ec Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Fri, 22 Mar 2019 22:46:19 -0700 Subject: [PATCH 12/20] Fixed errors found in CI. 1. Added __init__.py in fluentd folder. 2. Used requests' ConnectionError class instead of ConnectionRefusedError which is not supported in python2. Added user='root' inside fluentd container run function so that it can access conf file existing in a root folder within a container. --- .../clipper_admin/docker/docker_container_manager.py | 4 ++-- .../clipper_admin/docker/logging/fluentd/__init__.py | 0 .../clipper_admin/docker/logging/fluentd/fluentd.py | 1 + integration-tests/clipper_fluentd_logging_docker.py | 5 +++-- 4 files changed, 6 insertions(+), 4 deletions(-) create mode 100644 clipper_admin/clipper_admin/docker/logging/fluentd/__init__.py diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 66043783a..0d80b4fe9 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -476,7 +476,7 @@ def stop_all(self, graceful=True): def _is_valid_logging_state_to_connect(self, all_labels): if self.centralize_log and not self.logging_system.container_is_running(all_labels): - raise ConnectionRefusedError( + raise ConnectionError( "Invalid state detected. " "log centralization is {log_centralization_state}, " "but cannot find fluentd instance running. " @@ -484,7 +484,7 @@ def _is_valid_logging_state_to_connect(self, all_labels): .format(log_centralization_state=self.centralize_log) ) elif self.logging_system.container_is_running(all_labels) and not self.centralize_log: - raise ConnectionRefusedError( + raise ConnectionError( "Invalid state detected. " "Fluentd instance is running, " "but log centralization state is {log_centralization_state}. " diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/__init__.py b/clipper_admin/clipper_admin/docker/logging/fluentd/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py index f4b655972..cd9303042 100644 --- a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py +++ b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py @@ -63,6 +63,7 @@ def _run_fluentd_image(self, docker_client, fluentd_labels, fluend_port, fluentd 'mode': 'rw' } }, + user='root', labels=fluentd_labels, **extra_container_kwargs) diff --git a/integration-tests/clipper_fluentd_logging_docker.py b/integration-tests/clipper_fluentd_logging_docker.py index b7e00a275..63b20a341 100644 --- a/integration-tests/clipper_fluentd_logging_docker.py +++ b/integration-tests/clipper_fluentd_logging_docker.py @@ -7,6 +7,7 @@ import time import random import unittest +from requests.exceptions import ConnectionError import numpy as np import requests @@ -105,7 +106,7 @@ def test_invalid_clipper_conn_old_connection_use_log_centralization(self): # Raise a ConnectionError when new connection doesn't use log-centralization, although # the original connection uses log-centralization. new_conn = get_new_connection_instance(self.cluster_name, False) - self.assertRaises(ConnectionRefusedError, new_conn.connect) + self.assertRaises(ConnectionError, new_conn.connect) def test_invalid_clipper_conn_old_connection_not_use_log_centralization(self): # Raise a ConnectionError when new connection uses log-centralization, although @@ -115,7 +116,7 @@ def test_invalid_clipper_conn_old_connection_not_use_log_centralization(self): cleanup=True, start_clipper=False, cleanup_name=self.cluster_name) self.start_clipper(self.cluster_name, use_centralized_log=False) new_conn = get_new_connection_instance(self.cluster_name, True) - self.assertRaises(ConnectionRefusedError, new_conn.connect) + self.assertRaises(ConnectionError, new_conn.connect) def test_correct_fluentd_connection(self): new_clipper_conn = get_new_connection_instance(self.cluster_name, use_centralized_log=True) From a0c85015b2813bc956be903b806eb0f786e4a6ff Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Thu, 28 Mar 2019 10:50:58 -0700 Subject: [PATCH 13/20] Fixed port duplicated error. Internal and external port setting was wrong. --- .../docker/docker_container_manager.py | 4 ++-- .../docker/logging/fluentd/fluentd.py | 15 +++++++-------- .../clipper_fluentd_logging_docker.py | 2 +- integration-tests/test_utils.py | 1 + 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 0d80b4fe9..3c0430956 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -133,7 +133,7 @@ def __init__(self, self.docker_client, port=find_unbound_port(fluentd_port) ) - self.log_config = self.logging_system.get_log_config() + self.log_config = self.logging_system_instance.get_log_config() def start_clipper(self, query_frontend_image, @@ -313,7 +313,7 @@ def connect(self): port=all_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']], conf_path=all_labels[CLIPPER_FLUENTD_CONFIG_LABEL] ) - self.log_config = self.logging_system.get_log_config() + self.log_config = self.logging_system_instance.get_log_config() # Logging-TODO Add a Sqlite support def deploy_model(self, name, version, input_type, image, num_replicas=1): diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py index cd9303042..8acbf915a 100644 --- a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py +++ b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py @@ -54,8 +54,8 @@ def _run_fluentd_image(self, docker_client, fluentd_labels, fluend_port, fluentd command=fluentd_cmd, name=fluentd_name, ports={ - '%s/tcp' % fluend_port: CLIPPER_INTERNAL_FLUENTD_PORT, - '%s/udp' % fluend_port: CLIPPER_INTERNAL_FLUENTD_PORT + '%s/tcp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port, + '%s/udp' % CLIPPER_INTERNAL_FLUENTD_PORT: fluend_port }, volumes={ fluentd_conf_path: { @@ -74,10 +74,12 @@ def _get_labels(self, clipper_common_labels): return fluentd_labels - @staticmethod - def get_log_config(): + def get_log_config(self): return { 'type': 'fluentd', + 'Config': { + 'fluentd-address': '127.0.0.1:{port}'.format(port=self.port) + } } @@ -134,10 +136,7 @@ def build(self, fluentd_port): with open(self._file_path, 'w') as fluetnd_conf: for line in default_conf_file: # port number in a conf file should be the same as container manager's port number - if 'port' in line: - fluetnd_conf.write(' port {}\n'.format(fluentd_port)) - else: - fluetnd_conf.write(line) + fluetnd_conf.write(line) return self._file_path diff --git a/integration-tests/clipper_fluentd_logging_docker.py b/integration-tests/clipper_fluentd_logging_docker.py index 63b20a341..6d67da866 100644 --- a/integration-tests/clipper_fluentd_logging_docker.py +++ b/integration-tests/clipper_fluentd_logging_docker.py @@ -123,7 +123,7 @@ def test_correct_fluentd_connection(self): new_clipper_conn.connect() self.assertTrue(new_clipper_conn.cm.centralize_log) - self.assertTrue(new_clipper_conn.cm.log_config == new_clipper_conn.cm.logging_system.get_log_config()) + self.assertTrue(new_clipper_conn.cm.log_config == new_clipper_conn.cm.logging_system_instance.get_log_config()) old_conn_fluentd = self.clipper_conn.cm.logging_system_instance new_conn_fluentd = new_clipper_conn.cm.logging_system_instance diff --git a/integration-tests/test_utils.py b/integration-tests/test_utils.py index 782181ea4..2ad6cf7f0 100644 --- a/integration-tests/test_utils.py +++ b/integration-tests/test_utils.py @@ -115,6 +115,7 @@ def create_docker_connection(cleanup=False, clipper_query_port=find_unbound_port(), clipper_management_port=find_unbound_port(), clipper_rpc_port=find_unbound_port(), + fluentd_port=find_unbound_port(), redis_port=find_unbound_port(), ) cl = ClipperConnection(cm) From 18bef18f6eec95cdf4e16cc6bdc914c3c42f88f0 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Fri, 5 Apr 2019 15:38:58 -0700 Subject: [PATCH 14/20] Changed is_valid_state logic --- .../docker/docker_container_manager.py | 20 +++++++++---------- .../clipper_fluentd_logging_docker.py | 10 ++++++---- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index ea1532fee..c66848da9 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -314,6 +314,12 @@ def connect(self): self.prom_config_path = all_labels[CLIPPER_METRIC_CONFIG_LABEL] if self._is_valid_logging_state_to_connect(all_labels): + if (not self.centralize_log): + logger.info( + "The use_centralized_log flag was False, " + "but we found fluentd instance was running already." + "We will set the flag on for the consistency" + ) self.centralize_log= True self.logging_system_instance = \ self.logging_system( @@ -491,23 +497,15 @@ def stop_all(self, graceful=True): def _is_valid_logging_state_to_connect(self, all_labels): if self.centralize_log and not self.logging_system.container_is_running(all_labels): - raise ConnectionError( + raise ClipperException( "Invalid state detected. " "log centralization is {log_centralization_state}, " "but cannot find fluentd instance running. " "Please change your use_centralized_log parameter of DockerContainermanager" .format(log_centralization_state=self.centralize_log) ) - elif self.logging_system.container_is_running(all_labels) and not self.centralize_log: - raise ConnectionError( - "Invalid state detected. " - "Fluentd instance is running, " - "but log centralization state is {log_centralization_state}. " - "Please change your use_centralized_log parameter of DockerContainerManager to True" - .format(log_centralization_state=self.centralize_log) - ) - else: - return self.logging_system.container_is_running(all_labels) + + return self.logging_system.container_is_running(all_labels) def get_admin_addr(self): return "{host}:{port}".format( diff --git a/integration-tests/clipper_fluentd_logging_docker.py b/integration-tests/clipper_fluentd_logging_docker.py index 6d67da866..fe8250fac 100644 --- a/integration-tests/clipper_fluentd_logging_docker.py +++ b/integration-tests/clipper_fluentd_logging_docker.py @@ -21,6 +21,7 @@ cur_dir = os.path.dirname(os.path.abspath(__file__)) sys.path.insert(0, os.path.abspath("%s/../clipper_admin" % cur_dir)) from clipper_admin.deployers import python as python_deployer +from clipper_admin import ClipperException CLIPPER_NODES = [ @@ -103,20 +104,21 @@ def all_containers_found(self, containers): return True def test_invalid_clipper_conn_old_connection_use_log_centralization(self): - # Raise a ConnectionError when new connection doesn't use log-centralization, although + # When new connection doesn't use log-centralization, although # the original connection uses log-centralization. new_conn = get_new_connection_instance(self.cluster_name, False) - self.assertRaises(ConnectionError, new_conn.connect) + new_conn.connect() + self.assertTrue(new_conn.cm.centralize_log) def test_invalid_clipper_conn_old_connection_not_use_log_centralization(self): - # Raise a ConnectionError when new connection uses log-centralization, although + # Raise a ClipperException when new connection uses log-centralization, although # the original connection does not use log-centralization. # Recreate a cluster with self.clipper_conn = create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=self.cluster_name) self.start_clipper(self.cluster_name, use_centralized_log=False) new_conn = get_new_connection_instance(self.cluster_name, True) - self.assertRaises(ConnectionError, new_conn.connect) + self.assertRaises(ClipperException, new_conn.connect) def test_correct_fluentd_connection(self): new_clipper_conn = get_new_connection_instance(self.cluster_name, use_centralized_log=True) From d9cd323f69685ac31ae0882996e3c4cad5858e67 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Fri, 5 Apr 2019 16:34:40 -0700 Subject: [PATCH 15/20] Remove fluentd test to see if it happens due to the test --- bin/shipyard/clipper_test.cfg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/shipyard/clipper_test.cfg.py b/bin/shipyard/clipper_test.cfg.py index 288365bd5..1360b9f6d 100644 --- a/bin/shipyard/clipper_test.cfg.py +++ b/bin/shipyard/clipper_test.cfg.py @@ -14,7 +14,7 @@ DOCKER_INTEGRATION_TESTS = { "admin_unit_test": "python /clipper/integration-tests/clipper_admin_tests.py", - "fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", + #"fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", "many_apps_many_models": "python /clipper/integration-tests/many_apps_many_models.py", "pyspark": "python /clipper/integration-tests/deploy_pyspark_models.py", "pyspark_pipeline": "python /clipper/integration-tests/deploy_pyspark_pipeline_models.py", From 008d0ac8da3544776f0a04885927d8206516370f Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Tue, 9 Apr 2019 09:35:19 -0700 Subject: [PATCH 16/20] Clean some part of code and add some strings for prettifying test logs --- bin/shipyard/clipper_test.cfg.py | 2 +- .../docker/docker_container_manager.py | 40 ++++++++++--------- .../docker/logging/fluentd/fluentd.py | 4 ++ integration-tests/deploy_pyspark_models.py | 2 +- .../deploy_pyspark_pipeline_models.py | 2 +- .../deploy_pyspark_sparkml_models.py | 2 +- integration-tests/test_utils.py | 15 +++++-- 7 files changed, 42 insertions(+), 25 deletions(-) diff --git a/bin/shipyard/clipper_test.cfg.py b/bin/shipyard/clipper_test.cfg.py index 1360b9f6d..2d9ad299b 100644 --- a/bin/shipyard/clipper_test.cfg.py +++ b/bin/shipyard/clipper_test.cfg.py @@ -14,7 +14,6 @@ DOCKER_INTEGRATION_TESTS = { "admin_unit_test": "python /clipper/integration-tests/clipper_admin_tests.py", - #"fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", "many_apps_many_models": "python /clipper/integration-tests/many_apps_many_models.py", "pyspark": "python /clipper/integration-tests/deploy_pyspark_models.py", "pyspark_pipeline": "python /clipper/integration-tests/deploy_pyspark_pipeline_models.py", @@ -25,6 +24,7 @@ "multi_tenancy": "python /clipper/integration-tests/multi_tenancy_test.py", # "rclipper": "/clipper/integration-tests/r_integration_test/rclipper_test.sh", "docker_metric": "python /clipper/integration-tests/clipper_metric_docker.py", + "fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", } NUM_RETRIES = 2 diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index c66848da9..4c0da0da9 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -312,25 +312,8 @@ def connect(self): 'query_rpc']] self.prometheus_port = all_labels[CLIPPER_DOCKER_PORT_LABELS['metric']] self.prom_config_path = all_labels[CLIPPER_METRIC_CONFIG_LABEL] - if self._is_valid_logging_state_to_connect(all_labels): - if (not self.centralize_log): - logger.info( - "The use_centralized_log flag was False, " - "but we found fluentd instance was running already." - "We will set the flag on for the consistency" - ) - self.centralize_log= True - self.logging_system_instance = \ - self.logging_system( - self.logger, - self.cluster_name, - self.docker_client, - port=all_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']], - conf_path=all_labels[CLIPPER_FLUENTD_CONFIG_LABEL] - ) - self.log_config = self.logging_system_instance.get_log_config() - # Logging-TODO Add a Sqlite support + self.connect_to_logging_system(all_labels) def deploy_model(self, name, version, input_type, image, num_replicas=1): # Parameters @@ -507,6 +490,27 @@ def _is_valid_logging_state_to_connect(self, all_labels): return self.logging_system.container_is_running(all_labels) + def connect_to_logging_system(self, all_labels): + if not self.centralize_log: + logger.info( + "The current DockerContainerManager's use_centralized_log flag is False, " + "but there is a logging system {type} instance running in a cluster." + "It means that clipper cluster you want to connect uses log centralization." + "We will set the flag on to avoid unexpected bugs" + .format(type=self.logging_system.get_type()) + ) + self.centralize_log= True + self.logging_system_instance = \ + self.logging_system( + self.logger, + self.cluster_name, + self.docker_client, + port=all_labels[CLIPPER_DOCKER_PORT_LABELS['fluentd']], + conf_path=all_labels[CLIPPER_FLUENTD_CONFIG_LABEL] + ) + self.log_config = self.logging_system_instance.get_log_config() + # Logging-TODO Add a Sqlite support + def get_admin_addr(self): return "{host}:{port}".format( host=self.public_hostname, port=self.clipper_management_port) diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py index 8acbf915a..45ad91af1 100644 --- a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py +++ b/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py @@ -44,6 +44,10 @@ def get_logs(self, logging_dir): def container_is_running(all_labels): return CLIPPER_DOCKER_PORT_LABELS['fluentd'] in all_labels + @staticmethod + def get_type(): + return "Fluentd" + def _run_fluentd_image(self, docker_client, fluentd_labels, fluend_port, fluentd_conf_path, extra_container_kwargs): fluentd_cmd = [] # No cmd is required. fluentd_name = "fluentd-{}".format(random.randint(0, 100000)) diff --git a/integration-tests/deploy_pyspark_models.py b/integration-tests/deploy_pyspark_models.py index 833e7b544..a23bb8593 100644 --- a/integration-tests/deploy_pyspark_models.py +++ b/integration-tests/deploy_pyspark_models.py @@ -192,8 +192,8 @@ def get_test_point(): create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) except Exception as e: - log_docker(clipper_conn) logger.exception("Exception: {}".format(e)) + log_docker(clipper_conn) create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) sys.exit(1) diff --git a/integration-tests/deploy_pyspark_pipeline_models.py b/integration-tests/deploy_pyspark_pipeline_models.py index d2ed37b9e..d53f45840 100644 --- a/integration-tests/deploy_pyspark_pipeline_models.py +++ b/integration-tests/deploy_pyspark_pipeline_models.py @@ -182,8 +182,8 @@ def run_test(): cleanup=True, start_clipper=False, cleanup_name=cluster_name) logger.info("ALL TESTS PASSED") except Exception as e: - log_docker(clipper_conn) logger.exception("Exception: {}".format(e)) + log_docker(clipper_conn) create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) sys.exit(1) diff --git a/integration-tests/deploy_pyspark_sparkml_models.py b/integration-tests/deploy_pyspark_sparkml_models.py index 2650fabca..cd871dd02 100644 --- a/integration-tests/deploy_pyspark_sparkml_models.py +++ b/integration-tests/deploy_pyspark_sparkml_models.py @@ -166,8 +166,8 @@ def get_test_point(): create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) except Exception as e: - log_docker(clipper_conn) logger.exception("Exception: {}".format(e)) + log_docker(clipper_conn) create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) sys.exit(1) diff --git a/integration-tests/test_utils.py b/integration-tests/test_utils.py index 6232a5b71..49a094e90 100644 --- a/integration-tests/test_utils.py +++ b/integration-tests/test_utils.py @@ -218,16 +218,25 @@ def log_docker(clipper_conn): """Retrieve status and log for last ten containers""" container_runing = clipper_conn.cm.docker_client.containers.list(limit=10) - logger.info('----------------------') + logger.info('\n================================================================') logger.info('Last ten containers status') + logger.info('================================================================') for cont in container_runing: logger.info('Name {}, Image {}, Status {}, Label {}'.format( cont.name, cont.image, cont.status, cont.labels)) - logger.info('----------------------') + logger.info('\n=================================================================') logger.info('Printing out logs') + logger.info('================================================================') for cont in container_runing: logger.info('Name {}, Image {}, Status {}, Label {}'.format( cont.name, cont.image, cont.status, cont.labels)) - logger.info(cont.logs()) + try: + logger.info(cont.logs()) + except docker.errors.APIError as e: + logger.warning("Error while parsing logs. It is most likely because you use log centralization.") + + logger.info('\n=================================================================') + logger.info('log_docker is completed') + logger.info('================================================================\n\n') From 9aec9649d553eead071822defd685b434c2b8753 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Tue, 9 Apr 2019 14:26:07 -0700 Subject: [PATCH 17/20] Test with only one broken test to see if parallelization is an issue --- bin/shipyard/clipper_test.cfg.py | 20 +++++++++---------- integration-tests/deploy_pyspark_models.py | 4 ++-- .../deploy_pyspark_pipeline_models.py | 4 ++-- .../deploy_pyspark_sparkml_models.py | 4 ++-- integration-tests/test_utils.py | 10 +++++----- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/bin/shipyard/clipper_test.cfg.py b/bin/shipyard/clipper_test.cfg.py index 2d9ad299b..792aec763 100644 --- a/bin/shipyard/clipper_test.cfg.py +++ b/bin/shipyard/clipper_test.cfg.py @@ -13,18 +13,18 @@ } DOCKER_INTEGRATION_TESTS = { - "admin_unit_test": "python /clipper/integration-tests/clipper_admin_tests.py", - "many_apps_many_models": "python /clipper/integration-tests/many_apps_many_models.py", + # "admin_unit_test": "python /clipper/integration-tests/clipper_admin_tests.py", + # "many_apps_many_models": "python /clipper/integration-tests/many_apps_many_models.py", "pyspark": "python /clipper/integration-tests/deploy_pyspark_models.py", - "pyspark_pipeline": "python /clipper/integration-tests/deploy_pyspark_pipeline_models.py", - "pysparkml": "python /clipper/integration-tests/deploy_pyspark_sparkml_models.py", - "tensorflow": "python /clipper/integration-tests/deploy_tensorflow_models.py", - "mxnet": "python /clipper/integration-tests/deploy_mxnet_models.py", - "pytorch": "python /clipper/integration-tests/deploy_pytorch_models.py", - "multi_tenancy": "python /clipper/integration-tests/multi_tenancy_test.py", + # "pyspark_pipeline": "python /clipper/integration-tests/deploy_pyspark_pipeline_models.py", + # "pysparkml": "python /clipper/integration-tests/deploy_pyspark_sparkml_models.py", + # "tensorflow": "python /clipper/integration-tests/deploy_tensorflow_models.py", + # "mxnet": "python /clipper/integration-tests/deploy_mxnet_models.py", + # "pytorch": "python /clipper/integration-tests/deploy_pytorch_models.py", + # "multi_tenancy": "python /clipper/integration-tests/multi_tenancy_test.py", # "rclipper": "/clipper/integration-tests/r_integration_test/rclipper_test.sh", - "docker_metric": "python /clipper/integration-tests/clipper_metric_docker.py", - "fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", + #"docker_metric": "python /clipper/integration-tests/clipper_metric_docker.py", + #"fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", } NUM_RETRIES = 2 diff --git a/integration-tests/deploy_pyspark_models.py b/integration-tests/deploy_pyspark_models.py index a23bb8593..6324565ef 100644 --- a/integration-tests/deploy_pyspark_models.py +++ b/integration-tests/deploy_pyspark_models.py @@ -180,10 +180,10 @@ def get_test_point(): version += 1 deploy_and_test_model( sc, clipper_conn, lr_model, version, predict_fn=predict) - except BenchmarkException: + except BenchmarkException as e: + logger.exception("BenchmarkException: {}".format(e)) log_docker(clipper_conn) log_clipper_state(clipper_conn) - logger.exception("BenchmarkException") create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) sys.exit(1) diff --git a/integration-tests/deploy_pyspark_pipeline_models.py b/integration-tests/deploy_pyspark_pipeline_models.py index d53f45840..379e7733a 100644 --- a/integration-tests/deploy_pyspark_pipeline_models.py +++ b/integration-tests/deploy_pyspark_pipeline_models.py @@ -169,10 +169,10 @@ def run_test(): if num_defaults > num_preds / 2: raise BenchmarkException("Error querying APP %s, MODEL %s:%d" % (app_name, model_name, version)) - except BenchmarkException: + except BenchmarkException as e: + logger.exception("BenchmarkException: {}".format(e)) log_docker(clipper_conn) log_clipper_state(clipper_conn) - logger.exception("BenchmarkException") create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) sys.exit(1) diff --git a/integration-tests/deploy_pyspark_sparkml_models.py b/integration-tests/deploy_pyspark_sparkml_models.py index cd871dd02..18a384402 100644 --- a/integration-tests/deploy_pyspark_sparkml_models.py +++ b/integration-tests/deploy_pyspark_sparkml_models.py @@ -154,10 +154,10 @@ def get_test_point(): lr_model = train_logistic_regression(trainDf) deploy_and_test_model( sc, clipper_conn, lr_model, version, link_model=True) - except BenchmarkException: + except BenchmarkException as e: + logger.exception("BenchmarkException: {}".format(e)) log_docker(clipper_conn) log_clipper_state(clipper_conn) - logger.exception("BenchmarkException") create_docker_connection( cleanup=True, start_clipper=False, cleanup_name=cluster_name) sys.exit(1) diff --git a/integration-tests/test_utils.py b/integration-tests/test_utils.py index 49a094e90..18165ddcc 100644 --- a/integration-tests/test_utils.py +++ b/integration-tests/test_utils.py @@ -217,15 +217,15 @@ def log_docker(clipper_conn): return """Retrieve status and log for last ten containers""" - container_runing = clipper_conn.cm.docker_client.containers.list(limit=10) - logger.info('\n================================================================') + container_runing = clipper_conn.cm.docker_client.containers.list(limit=10, all=True) + logger.info('================================================================') logger.info('Last ten containers status') logger.info('================================================================') for cont in container_runing: logger.info('Name {}, Image {}, Status {}, Label {}'.format( cont.name, cont.image, cont.status, cont.labels)) - logger.info('\n=================================================================') + logger.info('=================================================================') logger.info('Printing out logs') logger.info('================================================================') @@ -237,6 +237,6 @@ def log_docker(clipper_conn): except docker.errors.APIError as e: logger.warning("Error while parsing logs. It is most likely because you use log centralization.") - logger.info('\n=================================================================') + logger.info('=================================================================') logger.info('log_docker is completed') - logger.info('================================================================\n\n') + logger.info('================================================================') From 5c9f25f0703e038f795b48f6e202f992bf61c4bf Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Tue, 9 Apr 2019 14:53:26 -0700 Subject: [PATCH 18/20] It will probably fix the bug --- .../clipper_admin/docker/docker_container_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 4c0da0da9..5d225521b 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -18,11 +18,11 @@ CLIPPER_FLUENTD_CONFIG_LABEL) from requests.exceptions import ConnectionError from .docker_metric_utils import * -from clipper_admin.docker.logging.docker_logging_utils import ( +from .logging.docker_logging_utils import ( get_logs_from_containers, get_default_log_config ) -from clipper_admin.docker.logging.fluentd.fluentd import Fluentd +from .logging.fluentd.fluentd import Fluentd logger = logging.getLogger(__name__) From 2ea53ce78f3f7725f8619b3e5aac4fd0b635a5eb Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Tue, 9 Apr 2019 17:17:03 -0700 Subject: [PATCH 19/20] Added a submodule logging in setup.py to resolve import error --- bin/shipyard/clipper_test.cfg.py | 20 +++++++++---------- .../docker/docker_container_manager.py | 2 +- .../{fluentd => }/clipper_fluentd.conf | 0 .../docker/logging/{fluentd => }/fluentd.py | 0 .../docker/logging/fluentd/__init__.py | 0 clipper_admin/setup.py | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) rename clipper_admin/clipper_admin/docker/logging/{fluentd => }/clipper_fluentd.conf (100%) rename clipper_admin/clipper_admin/docker/logging/{fluentd => }/fluentd.py (100%) delete mode 100644 clipper_admin/clipper_admin/docker/logging/fluentd/__init__.py diff --git a/bin/shipyard/clipper_test.cfg.py b/bin/shipyard/clipper_test.cfg.py index 792aec763..2d9ad299b 100644 --- a/bin/shipyard/clipper_test.cfg.py +++ b/bin/shipyard/clipper_test.cfg.py @@ -13,18 +13,18 @@ } DOCKER_INTEGRATION_TESTS = { - # "admin_unit_test": "python /clipper/integration-tests/clipper_admin_tests.py", - # "many_apps_many_models": "python /clipper/integration-tests/many_apps_many_models.py", + "admin_unit_test": "python /clipper/integration-tests/clipper_admin_tests.py", + "many_apps_many_models": "python /clipper/integration-tests/many_apps_many_models.py", "pyspark": "python /clipper/integration-tests/deploy_pyspark_models.py", - # "pyspark_pipeline": "python /clipper/integration-tests/deploy_pyspark_pipeline_models.py", - # "pysparkml": "python /clipper/integration-tests/deploy_pyspark_sparkml_models.py", - # "tensorflow": "python /clipper/integration-tests/deploy_tensorflow_models.py", - # "mxnet": "python /clipper/integration-tests/deploy_mxnet_models.py", - # "pytorch": "python /clipper/integration-tests/deploy_pytorch_models.py", - # "multi_tenancy": "python /clipper/integration-tests/multi_tenancy_test.py", + "pyspark_pipeline": "python /clipper/integration-tests/deploy_pyspark_pipeline_models.py", + "pysparkml": "python /clipper/integration-tests/deploy_pyspark_sparkml_models.py", + "tensorflow": "python /clipper/integration-tests/deploy_tensorflow_models.py", + "mxnet": "python /clipper/integration-tests/deploy_mxnet_models.py", + "pytorch": "python /clipper/integration-tests/deploy_pytorch_models.py", + "multi_tenancy": "python /clipper/integration-tests/multi_tenancy_test.py", # "rclipper": "/clipper/integration-tests/r_integration_test/rclipper_test.sh", - #"docker_metric": "python /clipper/integration-tests/clipper_metric_docker.py", - #"fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", + "docker_metric": "python /clipper/integration-tests/clipper_metric_docker.py", + "fluentd": "python /clipper/integration-tests/clipper_fluentd_logging_docker.py", } NUM_RETRIES = 2 diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index 5d225521b..a09470ce0 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -22,7 +22,7 @@ get_logs_from_containers, get_default_log_config ) -from .logging.fluentd.fluentd import Fluentd +from clipper_admin.docker.logging.fluentd import Fluentd logger = logging.getLogger(__name__) diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/clipper_fluentd.conf b/clipper_admin/clipper_admin/docker/logging/clipper_fluentd.conf similarity index 100% rename from clipper_admin/clipper_admin/docker/logging/fluentd/clipper_fluentd.conf rename to clipper_admin/clipper_admin/docker/logging/clipper_fluentd.conf diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py b/clipper_admin/clipper_admin/docker/logging/fluentd.py similarity index 100% rename from clipper_admin/clipper_admin/docker/logging/fluentd/fluentd.py rename to clipper_admin/clipper_admin/docker/logging/fluentd.py diff --git a/clipper_admin/clipper_admin/docker/logging/fluentd/__init__.py b/clipper_admin/clipper_admin/docker/logging/fluentd/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/clipper_admin/setup.py b/clipper_admin/setup.py index 34a9baf08..0bb3a47e0 100644 --- a/clipper_admin/setup.py +++ b/clipper_admin/setup.py @@ -18,7 +18,7 @@ license='Apache-2.0', packages=[ "clipper_admin", "clipper_admin.docker", "clipper_admin.kubernetes", - "clipper_admin.deployers", "clipper_admin.metrics" + "clipper_admin.deployers", "clipper_admin.metrics", "clipper_admin.docker.logging" ], package_data={'clipper_admin': ['*.txt', '*/*.yaml']}, keywords=['clipper', 'prediction', 'model', 'management'], From 6f5673a6353b32d2863a490d4e4e282f5d2cb2ee Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Sat, 27 Apr 2019 12:05:25 -0700 Subject: [PATCH 20/20] changed some code based on code review --- .../clipper_admin/docker/docker_container_manager.py | 8 ++------ .../clipper_admin/docker/logging/docker_logging_utils.py | 2 ++ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/clipper_admin/clipper_admin/docker/docker_container_manager.py b/clipper_admin/clipper_admin/docker/docker_container_manager.py index a09470ce0..0a5f00840 100644 --- a/clipper_admin/clipper_admin/docker/docker_container_manager.py +++ b/clipper_admin/clipper_admin/docker/docker_container_manager.py @@ -15,7 +15,7 @@ CLIPPER_INTERNAL_MANAGEMENT_PORT, CLIPPER_INTERNAL_METRIC_PORT, CLIPPER_INTERNAL_REDIS_PORT, CLIPPER_DOCKER_PORT_LABELS, CLIPPER_METRIC_CONFIG_LABEL, ClusterAdapter, - CLIPPER_FLUENTD_CONFIG_LABEL) + CLIPPER_FLUENTD_CONFIG_LABEL, CLIPPER_INTERNAL_FLUENTD_PORT) from requests.exceptions import ConnectionError from .docker_metric_utils import * from .logging.docker_logging_utils import ( @@ -28,12 +28,11 @@ class DockerContainerManager(ContainerManager): - # Logging-TODO Add SQLITE support def __init__(self, cluster_name="default-cluster", docker_ip_address="localhost", use_centralized_log=False, - fluentd_port=24224, + fluentd_port=CLIPPER_INTERNAL_FLUENTD_PORT, clipper_query_port=1337, clipper_management_port=1338, clipper_rpc_port=7000, @@ -124,7 +123,6 @@ def __init__(self, }) # Setting Docker cluster logging. - # Logging-TODO Add SQLITE support self.logging_system = Fluentd self.log_config = get_default_log_config() self.logging_system_instance = None @@ -178,7 +176,6 @@ def start_clipper(self, format(self.cluster_name)) if self.centralize_log: - # Logging-TODO Initialize SQLite Logging DB self.logging_system_instance.start(self.common_labels, self.extra_container_kwargs) # Redis for cluster configuration @@ -509,7 +506,6 @@ def connect_to_logging_system(self, all_labels): conf_path=all_labels[CLIPPER_FLUENTD_CONFIG_LABEL] ) self.log_config = self.logging_system_instance.get_log_config() - # Logging-TODO Add a Sqlite support def get_admin_addr(self): return "{host}:{port}".format( diff --git a/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py index 5d442090f..c7727ef85 100644 --- a/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py +++ b/clipper_admin/clipper_admin/docker/logging/docker_logging_utils.py @@ -32,4 +32,6 @@ def get_logs_from_containers(docker_container_manager, logging_dir): def get_default_log_config(): + # default config is defined here. + # https://docs.docker.com/config/containers/logging/configure/ return {'type': 'json-file'}