Address comments on sidecar, store updates in memory, rename base field

Signed-off-by: Kunjan Patel <[email protected]>

Author: coolkp

examples/dynamic-lora-sidecar/sidecar/configmap.yaml

@@ -1,4 +1,4 @@
-deployment:
+vLLMLoRAConfig:
   host: localhost
   models:
   - base-model: meta-llama/Llama-2-7b-hf

examples/dynamic-lora-sidecar/sidecar/sidecar.py

@@ -8,6 +8,7 @@
 import os
 
 CONFIG_MAP_FILE = os.environ.get('DYNAMIC_LORA_ROLLOUT_CONFIG',"configmap.yaml")
+BASE_FIELD = "vLLMLoRAConfig"
 logging.basicConfig(level=logging.INFO, 
                     format='%(asctime)s - %(levelname)s - %(message)s')
 
@@ -48,7 +49,7 @@ def __init__(self):
 
     def load_configmap(self):
         with open(CONFIG_MAP_FILE, "r") as f:
-            deployment = yaml.safe_load(f)["deployment"]
+            deployment = yaml.safe_load(f)[BASE_FIELD]
             self.deployment_name = deployment.get("name", "")
             lora_adapters = deployment["models"]
             self.host, self.port = (
@@ -88,25 +89,26 @@ def reconcile(self):
                     "operation": "load",
                     "errors": [e],
                 }
-        self.update_status_config()
+        self.log_status_config()
 
-    def update_status_config(self):
+    def log_status_config(self):
         models = list(self.config_map_adapters.values())
         deployment = {
             "name": self.deployment_name,
             "host": self.host,
             "port": self.port,
             "models": models,
         }
-        config = {"deployment":deployment}
-        with open(CONFIG_MAP_FILE, "w") as f:
-            yaml.dump(config, f, indent=2)
+        config = {BASE_FIELD:deployment}
+        yaml_string = yaml.dump(config,indent=2)
+        logging.info(f"current status of lora adapters on model server at {self.host}:{self.port} \n {yaml_string}")
+        
 
     def load_adapter(self, adapter):
         """Sends a request to load the specified model."""
         adapter_id = adapter["id"]
         if adapter_id in self.registered_adapters or adapter.get("toRemove"):
-            return "already loaded"
+            return 
         url = f"http://{self.host}:{self.port}/v1/load_lora_adapter"
         payload = {
             "lora_name": adapter_id,
@@ -127,7 +129,7 @@ def unload_adapter(self, adapter):
         """Sends a request to unload the specified model."""
         adapter_id = adapter["id"]
         if adapter_id not in self.registered_adapters:
-            return "already unloaded"
+            return 
         url = f"http://{self.host}:{self.port}/v1/unload_lora_adapter"
         payload = {"lora_name": adapter_id}
         try:

examples/dynamic-lora-sidecar/sidecar/test_sidecar.py

@@ -1,10 +1,10 @@
 import unittest
 from unittest.mock import patch, Mock, mock_open
 import yaml
-from sidecar import LoraReconciler, CONFIG_MAP_FILE
+from sidecar import LoraReconciler, CONFIG_MAP_FILE, BASE_FIELD
 
 TEST_CONFIG_DATA = {
-    "deployment": {
+    "vLLMLoRAConfig": {
         "name": "test-deployment",
         "host": "localhost",
         "port": "8000",
@@ -101,16 +101,10 @@ def test_load_adapter(self, mock_post):
 
         # loading a new adapter
         result = self.reconciler.load_adapter(
-            TEST_CONFIG_DATA["deployment"]["models"][0]
+            TEST_CONFIG_DATA[BASE_FIELD]["models"][0]
         )
         self.assertEqual(result, "")
 
-        # loading an already loaded adapter
-        self.reconciler.registered_adapters["lora1"] = {"id": "lora1"}
-        result = self.reconciler.load_adapter(
-            TEST_CONFIG_DATA["deployment"]["models"][0]
-        )
-        self.assertEqual(result, "already loaded")
 
     @patch("sidecar.requests.post")
     def test_unload_adapter(self, mock_post):
@@ -119,15 +113,9 @@ def test_unload_adapter(self, mock_post):
         # unloading an existing adapter
         self.reconciler.registered_adapters["lora2"] = {"id": "lora2"}
         result = self.reconciler.unload_adapter(
-            TEST_CONFIG_DATA["deployment"]["models"][1]
+            TEST_CONFIG_DATA[BASE_FIELD]["models"][1]
         )
         self.assertEqual(result, None)
-
-        # unloading an already unloaded adapter
-        result = self.reconciler.unload_adapter(
-            TEST_CONFIG_DATA["deployment"]["models"][1]
-        )
-        self.assertEqual(result, "already unloaded")
 
     @patch("builtins.open", new_callable=mock_open, read_data=yaml.dump(TEST_CONFIG_DATA))
     @patch("sidecar.requests.get")
@@ -154,8 +142,6 @@ def test_reconcile(self, mock_post, mock_get, mock_file):
         )
         updated_config = self.reconciler.config_map_adapters
         mock_file.return_value.write.side_effect = lambda data: data
-        self.reconciler.update_status_config()
-        mock_file.return_value.write.assert_called()
         self.assertTrue("timestamp" in updated_config["lora1"]["status"])
         self.assertTrue("status" in updated_config["lora2"])