On Thu Aug 21 2014 at 12:01:48 PM Aaron Karper <[email protected]> wrote:
> Thanks for the review! > > On Wed, Aug 20, 2014 at 04:59:11PM +0200, Michele Tartara wrote: > > Il 19/ago/2014 10:37 "'Aaron Karper' via ganeti-devel" < > > [email protected]> ha scritto: > > > > > > In order to modify the data collectors displayed, there are the options > > > --activate-data-collector=[inst-status-xen ..] and > > > --deactivate-data-collector=[inst-status-xen ..] > > > > > > Includes bootstrap.py and cfgupgrade.py setting data_collectors config > > > values. > > > > > > Implements 870 > > > > Actually, it's the other patch fixing the issue with the xen collector. > > This one is just an evolution of the fix. I'd remove this line. > > - Implements 870 > > > > > > > > > Signed-off-by: Aaron Karper <[email protected]> > > > --- > > > lib/bootstrap.py | 4 ++++ > > > lib/client/gnt_cluster.py | 28 ++++++++++++++++++++++++++-- > > > lib/cmdlib/cluster.py | 6 ++++++ > > > lib/objects.py | 2 +- > > > src/Ganeti/OpCodes.hs | 2 ++ > > > src/Ganeti/OpParams.hs | 14 ++++++++++++++ > > > test/py/cfgupgrade_unittest.py | 7 +++++++ > > > tools/cfgupgrade | 2 -- > > > 8 files changed, 60 insertions(+), 5 deletions(-) > > > > > > diff --git a/lib/bootstrap.py b/lib/bootstrap.py > > > index 5f79dd1..3da5f78 100644 > > > --- a/lib/bootstrap.py > > > +++ b/lib/bootstrap.py > > > @@ -799,6 +799,9 @@ def InitCluster(cluster_name, mac_prefix, # pylint: > > disable=R0913, R0914 > > > if compression_tools is not None: > > > cluster.CheckCompressionTools(compression_tools) > > > > > > + data_collectors = dict((name, dict(active=True)) for name in > > > + constants.DATA_COLLECTOR_NAMES) > > > + > > > # init of cluster config file > > > cluster_config = objects.Cluster( > > > serial_no=1, > > > @@ -828,6 +831,7 @@ def InitCluster(cluster_name, mac_prefix, # pylint: > > disable=R0913, R0914 > > > ctime=now, > > > mtime=now, > > > maintain_node_health=maintain_node_health, > > > + data_collectors=data_collectors, > > > drbd_usermode_helper=drbd_helper, > > > default_iallocator=default_iallocator, > > > default_iallocator_params=default_iallocator_params, > > > diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py > > > index 655cb43..67cab0a 100644 > > > --- a/lib/client/gnt_cluster.py > > > +++ b/lib/client/gnt_cluster.py > > > @@ -75,6 +75,16 @@ TO_OPT = cli_option("--to", default=None, > > type="string", > > > RESUME_OPT = cli_option("--resume", default=False, > action="store_true", > > > help="Resume any pending Ganeti upgrades") > > > > > > +ACTIVATE_DATA_COLLECTOR = cli_option( > > > + "--activate-data-collector", > > > + action="append", choices=list(constants.DATA_COLLECTOR_NAMES), > > > + help="Reactivate a data collector for reporting.") > > > + > > > +DEACTIVATE_DATA_COLLECTOR = cli_option( > > > + "--deactivate-data-collector", > > > + action="append", choices=list(constants.DATA_COLLECTOR_NAMES), > > > + help="Deactivate a data collector for reporting.") > > > > In similar situations we tend to use enable/disable instead of > > activate/deactivate. Could you please rename them, for consistency? > > Done, the changes are in the patch attached. > > > > + > > > _EPO_PING_INTERVAL = 30 # 30 seconds between pings > > > _EPO_PING_TIMEOUT = 1 # 1 second > > > _EPO_REACHABLE_TIMEOUT = 15 * 60 # 15 minutes > > > @@ -1173,7 +1183,9 @@ def SetClusterParams(opts, args): > > > opts.shared_file_storage_dir is not None or > > > opts.compression_tools is not None or > > > opts.shared_file_storage_dir is not None or > > > - opts.enabled_user_shutdown is not None): > > > + opts.enabled_user_shutdown is not None or > > > + opts.activate_data_collector is not None or > > > + opts.deactivate_data_collector is not None): > > > ToStderr("Please give at least one of the parameters.") > > > return 1 > > > > > > @@ -1256,6 +1268,14 @@ def SetClusterParams(opts, args): > > > > > > compression_tools = _GetCompressionTools(opts) > > > > > > + activate_dcs = set(opts.activate_data_collector or []) > > > + deactivate_dcs = set(opts.deactivate_data_collector or []) > > > + activate_and_deactivate = activate_dcs & deactivate_dcs > > > + if activate_and_deactivate: > > > + ToStderr("Can't activate and deactivate %s at the same time." % > > > + sorted(activate_and_deactivate)) > > > + return 1 > > > + > > > op = opcodes.OpClusterSetParams( > > > vg_name=vg_name, > > > drbd_helper=drbd_helper, > > > @@ -1294,6 +1314,8 @@ def SetClusterParams(opts, args): > > > shared_file_storage_dir=opts.shared_file_storage_dir, > > > compression_tools=compression_tools, > > > enabled_user_shutdown=opts.enabled_user_shutdown, > > > + activate_data_collectors=list(activate_dcs), > > > + deactivate_data_collectors=list(deactivate_dcs), > > > ) > > > return base.GetResult(None, opts, SubmitOrSend(op, opts)) > > > > > > @@ -2279,7 +2301,9 @@ commands = { > > > ENABLED_USER_SHUTDOWN_OPT] + > > > INSTANCE_POLICY_OPTS + > > > [GLOBAL_FILEDIR_OPT, GLOBAL_SHARED_FILEDIR_OPT, > ZEROING_IMAGE_OPT, > > > - COMPRESSION_TOOLS_OPT], > > > + COMPRESSION_TOOLS_OPT] + > > > + [ACTIVATE_DATA_COLLECTOR, > > > + DEACTIVATE_DATA_COLLECTOR], > > > "[opts...]", > > > "Alters the parameters of the cluster"), > > > "renew-crypto": ( > > > diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py > > > index 10c9d8c..2ddd81d 100644 > > > --- a/lib/cmdlib/cluster.py > > > +++ b/lib/cmdlib/cluster.py > > > @@ -1559,6 +1559,12 @@ class LUClusterSetParams(LogicalUnit): > > > > > > ensure_kvmd = False > > > > > > + for name in self.op.activate_data_collectors: > > > + self.cluster.data_collectors[name]["active"] = True > > > > "Active" here is like a magic number. Please add a constant for it and > > substitute all of its uses. > > Done. > > > > > > + > > > + for name in self.op.deactivate_data_collectors: > > > + self.cluster.data_collectors[name]["active"] = False > > > + > > > if self.op.hvparams: > > > self.cluster.hvparams = self.new_hvparams > > > if self.op.os_hvp: > > > diff --git a/lib/objects.py b/lib/objects.py > > > index 2fa72a0..02ea411 100644 > > > --- a/lib/objects.py > > > +++ b/lib/objects.py > > > @@ -407,7 +407,6 @@ class ConfigData(ConfigObject): > > > "networks", > > > "disks", > > > "serial_no", > > > - "datacollectors", > > > ] + _TIMESTAMPS > > > > > > def ToDict(self, _with_private=False): > > > @@ -1618,6 +1617,7 @@ class Cluster(TaggableObject): > > > "zeroing_image", > > > "compression_tools", > > > "enabled_user_shutdown", > > > + "data_collectors", > > > ] + _TIMESTAMPS + _UUID > > > > > > def UpgradeConfig(self): > > > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs > > > index 635cd1b..858d629 100644 > > > --- a/src/Ganeti/OpCodes.hs > > > +++ b/src/Ganeti/OpCodes.hs > > > @@ -246,6 +246,8 @@ $(genOpCode "OpCode" > > > , pZeroingImage > > > , pCompressionTools > > > , pEnabledUserShutdown > > > + , pActivateDataCollectors > > > + , pDeactivateDataCollectors > > > ], > > > []) > > > , ("OpClusterRedistConf", > > > diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs > > > index 413fae9..83255d4 100644 > > > --- a/src/Ganeti/OpParams.hs > > > +++ b/src/Ganeti/OpParams.hs > > > @@ -282,6 +282,8 @@ module Ganeti.OpParams > > > , pEnabledDiskTemplates > > > , pEnabledUserShutdown > > > , pAdminStateSource > > > + , pActivateDataCollectors > > > + , pDeactivateDataCollectors > > > ) where > > > > > > import Control.Monad (liftM, mplus) > > > @@ -1829,3 +1831,15 @@ pNetworkVlan :: Field > > > pNetworkVlan = > > > withDoc "Network vlan when connecting to a group" . > > > defaultField [| "" |] $ stringField "network_vlan" > > > + > > > +pActivateDataCollectors :: Field > > > +pActivateDataCollectors = > > > + withDoc "Reactivate the data collectors" . > > > + defaultField [| emptyListSet |] $ > > > + simpleField "activate_data_collectors" [t| ListSet String |] > > > + > > > +pDeactivateDataCollectors :: Field > > > +pDeactivateDataCollectors = > > > + withDoc "Deactivate the data collectors" . > > > + defaultField [| emptyListSet |] $ > > > + simpleField "deactivate_data_collectors" [t| ListSet String |] > > > diff --git a/test/py/cfgupgrade_unittest.py > > b/test/py/cfgupgrade_unittest.py > > > index 3f8c3cf..b61967e 100755 > > > --- a/test/py/cfgupgrade_unittest.py > > > +++ b/test/py/cfgupgrade_unittest.py > > > @@ -54,6 +54,13 @@ def GetMinimalConfig(): > > > "zeroing_image": "", > > > "compression_tools": constants.IEC_DEFAULT_TOOLS, > > > "enabled_user_shutdown": False, > > > + "data_collectors": { > > > + "diskstats": { "active": True }, > > > + "drbd": { "active": True }, > > > + "lv": { "active": True }, > > > + "inst-status-xen": { "active": True }, > > > + "cpu-avg-load": { "active": True }, > > > + }, > > > }, > > > "instances": {}, > > > "disks": {}, > > > diff --git a/tools/cfgupgrade b/tools/cfgupgrade > > > index b97ebd4..78e26d9 100755 > > > --- a/tools/cfgupgrade > > > +++ b/tools/cfgupgrade > > > @@ -437,8 +437,6 @@ def UpgradeTopLevelDisks(config_data): > > > iobj["disks"] = disk_uuids > > > > > > > > > - > > > - > > > def UpgradeAll(config_data): > > > config_data["version"] = version.BuildVersion(TARGET_MAJOR, > > TARGET_MINOR, 0) > > > UpgradeRapiUsers() > > > -- > > > 2.1.0.rc2.206.gedb03e5 > > > > > > > Thanks, > > Michele > LGTM, thanks. Could you please send it in the standard format, with git send-email? Cheers, Michele
