From 67302021511807cf4a2ea41bfde60f5dbb5a6f60 Mon Sep 17 00:00:00 2001 From: Joe Cropper Date: Tue, 19 Jul 2016 09:31:23 -0700 Subject: [PATCH] Fix typos and messages in strategies This patch fixes various typos and other nits in the strategies. It also updates some of the log messages to be a little more operator friendly. Change-Id: Ic9268c6d7376dad215a1a40798485b1d836ba7ae Closes-Bug: #1604248 --- watcher/decision_engine/audit/continuous.py | 4 +- watcher/decision_engine/manager.py | 8 ++-- .../strategies/basic_consolidation.py | 30 ++++++------ .../strategy/strategies/dummy_strategy.py | 4 +- .../strategies/outlet_temp_control.py | 1 - .../strategy/strategies/uniform_airflow.py | 47 +++++++++---------- .../strategies/vm_workload_consolidation.py | 2 +- .../strategy/strategies/workload_balance.py | 27 +++++------ 8 files changed, 61 insertions(+), 62 deletions(-) diff --git a/watcher/decision_engine/audit/continuous.py b/watcher/decision_engine/audit/continuous.py index 3aaf09e8a..137f1a55f 100644 --- a/watcher/decision_engine/audit/continuous.py +++ b/watcher/decision_engine/audit/continuous.py @@ -33,8 +33,8 @@ CONF = cfg.CONF WATCHER_CONTINUOUS_OPTS = [ cfg.IntOpt('continuous_audit_interval', default=10, - help='Interval, in seconds, for checking new created' - 'continuous audit.') + help='Interval (in seconds) for checking newly created ' + 'continuous audits.') ] CONF.register_opts(WATCHER_CONTINUOUS_OPTS, 'watcher_decision_engine') diff --git a/watcher/decision_engine/manager.py b/watcher/decision_engine/manager.py index 009ddef5f..e09af3b57 100644 --- a/watcher/decision_engine/manager.py +++ b/watcher/decision_engine/manager.py @@ -47,19 +47,19 @@ CONF = cfg.CONF WATCHER_DECISION_ENGINE_OPTS = [ cfg.StrOpt('conductor_topic', default='watcher.decision.control', - help='The topic name used for' + help='The topic name used for ' 'control events, this topic ' - 'used for rpc call '), + 'used for RPC calls'), cfg.StrOpt('status_topic', default='watcher.decision.status', help='The topic name used for ' - 'status events, this topic ' + 'status events; this topic ' 'is used so as to notify' 'the others components ' 'of the system'), cfg.StrOpt('publisher_id', default='watcher.decision.api', - help='The identifier used by watcher ' + help='The identifier used by the Watcher ' 'module on the message broker'), cfg.IntOpt('max_workers', default=2, diff --git a/watcher/decision_engine/strategy/strategies/basic_consolidation.py b/watcher/decision_engine/strategy/strategies/basic_consolidation.py index 07a82a97a..56c8fc9ff 100644 --- a/watcher/decision_engine/strategy/strategies/basic_consolidation.py +++ b/watcher/decision_engine/strategy/strategies/basic_consolidation.py @@ -58,7 +58,7 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): - It has been developed only for tests. - It assumes that the virtual machine and the compute node are on the same private network. - - It assume that live migrations are possible + - It assumes that live migrations are possible. *Spec URL* @@ -92,20 +92,20 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): self._ceilometer = None - # TODO(jed) improve threshold overbooking ?,... + # TODO(jed): improve threshold overbooking? self.threshold_mem = 1 self.threshold_disk = 1 self.threshold_cores = 1 - # TODO(jed) target efficacy + # TODO(jed): target efficacy self.target_efficacy = 60 - # TODO(jed) weight + # TODO(jed): weight self.weight_cpu = 1 self.weight_mem = 1 self.weight_disk = 1 - # TODO(jed) bound migration attempts (80 %) + # TODO(jed): bound migration attempts (80 %) self.bound_migration = 0.80 @classmethod @@ -149,7 +149,7 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): if src_hypervisor == dest_hypervisor: return False - LOG.debug('Migrate VM %s from %s to %s', + LOG.debug('Migrate VM %s from %s to %s', vm_to_mig, src_hypervisor, dest_hypervisor) total_cores = 0 @@ -181,13 +181,14 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): total_disk, total_mem): """Check threshold - check the threshold value defined by the ratio of + Check the threshold value defined by the ratio of aggregated CPU capacity of VMs on one node to CPU capacity of this node must not exceed the threshold value. + :param dest_hypervisor: the destination of the virtual machine - :param total_cores - :param total_disk - :param total_mem + :param total_cores: total cores of the virtual machine + :param total_disk: total disk size used by the virtual machine + :param total_mem: total memory used by the virtual machine :return: True if the threshold is not exceed """ cpu_capacity = self.model.get_resource_from_id( @@ -205,7 +206,8 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): """Allowed migration Maximum allowed number of migrations this allows us to fix - the upper bound of the number of migrations + the upper bound of the number of migrations. + :return: """ return self.migration_attempts @@ -232,7 +234,7 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): score_cores = (1 - (float(cpu_capacity) - float(total_cores_used)) / float(cpu_capacity)) - # It's possible that disk_capacity is 0, e.g. m1.nano.disk = 0 + # It's possible that disk_capacity is 0, e.g., m1.nano.disk = 0 if disk_capacity == 0: score_disk = 0 else: @@ -242,7 +244,7 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): score_memory = ( 1 - (float(memory_capacity) - float(total_memory_used)) / float(memory_capacity)) - # todo(jed) take in account weight + # TODO(jed): take in account weight return (score_cores + score_disk + score_memory) / 3 def calculate_score_node(self, hypervisor): @@ -351,7 +353,7 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): return score def node_and_vm_score(self, sorted_score, score): - """Get List of VMs from Node""" + """Get List of VMs from node""" node_to_release = sorted_score[len(score) - 1][0] vms_to_mig = self.model.get_mapping().get_node_vms_from_id( node_to_release) diff --git a/watcher/decision_engine/strategy/strategies/dummy_strategy.py b/watcher/decision_engine/strategy/strategies/dummy_strategy.py index 253b6e6ff..e191875de 100644 --- a/watcher/decision_engine/strategy/strategies/dummy_strategy.py +++ b/watcher/decision_engine/strategy/strategies/dummy_strategy.py @@ -30,8 +30,8 @@ class DummyStrategy(base.DummyBaseStrategy): *Description* - This strategy does not provide any useful optimization. Indeed, its only - purpose is to be used by Tempest tests. + This strategy does not provide any useful optimization. Its only purpose + is to be used by Tempest tests. *Requirements* diff --git a/watcher/decision_engine/strategy/strategies/outlet_temp_control.py b/watcher/decision_engine/strategy/strategies/outlet_temp_control.py index dbe9f714b..c5b66b369 100644 --- a/watcher/decision_engine/strategy/strategies/outlet_temp_control.py +++ b/watcher/decision_engine/strategy/strategies/outlet_temp_control.py @@ -73,7 +73,6 @@ class OutletTempControl(base.ThermalOptimizationBaseStrategy): # The meter to report outlet temperature in ceilometer METER_NAME = "hardware.ipmi.node.outlet_temperature" - MIGRATION = "migrate" def __init__(self, config, osc=None): diff --git a/watcher/decision_engine/strategy/strategies/uniform_airflow.py b/watcher/decision_engine/strategy/strategies/uniform_airflow.py index 95ab9de99..290c06814 100644 --- a/watcher/decision_engine/strategy/strategies/uniform_airflow.py +++ b/watcher/decision_engine/strategy/strategies/uniform_airflow.py @@ -33,13 +33,13 @@ class UniformAirflow(base.BaseStrategy): *Description* - It is a migration strategy based on the Airflow of physical - servers. It generates solutions to move vm whenever a server's - Airflow is higher than the specified threshold. + It is a migration strategy based on the airflow of physical + servers. It generates solutions to move VM whenever a server's + airflow is higher than the specified threshold. *Requirements* - * Hardware: compute node with NodeManager3.0 support + * Hardware: compute node with NodeManager 3.0 support * Software: Ceilometer component ceilometer-agent-compute running in each compute node, and Ceilometer API can report such telemetry "airflow, system power, inlet temperature" successfully. @@ -47,12 +47,11 @@ class UniformAirflow(base.BaseStrategy): *Limitations* - - This is a proof of concept that is not meant to be used in production + - This is a proof of concept that is not meant to be used in production. - We cannot forecast how many servers should be migrated. This is the reason why we only plan a single virtual machine migration at a time. So it's better to use this algorithm with `CONTINUOUS` audits. - - It assume that live migrations are possible - + - It assumes that live migrations are possible. """ # The meter to report Airflow of physical server in ceilometer @@ -82,7 +81,7 @@ class UniformAirflow(base.BaseStrategy): :param osc: an OpenStackClients object """ super(UniformAirflow, self).__init__(config, osc) - # The migration plan will be triggered when the Ariflow reaches + # The migration plan will be triggered when the airflow reaches # threshold # TODO(Junjie): Threshold should be configurable for each audit self.threshold_airflow = self.THRESHOLD_AIRFLOW @@ -110,11 +109,11 @@ class UniformAirflow(base.BaseStrategy): @classmethod def get_display_name(cls): - return _("uniform airflow migration strategy") + return _("Uniform airflow migration strategy") @classmethod def get_translatable_display_name(cls): - return "uniform airflow migration strategy" + return "Uniform airflow migration strategy" @classmethod def get_goal_name(cls): @@ -122,7 +121,7 @@ class UniformAirflow(base.BaseStrategy): @classmethod def get_goal_display_name(cls): - return _("AIRFLOW optimization") + return _("Airflow optimization") @classmethod def get_translatable_goal_display_name(cls): @@ -130,7 +129,7 @@ class UniformAirflow(base.BaseStrategy): def calculate_used_resource(self, hypervisor, cap_cores, cap_mem, cap_disk): - '''calculate the used vcpus, memory and disk based on VM flavors''' + """Calculate the used vcpus, memory and disk based on VM flavors""" vms = self.model.get_mapping().get_node_vms(hypervisor) vcpus_used = 0 memory_mb_used = 0 @@ -144,7 +143,7 @@ class UniformAirflow(base.BaseStrategy): return vcpus_used, memory_mb_used, disk_gb_used def choose_vm_to_migrate(self, hosts): - """pick up an active vm instance to migrate from provided hosts + """Pick up an active vm instance to migrate from provided hosts :param hosts: the array of dict which contains hypervisor object """ @@ -172,7 +171,7 @@ class UniformAirflow(base.BaseStrategy): vm = self.model.get_vm_from_id(vm_id) vms_tobe_migrate.append(vm) except wexc.InstanceNotFound: - LOG.error(_LE("VM not found Error: %s"), vm_id) + LOG.error(_LE("VM not found; error: %s"), vm_id) return source_hypervisor, vms_tobe_migrate else: # migrate the first active vm @@ -180,19 +179,19 @@ class UniformAirflow(base.BaseStrategy): try: vm = self.model.get_vm_from_id(vm_id) if vm.state != vm_state.VMState.ACTIVE.value: - LOG.info(_LE("VM not active, skipped: %s"), + LOG.info(_LE("VM not active; skipped: %s"), vm.uuid) continue vms_tobe_migrate.append(vm) return source_hypervisor, vms_tobe_migrate except wexc.InstanceNotFound: - LOG.error(_LE("VM not found Error: %s"), vm_id) + LOG.error(_LE("VM not found; error: %s"), vm_id) else: - LOG.info(_LI("VM not found from hypervisor: %s"), + LOG.info(_LI("VM not found on hypervisor: %s"), source_hypervisor.uuid) def filter_destination_hosts(self, hosts, vms_to_migrate): - '''return vm and host with sufficient available resources''' + """Return vm and host with sufficient available resources""" cap_cores = self.model.get_resource_from_id( resource.ResourceType.cpu_cores) @@ -233,8 +232,8 @@ class UniformAirflow(base.BaseStrategy): break # check if all vms have target hosts if len(destination_hosts) != len(vms_to_migrate): - LOG.warning(_LW("Not all target hosts could be found, it might " - "be because of there's no enough resource")) + LOG.warning(_LW("Not all target hosts could be found; it might " + "be because there is not enough resource")) return None return destination_hosts @@ -283,8 +282,8 @@ class UniformAirflow(base.BaseStrategy): return self.solution if not target_hypervisors: - LOG.warning(_LW("No hosts current have airflow under %s " - ", therefore there are no possible target " + LOG.warning(_LW("No hosts currently have airflow under %s, " + "therefore there are no possible target " "hosts for any migration"), self.threshold_airflow) return self.solution @@ -304,8 +303,8 @@ class UniformAirflow(base.BaseStrategy): destination_hosts = self.filter_destination_hosts(target_hypervisors, vms_src) if not destination_hosts: - LOG.warning(_LW("No proper target host could be found, it might " - "be because of there's no enough resource")) + LOG.warning(_LW("No target host could be found; it might " + "be because there is not enough resources")) return self.solution # generate solution to migrate the vm to the dest server, for info in destination_hosts: diff --git a/watcher/decision_engine/strategy/strategies/vm_workload_consolidation.py b/watcher/decision_engine/strategy/strategies/vm_workload_consolidation.py index b1d9c1241..39554a5d9 100644 --- a/watcher/decision_engine/strategy/strategies/vm_workload_consolidation.py +++ b/watcher/decision_engine/strategy/strategies/vm_workload_consolidation.py @@ -372,7 +372,7 @@ class VMWorkloadConsolidation(base.ServerConsolidationBaseStrategy): return False def vm_fits(self, vm_uuid, hypervisor, model, cc): - """Indicate whether is a hypervisor able to accomodate a VM. + """Indicate whether is a hypervisor able to accommodate a VM. This considers provided resource capacity coefficients (cc). :param vm_uuid: string diff --git a/watcher/decision_engine/strategy/strategies/workload_balance.py b/watcher/decision_engine/strategy/strategies/workload_balance.py index 507047e5b..43ac76583 100644 --- a/watcher/decision_engine/strategy/strategies/workload_balance.py +++ b/watcher/decision_engine/strategy/strategies/workload_balance.py @@ -54,7 +54,6 @@ class WorkloadBalance(base.WorkloadStabilizationBaseStrategy): reason why we only plan a single virtual machine migration at a time. So it's better to use this algorithm with `CONTINUOUS` audits. - It assume that live migrations are possible - """ # The meter to report CPU utilization % of VM in ceilometer @@ -100,11 +99,11 @@ class WorkloadBalance(base.WorkloadStabilizationBaseStrategy): @classmethod def get_display_name(cls): - return _("workload balance migration strategy") + return _("Workload Balance Migration Strategy") @classmethod def get_translatable_display_name(cls): - return "workload balance migration strategy" + return "Workload Balance Migration Strategy" def calculate_used_resource(self, hypervisor, cap_cores, cap_mem, cap_disk): @@ -141,7 +140,7 @@ class WorkloadBalance(base.WorkloadStabilizationBaseStrategy): # select the first active VM to migrate vm = self.model.get_vm_from_id(vm_id) if vm.state != vm_state.VMState.ACTIVE.value: - LOG.debug("VM not active, skipped: %s", + LOG.debug("VM not active; skipped: %s", vm.uuid) continue current_delta = delta_workload - workload_cache[vm_id] @@ -149,12 +148,12 @@ class WorkloadBalance(base.WorkloadStabilizationBaseStrategy): min_delta = current_delta instance_id = vm_id except wexc.InstanceNotFound: - LOG.error(_LE("VM not found Error: %s"), vm_id) + LOG.error(_LE("VM not found; error: %s"), vm_id) if instance_id: return source_hypervisor, self.model.get_vm_from_id( instance_id) else: - LOG.info(_LI("VM not found from hypervisor: %s"), + LOG.info(_LI("VM not found on hypervisor: %s"), source_hypervisor.uuid) def filter_destination_hosts(self, hosts, vm_to_migrate, @@ -230,15 +229,15 @@ class WorkloadBalance(base.WorkloadStabilizationBaseStrategy): aggregate='avg') except Exception as exc: LOG.exception(exc) - LOG.error(_LE("Can not get cpu_util")) + LOG.error(_LE("Can not get cpu_util from Ceilometer")) continue if cpu_util is None: - LOG.debug("%s: cpu_util is None", vm_id) + LOG.debug("VM (%s): cpu_util is None", vm_id) continue vm_cores = cap_cores.get_capacity(vm) workload_cache[vm_id] = cpu_util * vm_cores / 100 hypervisor_workload += workload_cache[vm_id] - LOG.debug("%s: cpu_util %f", vm_id, cpu_util) + LOG.debug("VM (%s): cpu_util %f", vm_id, cpu_util) hypervisor_cores = cap_cores.get_capacity(hypervisor) hy_cpu_util = hypervisor_workload / hypervisor_cores * 100 @@ -281,7 +280,7 @@ class WorkloadBalance(base.WorkloadStabilizationBaseStrategy): if not target_hypervisors: LOG.warning(_LW("No hosts current have CPU utilization under %s " "percent, therefore there are no possible target " - "hosts for any migration"), + "hosts for any migrations"), self.threshold) return self.solution @@ -301,15 +300,15 @@ class WorkloadBalance(base.WorkloadStabilizationBaseStrategy): # sort the filtered result by workload # pick up the lowest one as dest server if not destination_hosts: - # for instance. - LOG.warning(_LW("No proper target host could be found, it might " - "be because of there's no enough CPU/Memory/DISK")) + LOG.warning(_LW("No target host could be found; it might " + "be because there is not enough CPU, memory " + "or disk")) return self.solution destination_hosts = sorted(destination_hosts, key=lambda x: (x["cpu_util"])) # always use the host with lowerest CPU utilization mig_dst_hypervisor = destination_hosts[0]['hv'] - # generate solution to migrate the vm to the dest server, + # generate solution to migrate the vm to the dest server if self.model.get_mapping().migrate_vm(vm_src, source_hypervisor, mig_dst_hypervisor): parameters = {'migration_type': 'live',