Allow multiple for nodes reboot/del

- Ask confirmation for reboot, prune, zap

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## [Unreleased]
 
+### Changed
+- `pool nodes del` and `pool nodes reboot` now accept multiple `--nodeid`
+arguments to specify deleting and rebooting multiple nodes at the same time,
+respectively
+- `pool nodes prune`, `pool nodes reboot`, `pool nodes zap` will now ask
+for confirmation first. `-y` flag can be specified to suppress confirmation.
+
 ## [3.3.0] - 2018-03-01
 ### Added
 - Support for specifying default task exit conditions (i.e., non-zero exit

convoy/batch.py

@@ -512,7 +512,7 @@ def _block_for_nodes_ready(
             if pool_settings.attempt_recovery_on_unusable:
                 logger.warning(
                     'Unusable nodes detected, deleting unusable nodes')
-                del_node(
+                del_nodes(
                     batch_client, config, False, False, True, None,
                     suppress_confirm=True)
                 unusable_delete = True
@@ -1428,14 +1428,14 @@ def pool_autoscale_lastexec(batch_client, config):
     _output_autoscale_result(pool.auto_scale_run)
 
 
-def reboot_nodes(batch_client, config, all_start_task_failed, node_id):
+def reboot_nodes(batch_client, config, all_start_task_failed, node_ids):
-    # type: (batch.BatchServiceClient, dict, bool, str) -> None
+    # type: (batch.BatchServiceClient, dict, bool, list) -> None
     """Reboot nodes in a pool
     :param batch_client: The batch client to use.
     :type batch_client: `azure.batch.batch_service_client.BatchServiceClient`
     :param dict config: configuration dict
     :param bool all_start_task_failed: reboot all start task failed nodes
-    :param str node_id: node id to delete
+    :param list node_ids: list of node ids to reboot
     """
     pool_id = settings.pool_id(config)
     if all_start_task_failed:
@@ -1453,25 +1453,33 @@ def reboot_nodes(batch_client, config, all_start_task_failed, node_id):
                 continue
             _reboot_node(batch_client, pool_id, node.id, False)
     else:
-        _reboot_node(batch_client, pool_id, node_id, False)
+        if util.is_none_or_empty(node_ids):
+            raise ValueError('node ids to reboot is empty or invalid')
+        for node_id in node_ids:
+            if not util.confirm_action(
+                    config, 'reboot node {} from {} pool'.format(
+                        node_id, pool_id)):
+                continue
+            _reboot_node(batch_client, pool_id, node_id, False)
 
 
-def del_node(
+def del_nodes(
         batch_client, config, all_start_task_failed, all_starting,
-        all_unusable, node_id, suppress_confirm=False):
+        all_unusable, node_ids, suppress_confirm=False):
-    # type: (batch.BatchServiceClient, dict, bool, bool, bool, str,
+    # type: (batch.BatchServiceClient, dict, bool, bool, bool, list,
     #        bool) -> None
-    """Delete a node in a pool
+    """Delete nodes from a pool
     :param batch_client: The batch client to use.
     :type batch_client: `azure.batch.batch_service_client.BatchServiceClient`
     :param dict config: configuration dict
     :param bool all_start_task_failed: delete all start task failed nodes
     :param bool all_starting: delete all starting nodes
     :param bool all_unusable: delete all unusable nodes
-    :param str node_id: node id to delete
+    :param list node_ids: list of node ids to delete
     :param bool suppress_confirm: suppress confirm ask
     """
-    node_ids = []
+    if util.is_none_or_empty(node_ids):
+        node_ids = []
     pool_id = settings.pool_id(config)
     if all_start_task_failed or all_starting or all_unusable:
         filters = []
@@ -1494,12 +1502,12 @@ def del_node(
                         node.id, pool_id)):
                 node_ids.append(node.id)
     else:
-        if util.is_none_or_empty(node_id):
+        if util.is_none_or_empty(node_ids):
-            raise ValueError('node id is invalid')
+            raise ValueError('node ids to delete is empty or invalid')
-        if suppress_confirm or util.confirm_action(
+        if not suppress_confirm and not util.confirm_action(
-                config, 'delete node {} from {} pool'.format(
+                config, 'delete {} nodes from {} pool'.format(
-                    node_id, pool_id)):
+                    len(node_ids), pool_id)):
-            node_ids.append(node_id)
+            return
     if util.is_none_or_empty(node_ids):
         logger.warning('no nodes to delete from pool: {}'.format(pool_id))
         return

convoy/fleet.py

@@ -767,6 +767,20 @@ def _explode_arm_subnet_id(arm_subnet_id):
     return subid, rg, provider, vnet, subnet
 
 
+def _check_for_batch_aad(bc, rmsg):
+    # type: (settings.BatchCredentialSettings, str) -> None
+    """Check for Batch AAD
+    :param settings.BatchCredentialsSettings bc: batch cred settings
+    :param str rmsg: error message
+    """
+    if util.is_not_empty(bc.account_key):
+        raise RuntimeError(
+            'Cannot {} without Batch AAD credentials. Please ensure that '
+            'an "account_key" is not specified under the "batch" section in '
+            'credentials and an "aad" section is specified either directly '
+            'under "credentials" or under "batch".')
+
+
 def _pool_virtual_network_subnet_address_space_check(
         resource_client, network_client, config, pool_settings, bc):
     # type: (azure.mgmt.resource.resources.ResourceManagementClient,
@@ -789,10 +803,7 @@ def _pool_virtual_network_subnet_address_space_check(
         logger.debug('no virtual network settings specified')
         return None
     # check if AAD is enabled
-    if util.is_not_empty(bc.account_key):
+    _check_for_batch_aad(bc, 'allocate a pool with a virtual network')
-        raise RuntimeError(
-            'cannot allocate a pool with a virtual network without AAD '
-            'credentials')
     # get subnet object
     subnet_id = None
     if util.is_not_empty(pool_settings.virtual_network.arm_subnet_id):
@@ -1051,10 +1062,7 @@ def _construct_pool_object(
     if native:
         if util.is_not_empty(custom_image_na):
             # check if AAD is enabled
-            if util.is_not_empty(bc.account_key):
+            _check_for_batch_aad(bc, 'allocate a pool with a custom image')
-                raise RuntimeError(
-                    'cannot allocate a pool with a custom image without AAD '
-                    'credentials')
             vmconfig = batchmodels.VirtualMachineConfiguration(
                 image_reference=batchmodels.ImageReference(
                     virtual_machine_image_id=pool_settings.
@@ -1118,10 +1126,7 @@ def _construct_pool_object(
             )
     elif util.is_not_empty(custom_image_na):
         # check if AAD is enabled
-        if util.is_not_empty(bc.account_key):
+        _check_for_batch_aad(bc, 'allocate a pool with a custom image')
-            raise RuntimeError(
-                'cannot allocate a pool with a custom image without AAD '
-                'credentials')
         _rflist.append(_NODEPREP_CUSTOMIMAGE_FILE)
         vmconfig = batchmodels.VirtualMachineConfiguration(
             image_reference=batchmodels.ImageReference(
@@ -1724,6 +1729,10 @@ def _docker_system_prune_over_ssh(batch_client, config, volumes):
     :param bool volumes: remove volumes as well
     """
     pool_id = settings.pool_id(config)
+    if not util.confirm_action(
+            config,
+            msg='prune all unused Docker data on pool {}'.format(pool_id)):
+        return
     pool = batch_client.pool.get(pool_id)
     desc = 'prune unused data'
     cmd = [
@@ -1744,6 +1753,10 @@ def _zap_all_container_processes_over_ssh(batch_client, config, remove, stop):
     :param bool stop: docker stop instead of kill
     """
     pool_id = settings.pool_id(config)
+    if not util.confirm_action(
+            config,
+            msg='zap all Docker containers on pool {}'.format(pool_id)):
+        return
     pool = batch_client.pool.get(pool_id)
     desc = 'zap all container processes'
     cmd = [
@@ -3078,7 +3091,7 @@ def action_pool_rdp(batch_client, config, cardinal, nodeid, no_auto=False):
 def action_pool_nodes_del(
         batch_client, config, all_start_task_failed, all_starting,
         all_unusable, nodeid):
-    # type: (batchsc.BatchServiceClient, dict, bool, bool, bool, str) -> None
+    # type: (batchsc.BatchServiceClient, dict, bool, bool, bool, list) -> None
     """Action: Pool Nodes Del
     :param azure.batch.batch_service_client.BatchServiceClient batch_client:
         batch client
@@ -3086,7 +3099,7 @@ def action_pool_nodes_del(
     :param bool all_start_task_failed: delete all start task failed nodes
     :param bool all_starting: delete all starting nodes
     :param bool all_unusable: delete all unusable nodes
-    :param str nodeid: nodeid to delete
+    :param list nodeid: list of nodeids to delete
     """
     _check_batch_client(batch_client)
     if ((all_start_task_failed or all_starting or all_unusable) and
@@ -3094,20 +3107,20 @@ def action_pool_nodes_del(
         raise ValueError(
             'cannot specify all start task failed nodes or unusable with '
             'a specific node id')
-    batch.del_node(
+    batch.del_nodes(
         batch_client, config, all_start_task_failed, all_starting,
         all_unusable, nodeid)
 
 
 def action_pool_nodes_reboot(
         batch_client, config, all_start_task_failed, nodeid):
-    # type: (batchsc.BatchServiceClient, dict, bool, str) -> None
+    # type: (batchsc.BatchServiceClient, dict, bool, list) -> None
     """Action: Pool Nodes Reboot
     :param azure.batch.batch_service_client.BatchServiceClient batch_client:
         batch client
     :param dict config: configuration dict
     :param bool all_start_task_failed: reboot all start task failed nodes
-    :param str nodeid: nodeid to reboot
+    :param list nodeid: list of nodeids to reboot
     """
     _check_batch_client(batch_client)
     if all_start_task_failed and nodeid is not None:

shipyard.py

@@ -1539,7 +1539,8 @@ def nodes_ps(ctx):
     is_flag=True,
     help='Delete all nodes in unusable state')
 @click.option(
-    '--nodeid', help='NodeId of compute node in pool to delete')
+    '--nodeid', multiple=True,
+    help='NodeId of compute node in pool to delete')
 @common_options
 @batch_options
 @keyvault_options
@@ -1560,7 +1561,8 @@ def nodes_del(
     is_flag=True,
     help='Reboot all nodes in start task failed state')
 @click.option(
-    '--nodeid', help='NodeId of compute node in pool to reboot')
+    '--nodeid', multiple=True,
+    help='NodeId of compute node in pool to reboot')
 @common_options
 @batch_options
 @keyvault_options