Implement delete_model_links API (ucbrise#654)

* Implement delete_model_links API

* related issue : ucbrise#603
* Delete the link between a model and an app.
* Implemented `unlink_model_from_app` Clipper API in clipper_admin.

> Description
> -----------
> unlink_model_from_app(app_name, model_name)

> Parameters
> ----------
> app_name : str
>     The name of the application
> model_name : str
>     The name of the model to link to the application

> Raises
> ------
> :py:exc:`clipper.UnconnectedException`
>     versions. All replicas for each version of each model will be stopped.

* Implemented `delete_model_links` Management-Frontend API.

> API path
> --------
> POST /admin/delete_model_links

> Request schema
> --------------
> const std::string DELETE_MODEL_LINKS_JSON_SCHEMA = R"(
>   {
>   "app_name" := string,
>   "model_names" := [string]
>   }
> )";

* Fix build error in redis_test.cpp

* Decrease test samples for test_stop_models case

clipper_admin/clipper_admin/clipper_admin.py

@@ -281,6 +281,47 @@ def link_model_to_app(self, app_name, model_name):
                 "Model {model} is now linked to application {app}".format(
                     model=model_name, app=app_name))
 
+    def unlink_model_from_app(self, app_name, model_name):
+        """
+        Prevents the model with `model_name` from being used by the app with `app_name`.
+        The model and app should both be registered with Clipper and a link should
+        already exist between them.
+
+        Parameters
+        ----------
+        app_name : str
+            The name of the application
+        model_name : str
+            The name of the model to link to the application
+
+        Raises
+        ------
+        :py:exc:`clipper.UnconnectedException`
+        :py:exc:`clipper.ClipperException`
+        """
+
+        if not self.connected:
+            raise UnconnectedException()
+
+        url = "http://{host}/admin/delete_model_links".format(
+            host=self.cm.get_admin_addr())
+        req_json = json.dumps({
+            "app_name": app_name,
+            "model_names": [model_name]
+        })
+        headers = {'Content-type': 'application/json'}
+        r = requests.post(url, headers=headers, data=req_json)
+        logger.debug(r.text)
+        if r.status_code != requests.codes.ok:
+            msg = "Received error status code: {code} and message: {msg}".format(
+                code=r.status_code, msg=r.text)
+            logger.error(msg)
+            raise ClipperException(msg)
+        else:
+            logger.info(
+                "Model {model} is now removed to application {app}".format(
+                    model=model_name, app=app_name))
+
     def build_and_deploy_model(self,
                                name,
                                version,
@@ -859,7 +900,7 @@ def get_linked_models(self, app_name):
         -------
         list
             Returns a list of the names of models linked to the app.
-            If no models are linked to the specified app, None is returned.
+            If no models are linked to the specified app, empty list is returned.
 
         Raises
         ------

integration-tests/clipper_admin_tests.py

@@ -130,6 +130,25 @@ def test_link_registered_model_to_app_succeeds(self):
         result = self.clipper_conn.get_linked_models(app_name)
         self.assertEqual([model_name], result)
 
+    def test_unlink_registered_model_from_app_succeeds(self):
+        # Register app
+        app_name = "testapp"
+        input_type = "doubles"
+        default_output = "DEFAULT"
+        slo_micros = 30000
+        self.clipper_conn.register_application(app_name, input_type,
+                                               default_output, slo_micros)
+
+        # Register model
+        model_name = "m"
+        self.clipper_conn.register_model(model_name, "v1", input_type)
+
+        self.clipper_conn.link_model_to_app(app_name, model_name)
+        self.clipper_conn.unlink_model_from_app(app_name, model_name)
+
+        result = self.clipper_conn.get_linked_models(app_name)
+        self.assertEqual([], result)
+
     def get_app_info_for_registered_app_returns_info_dictionary(self):
         # Register app
         app_name = "testapp"

src/frontends/src/query_frontend.hpp

@@ -204,6 +204,13 @@ class RequestHandler {
             auto linked_model_names =
                 clipper::redis::get_linked_models(redis_connection_, app_name);
             set_linked_models_for_app(app_name, linked_model_names);
+
+          } else if (event_type == "srem") {
+            clipper::log_info_formatted(LOGGING_TAG_QUERY_FRONTEND,
+                                        "Model link removal detected for app: {}", app_name);
+            auto linked_model_names =
+                clipper::redis::get_linked_models(redis_connection_, app_name);
+            set_linked_models_for_app(app_name, linked_model_names);
           }
         });
 

src/libclipper/include/clipper/redis.hpp

@@ -281,6 +281,14 @@ bool add_application(redox::Redox& redis, const std::string& app_name,
 bool add_model_links(redox::Redox& redis, const std::string& app_name,
                      const std::vector<std::string>& model_names);
 
+ /**
+  * Deletes links between the specified app and models.
+  *
+  * \return Returns true if the removal was successful.
+  */
+ bool delete_model_links(redox::Redox& redis, const std::string& app_name,
+                         const std::vector<std::string>& model_names);
+
 /**
  * Deletes a container from the container table if it exists.
  *

src/libclipper/src/redis.cpp

@@ -489,6 +489,22 @@ bool add_model_links(redox::Redox& redis, const std::string& appname,
   }
 }
 
+bool delete_model_links(redox::Redox& redis, const std::string& appname,
+                        const std::vector<std::string>& model_names) {
+  if (send_cmd_no_reply<string>(
+          redis, {"SELECT", std::to_string(REDIS_APP_MODEL_LINKS_DB_NUM)})) {
+    for (auto model_name : model_names) {
+      if (!send_cmd_no_reply<int>(
+              redis, vector<string>{"SREM", appname, model_name})) {
+        return false;
+      }
+    }
+    return true;
+  } else {
+    return false;
+  }
+}
+
 bool delete_application(redox::Redox& redis, const std::string& appname) {
   if (send_cmd_no_reply<string>(
           redis, {"SELECT", std::to_string(REDIS_APPLICATION_DB_NUM)})) {

src/libclipper/test/redis_test.cpp

@@ -134,6 +134,37 @@ TEST_F(RedisTest, AddModelLinks) {
   ASSERT_EQ(model_names, linked_models);
 }
 
+TEST_F(RedisTest, DeleteModelLinks) {
+  std::string app_name = "my_app_name";
+  InputType input_type = InputType::Doubles;
+  std::string policy = DefaultOutputSelectionPolicy::get_name();
+  std::string default_output = "1.0";
+  int latency_slo_micros = 10000;
+  ASSERT_TRUE(add_application(*redis_, app_name, input_type, policy,
+                              default_output, latency_slo_micros));
+
+  std::vector<std::string> labels{"ads", "images", "experimental", "other",
+                                  "labels"};
+  std::string model_name_1 = "model_1";
+  std::string model_name_2 = "model_2";
+  VersionedModelId model_1 = VersionedModelId(model_name_1, "1");
+  VersionedModelId model_2 = VersionedModelId(model_name_2, "1");
+  std::string container_name = "clipper/test_container";
+  std::string model_path = "/tmp/models/m/1";
+  ASSERT_TRUE(add_model(*redis_, model_1, input_type, labels, container_name,
+                        model_path, DEFAULT_BATCH_SIZE));
+  ASSERT_TRUE(add_model(*redis_, model_2, input_type, labels, container_name,
+                        model_path, DEFAULT_BATCH_SIZE));
+
+  std::vector<std::string> model_names =
+      std::vector<std::string>{model_name_1, model_name_2};
+  ASSERT_TRUE(add_model_links(*redis_, app_name, model_names));
+  ASSERT_TRUE(delete_model_links(*redis_, app_name, model_names));
+
+  auto linked_models = get_linked_models(*redis_, app_name);
+  ASSERT_EQ(linked_models.size(), (size_t)0);
+}
+
 TEST_F(RedisTest, SetCurrentModelVersion) {
   std::string model_name = "mymodel";
   ASSERT_TRUE(set_current_model_version(*redis_, model_name, "2"));

src/management/src/management_frontend.hpp

@@ -65,6 +65,7 @@ const std::string GET_ALL_MODELS = ADMIN_PATH + "/get_all_models$";
 const std::string GET_MODEL = ADMIN_PATH + "/get_model$";
 const std::string GET_ALL_CONTAINERS = ADMIN_PATH + "/get_all_containers$";
 const std::string GET_CONTAINER = ADMIN_PATH + "/get_container$";
+const std::string DELETE_MODEL_LINKS = ADMIN_PATH + "/delete_model_links$";
 
 const std::string PING = ADMIN_PATH + "/ping$";
 
@@ -84,6 +85,13 @@ const std::string ADD_MODEL_LINKS_JSON_SCHEMA = R"(
   }
 )";
 
+const std::string DELETE_MODEL_LINKS_JSON_SCHEMA = R"(
+  {
+  "app_name" := string,
+  "model_names" := [string]
+  }
+)";
+
 const std::string GET_LINKED_MODELS_REQUESTS_SCHEMA = R"(
   {
     "app_name" := string
@@ -241,6 +249,27 @@ class RequestHandler {
             respond_http(e.what(), "400 Bad Request", response);
           }
         });
+    server_.add_endpoint(
+        DELETE_MODEL_LINKS, "POST",
+        [this](std::shared_ptr<HttpServer::Response> response,
+               std::shared_ptr<HttpServer::Request> request) {
+          try {
+            clipper::log_info(LOGGING_TAG_MANAGEMENT_FRONTEND,
+                              "Remove application links POST request");
+            std::string result = delete_model_links(request->content.string());
+            respond_http(result, "200 OK", response);
+          } catch (const json_parse_error& e) {
+            std::string err_msg =
+                json_error_msg(e.what(), DELETE_MODEL_LINKS_JSON_SCHEMA);
+            respond_http(err_msg, "400 Bad Request", response);
+          } catch (const json_semantic_error& e) {
+            std::string err_msg =
+                json_error_msg(e.what(), DELETE_MODEL_LINKS_JSON_SCHEMA);
+            respond_http(err_msg, "400 Bad Request", response);
+          } catch (const std::invalid_argument& e) {
+            respond_http(e.what(), "400 Bad Request", response);
+          }
+        });
     server_.add_endpoint(
         ADD_MODEL, "POST",
         [this](std::shared_ptr<HttpServer::Response> response,
@@ -620,6 +649,58 @@ class RequestHandler {
     }
   }
 
+  /**
+   * Creates an endpoint that listens for requests to remove links between
+   * apps and models
+   *
+   * JSON format:
+   * {
+   *  "app_name" := string,
+   *  "model_names" := [string]
+   * }
+   */
+  std::string delete_model_links(const std::string& json) {
+    rapidjson::Document d;
+    parse_json(json, d);
+
+    std::string app_name = get_string(d, "app_name");
+    std::vector<string> model_names = get_string_array(d, "model_names");
+
+    // Confirm that the app exists
+    auto app_info =
+        clipper::redis::get_application(redis_connection_, app_name);
+    if (app_info.size() == 0) {
+      std::stringstream ss;
+      ss << "No app with name " << app_name << " exists.";
+      throw std::invalid_argument(ss.str());
+    }
+
+    // Confirm that the model names supplied are of linked models
+    auto existing_linked_models =
+        clipper::redis::get_linked_models(redis_connection_, app_name);
+
+    for (auto const& model_name : model_names) {
+      if (std::find(existing_linked_models.begin(),
+                    existing_linked_models.end(),
+                    model_name) == existing_linked_models.end()) {
+        std::stringstream ss;
+        ss << "Cannot remove nonexistent link between app " << app_name
+           << " and model " << model_name;
+        throw std::invalid_argument(ss.str());
+      }
+    }
+
+    if (clipper::redis::delete_model_links(redis_connection_, app_name,
+                                           model_names)) {
+      return "Success!";
+    } else {
+      std::stringstream ss;
+      ss << "Error deleting linked models from " << app_name << " in Redis";
+      throw std::invalid_argument(ss.str());
+    }
+  }
+
+
   /**
    * Processes a request to add a new application to Clipper
    *