Support adding image datastores - agent update_config API

Add an agent thrift API (AgentControl.update_config) to allow
updating agent's configuration after provision has completed,
and new configuration can take effect without restarting agent.

Currently this API only updates image datastores.

Change-Id: I91d3e37239b40ee6cefe12ea9d46c328329baee8

Author: longzhou

python/src/agent/agent/agent_config.py

@@ -144,7 +144,7 @@ def _check_and_set_attr(self, attr_name, value):
         return True
 
     @locked
-    def update_config(self, provision_req):
+    def provision(self, provision_req):
         """
         Update the agent configuration using the provisioning request
         configuration.
@@ -238,6 +238,13 @@ def update_config(self, provision_req):
                                      str(self._options))
             self._reboot_required = True
 
+    @locked
+    def update(self, update_config_req):
+        image_datastores = self._convert_image_datastores(update_config_req.image_datastores)
+        if self._check_and_set_attr(self.IMAGE_DATASTORES, image_datastores):
+            self._persist_config()
+            self._trigger_callbacks(self.IMAGE_DATASTORES, image_datastores)
+
     @property
     @locked
     def datastores(self):

python/src/agent/agent/agent_control_handler.py

@@ -25,6 +25,8 @@
 from gen.agent.ttypes import AgentStatusCode
 from gen.agent.ttypes import ProvisionResponse
 from gen.agent.ttypes import ProvisionResultCode
+from gen.agent.ttypes import UpdateConfigResponse
+from gen.agent.ttypes import UpdateConfigResultCode
 from gen.agent.ttypes import UpgradeResponse
 from gen.agent.ttypes import UpgradeResultCode
 from gen.agent.ttypes import VersionResponse
@@ -60,7 +62,7 @@ def provision(self, request):
 
         try:
             agent_config = common.services.get(ServiceName.AGENT_CONFIG)
-            agent_config.update_config(request)
+            agent_config.provision(request)
         except InvalidConfig as e:
             return ProvisionResponse(ProvisionResultCode.INVALID_CONFIG, str(e))
         except Exception, e:
@@ -69,6 +71,24 @@ def provision(self, request):
 
         return ProvisionResponse(ProvisionResultCode.OK)
 
+    @log_request
+    @error_handler(UpdateConfigResponse, UpdateConfigResultCode)
+    def update_config(self, request):
+        """
+        Update agent's configuration.
+        :type request: UpdateConfigRequest
+        :rtype: UpdateConfigResponse
+        """
+
+        try:
+            agent_config = common.services.get(ServiceName.AGENT_CONFIG)
+            agent_config.update(request)
+        except Exception, e:
+            self._logger.warning("Unexpected exception", exc_info=True)
+            return UpdateConfigResponse(UpdateConfigResultCode.SYSTEM_ERROR, str(e))
+
+        return UpdateConfigResponse(UpdateConfigResultCode.OK)
+
     @log_request
     @error_handler(UpgradeResponse, UpgradeResultCode)
     def upgrade(self, request):

python/src/agent/agent/tests/unit/test_agent_config.py

@@ -23,7 +23,7 @@
 from common.mode import Mode
 from common.service_name import ServiceName
 from common.state import State
-from gen.agent.ttypes import ProvisionRequest
+from gen.agent.ttypes import ProvisionRequest, UpdateConfigRequest
 from gen.common.ttypes import ServerAddress
 from gen.resource.ttypes import ImageDatastore
 from gen.stats.plugin.ttypes import StatsPluginConfig
@@ -138,7 +138,7 @@ def test_agent_config_update(self):
 
         req.host_id = "host1"
         req.deployment_id = "deployment1"
-        self.agent.update_config(req)
+        self.agent.provision(req)
 
         assert_that(self.agent.stats_store_endpoint, equal_to("10.0.0.100"))
         assert_that(self.agent.stats_store_port, equal_to(8081))
@@ -158,7 +158,7 @@ def test_agent_config_update(self):
         # Verify we are able to unset all the configuration.
         req = ProvisionRequest()
 
-        self.agent.update_config(req)
+        self.agent.provision(req)
         assert_that(self.agent.hostname, equal_to(None))
         assert_that(self.agent.host_port, equal_to(8835))
         assert_that(self.agent.datastores, equal_to([]))
@@ -178,7 +178,7 @@ def test_agent_config_update(self):
         req.address = addr
 
         # Verify an exception is raised.
-        self.assertRaises(InvalidConfig, self.agent.update_config, req)
+        self.assertRaises(InvalidConfig, self.agent.provision, req)
         assert_that(self.agent.hostname, equal_to(None))
         assert_that(self.agent.host_port, equal_to(8835))
         assert_that(self.agent.datastores, equal_to([]))
@@ -200,7 +200,7 @@ def test_reboot_required(self):
         req.stats_plugin_config.enabled = False
         addr = ServerAddress(host="localhost", port=2345)
         req.address = addr
-        self.agent.update_config(req)
+        self.agent.provision(req)
         # Verify that the bootstrap is still false as zk config is not
         # specified.
         self.assertFalse(self.agent.bootstrap_ready)
@@ -210,7 +210,7 @@ def test_reboot_required(self):
         req.datastores = ["ds3", "ds4"]
         addr = ServerAddress(host="localhost", port=2345)
         req.address = addr
-        self.agent.update_config(req)
+        self.agent.provision(req)
         self.assertTrue(self.agent.reboot_required)
 
     def test_thrift_thread_settings(self):
@@ -253,15 +253,27 @@ def test_load_image_datastores(self):
             {"name": "ds2", "used_for_vms": False},
         ]
         req = ProvisionRequest()
-        req.datastores = ["ds1", "ds2", "ds3"]
-        req.image_datastores = set([ImageDatastore("ds1", True),
-                                    ImageDatastore("ds2", False)])
-        self.agent.update_config(req)
-        self.agent._persist_config()
+        req.datastores = ["ds1", "ds2", "ds3", "ds4"]
+        req.image_datastores = {ImageDatastore("ds1", True), ImageDatastore("ds2", False)}
+        self.agent.provision(req)
         self.agent._load_config()
-        assert_that(self.agent.datastores, equal_to(["ds1", "ds2", "ds3"]))
-        assert_that(self.agent.image_datastores,
-                    contains_inanyorder(*expected_image_ds))
+        assert_that(self.agent.datastores, equal_to(["ds1", "ds2", "ds3", "ds4"]))
+        assert_that(self.agent.image_datastores, contains_inanyorder(*expected_image_ds))
+
+        imageds_callback = mock.MagicMock()
+        self.agent.on_config_change(self.agent.IMAGE_DATASTORES, imageds_callback)
+        req = UpdateConfigRequest()
+        req.image_datastores = {ImageDatastore("ds1", True), ImageDatastore("ds2", False), ImageDatastore("ds3", True)}
+        self.agent.update(req)
+        expected_image_ds = [
+            {"name": "ds1", "used_for_vms": True},
+            {"name": "ds2", "used_for_vms": False},
+            {"name": "ds3", "used_for_vms": True},
+        ]
+        assert_that(self.agent.image_datastores, contains_inanyorder(*expected_image_ds))
+        imageds_callback.assert_called_once_with(self.agent.image_datastores)
+        self.agent._load_config()
+        assert_that(self.agent.image_datastores, contains_inanyorder(*expected_image_ds))
 
     def test_config_change(self):
         # Test cpu_overcommit and memory_overcommit config change
@@ -273,10 +285,9 @@ def test_config_change(self):
         provision.memory_overcommit = 6.0
         self.agent.on_config_change(self.agent.CPU_OVERCOMMIT, cpu_callback)
         self.agent.on_config_change(self.agent.MEMORY_OVERCOMMIT, mem_callback)
-        self.agent.update_config(provision)
+        self.agent.provision(provision)
         cpu_callback.assert_called_once_with(5.0)
         mem_callback.assert_called_once_with(6.0)
 
-
 if __name__ == "__main__":
     unittest.main()

python/src/host/host/hypervisor/datastore_manager.py

@@ -169,6 +169,10 @@ def datastores_updated(self):
         """vim client callback for datastore change"""
         self._initialize_datastores()
 
+    def set_image_datastores(self, image_datastores):
+        self._configured_image_datastores = image_datastores
+        self._initialize_datastores()
+
     def datastore_type(self, datastore_id):
         """ Get datastore_type from datastore_id
 

python/src/host/host/hypervisor/hypervisor.py

@@ -108,3 +108,6 @@ def cpu_overcommit(self):
 
     def set_cpu_overcommit(self, value):
         self.placement_manager.cpu_overcommit = value
+
+    def set_image_datastores(self, value):
+        self.datastore_manager.set_image_datastores(value)

python/src/host/host/plugin.py

@@ -32,10 +32,9 @@ def init(self):
         hv = hypervisor.Hypervisor(config)
 
         # When configuration changes, notify hypervisor
-        config.on_config_change(config.CPU_OVERCOMMIT,
-                                hv.set_cpu_overcommit)
-        config.on_config_change(config.MEMORY_OVERCOMMIT,
-                                hv.set_memory_overcommit)
+        config.on_config_change(config.CPU_OVERCOMMIT, hv.set_cpu_overcommit)
+        config.on_config_change(config.MEMORY_OVERCOMMIT, hv.set_memory_overcommit)
+        config.on_config_change(config.IMAGE_DATASTORES, hv.set_image_datastores)
 
         # Register hypervisor in services
         common.services.register(ServiceName.HYPERVISOR, hv)

python/src/tools/bin/eccli-config-update

@@ -0,0 +1,31 @@
+#!/usr/bin/env python
+# Copyright (c) 2015 VMware, Inc. All Rights Reserved.
+import sys
+
+from eccli.format import print_request
+from eccli.format import print_response
+from eccli.optparser import default_parser
+from eccli.thrift import get_client
+from gen.agent.ttypes import UpdateConfigRequest
+from gen.resource.ttypes import ImageDatastore
+
+parser = default_parser(usage="eccli-config-update [options]",
+                        add_help=True)
+parser.add_option("-d", "--image_datastores",
+                  action="store", type="string", dest="imageds",
+                  help="list of image datastore names (e.g. ds1,ds2)")
+(options, args) = parser.parse_args()
+
+if not options.imageds:
+    print >> sys.stderr, "Error: image datastores are required\n"
+    parser.print_help()
+    exit(1)
+
+client = get_client(options.host, "AgentControl")
+
+request = UpdateConfigRequest()
+request.image_datastores = set([ImageDatastore(name=ds, used_for_vms=True) for ds in options.imageds.split(',')])
+
+print_request(request)
+response = client.update_config(request)
+print_response(response)

thrift/agent.thrift

@@ -78,20 +78,6 @@ struct VersionResponse {
   4: optional string revision
 }
 
-
-// Structure describing the refcount preserved with the image.
-// All fields are optional for future compatibility reasons.
-struct RefCount {
-  // The generation number of the ref count file.
-  1: optional i16 generation_num
-  // Version of refcount implementation.
-  // If unset assume initial version.
-  2: optional byte version
-  3: optional bool tombstone
-  4: optional i16 ref_count
-  5: optional binary vm_ids
-}
-
 // Struct describing the provisioning configuration of the esx agent.
 struct ProvisionRequest {
   // The datastores to use for cloud virtual machine workloads
@@ -107,9 +93,6 @@ struct ProvisionRequest {
   // i.e. no overcommit
   8: optional double memory_overcommit
 
-  // The information about the image datastore configuration
-  10: optional resource.ImageDatastore image_datastore_info
-
   // The cpu overcommit for this host. If unspecified it defaults to 1.0,
   // i.e. no overcommit
   11: optional double cpu_overcommit
@@ -124,7 +107,6 @@ struct ProvisionRequest {
   14: optional string ntp_endpoint
 
   // A set of image datastores for this host.
-  // The image_datastore_info field will be deprecated.
   15: optional set<resource.ImageDatastore> image_datastores
 
   // Configuration of the Stats Plugin
@@ -160,6 +142,31 @@ struct ProvisionResponse {
   2: optional string error
 }
 
+// Struct describing the updating configuration of the esx agent.
+struct UpdateConfigRequest {
+  // A set of image datastores for this host.
+  1: optional set<resource.ImageDatastore> image_datastores
+
+  99: optional tracing.TracingInfo tracing_info
+}
+
+// Update config result code
+enum UpdateConfigResultCode {
+  // Update config was successful.
+  OK = 0
+  // The configuration provided is invalid.
+  INVALID_CONFIG = 1
+
+  // Catch all error
+  SYSTEM_ERROR = 15
+}
+
+// Update config response
+struct UpdateConfigResponse {
+  1: required UpdateConfigResultCode result
+  2: optional string error
+}
+
 // Upgrade request
 struct UpgradeRequest {
   99: optional tracing.TracingInfo tracing_info
@@ -205,6 +212,9 @@ service AgentControl {
   // Method to provision an agent for esxcloud purposes.
   ProvisionResponse provision(1: ProvisionRequest request)
 
+  // Method to update agent's configuration.
+  UpdateConfigResponse update_config(1: UpdateConfigRequest request)
+
   // Method to upgrade an agent.
   UpgradeResponse upgrade(1: UpgradeRequest request)
 

thrift/resource.thrift

@@ -111,11 +111,6 @@ struct ImageDatastore {
   2: required bool used_for_vms
 }
 
-// FaultDomain
-struct FaultDomain {
-  1: required string id
-}
-
 // ResourceConstraint
 struct ResourceConstraint {
   1: required ResourceConstraintType type