Fix AWS on windows

The AWS CLI does not correctly set either $LastExitCode or $? when it
fails. This leads to us thinking a command succeeded when it didn't.
By adding the option to retry the command in IssueRetryableCommand when
there is output on stderr, we can work around this.

perfkitbenchmarker/aws/aws_disk.py

@@ -88,7 +88,8 @@ def _Exists(self):
         'describe-volumes',
         '--region=%s' % self.region,
         '--filter=Name=volume-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd,
+                                              retry_on_stderr=True)
     response = json.loads(stdout)
     volumes = response['Volumes']
     assert len(volumes) < 2, 'Too many volumes.'
@@ -121,7 +122,7 @@ def Attach(self, vm):
         '--device=%s' % self.GetDevicePath()]
     logging.info('Attaching AWS volume %s. This may fail if the disk is not '
                  'ready, but will be retried.', self.id)
-    vm_util.IssueRetryableCommand(attach_cmd)
+    vm_util.IssueRetryableCommand(attach_cmd, retry_on_stderr=True)
 
   def Detach(self):
     """Detaches the disk from a VM."""
@@ -131,7 +132,7 @@ def Detach(self):
         '--region=%s' % self.region,
         '--instance-id=%s' % self.attached_vm_id,
         '--volume-id=%s' % self.id]
-    vm_util.IssueRetryableCommand(detach_cmd)
+    vm_util.IssueRetryableCommand(detach_cmd, retry_on_stderr=True)
 
     with self._lock:
       assert self.attached_vm_id in AwsDisk.vm_devices

perfkitbenchmarker/aws/aws_network.py

@@ -76,9 +76,9 @@ def AllowPort(self, vm, port):
           '--port=%s' % port,
           '--cidr=0.0.0.0/0']
       vm_util.IssueRetryableCommand(
-          authorize_cmd + ['--protocol=tcp'])
+          authorize_cmd + ['--protocol=tcp'], retry_on_stderr=True)
       vm_util.IssueRetryableCommand(
-          authorize_cmd + ['--protocol=udp'])
+          authorize_cmd + ['--protocol=udp'], retry_on_stderr=True)
       self.firewall_set.add(entry)
 
   def DisallowAllPorts(self):
@@ -114,7 +114,8 @@ def _Exists(self):
         'describe-vpcs',
         '--region=%s' % self.region,
         '--filter=Name=vpc-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd,
+                                              retry_on_stderr=True)
     response = json.loads(stdout)
     vpcs = response['Vpcs']
     assert len(vpcs) < 2, 'Too many VPCs.'
@@ -136,7 +137,8 @@ def _EnableDnsHostnames(self):
         '--enable-dns-hostnames',
         '{ "Value": true }']
 
-    vm_util.IssueRetryableCommand(enable_hostnames_command)
+    vm_util.IssueRetryableCommand(enable_hostnames_command,
+                                  retry_on_stderr=True)
 
   def _Delete(self):
     """Delete's the VPC."""
@@ -191,7 +193,8 @@ def _Exists(self):
         'describe-subnets',
         '--region=%s' % self.region,
         '--filter=Name=subnet-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd,
+                                              retry_on_stderr=True)
     response = json.loads(stdout)
     subnets = response['Subnets']
     assert len(subnets) < 2, 'Too many subnets.'
@@ -235,7 +238,8 @@ def _Exists(self):
         'describe-internet-gateways',
         '--region=%s' % self.region,
         '--filter=Name=internet-gateway-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd,
+                                              retry_on_stderr=True)
     response = json.loads(stdout)
     internet_gateways = response['InternetGateways']
     assert len(internet_gateways) < 2, 'Too many internet gateways.'
@@ -251,7 +255,7 @@ def Attach(self, vpc_id):
           '--region=%s' % self.region,
           '--internet-gateway-id=%s' % self.id,
           '--vpc-id=%s' % self.vpc_id]
-      vm_util.IssueRetryableCommand(attach_cmd)
+      vm_util.IssueRetryableCommand(attach_cmd, retry_on_stderr=True)
       self.attached = True
 
   def Detach(self):
@@ -263,7 +267,7 @@ def Detach(self):
           '--region=%s' % self.region,
           '--internet-gateway-id=%s' % self.id,
           '--vpc-id=%s' % self.vpc_id]
-      vm_util.IssueRetryableCommand(detach_cmd)
+      vm_util.IssueRetryableCommand(detach_cmd, retry_on_stderr=True)
       self.attached = False
 
 
@@ -297,7 +301,8 @@ def _PostCreate(self):
         'describe-route-tables',
         '--region=%s' % self.region,
         '--filters=Name=vpc-id,Values=%s' % self.vpc_id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd,
+                                              retry_on_stderr=True)
     response = json.loads(stdout)
     self.id = response['RouteTables'][0]['RouteTableId']
 
@@ -310,7 +315,8 @@ def CreateRoute(self, internet_gateway_id):
         '--route-table-id=%s' % self.id,
         '--gateway-id=%s' % internet_gateway_id,
         '--destination-cidr-block=0.0.0.0/0']
-    vm_util.IssueRetryableCommand(create_cmd)
+    vm_util.IssueRetryableCommand(create_cmd,
+                                  retry_on_stderr=True)
 
 
 class AwsPlacementGroup(resource.BaseResource):
@@ -358,7 +364,8 @@ def _Exists(self):
         'describe-placement-groups',
         '--region=%s' % self.region,
         '--filter=Name=group-name,Values=%s' % self.name]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd,
+                                              retry_on_stderr=True)
     response = json.loads(stdout)
     placement_groups = response['PlacementGroups']
     assert len(placement_groups) < 2, 'Too many placement groups.'

perfkitbenchmarker/aws/aws_virtual_machine.py

@@ -162,7 +162,7 @@ def ImportKeyfile(self):
           'import-key-pair',
           '--key-name=%s' % 'perfkit-key-%s' % FLAGS.run_uri,
           '--public-key-material=%s' % keyfile]
-      vm_util.IssueRetryableCommand(import_cmd)
+      vm_util.IssueRetryableCommand(import_cmd, retry_on_stderr=True)
       self.imported_keyfile_set.add(self.region)
       if self.region in self.deleted_keyfile_set:
         self.deleted_keyfile_set.remove(self.region)
@@ -176,7 +176,7 @@ def DeleteKeyfile(self):
           'ec2', '--region=%s' % self.region,
           'delete-key-pair',
           '--key-name=%s' % 'perfkit-key-%s' % FLAGS.run_uri]
-      vm_util.IssueRetryableCommand(delete_cmd)
+      vm_util.IssueRetryableCommand(delete_cmd, retry_on_stderr=True)
       self.deleted_keyfile_set.add(self.region)
       if self.region in self.imported_keyfile_set:
         self.imported_keyfile_set.remove(self.region)
@@ -191,7 +191,8 @@ def _PostCreate(self):
         '--instance-ids=%s' % self.id]
     logging.info('Getting instance %s public IP. This will fail until '
                  'a public IP is available, but will be retried.', self.id)
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd,
+                                              retry_on_stderr=True)
     response = json.loads(stdout)
     instance = response['Reservations'][0]['Instances'][0]
     self.ip_address = instance['PublicIpAddress']
@@ -248,7 +249,8 @@ def _Exists(self):
         'describe-instances',
         '--region=%s' % self.region,
         '--filter=Name=instance-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd,
+                                              retry_on_stderr=True)
     response = json.loads(stdout)
     reservations = response['Reservations']
     assert len(reservations) < 2, 'Too many reservations.'

perfkitbenchmarker/vm_util.py

@@ -433,12 +433,15 @@ def IssueBackgroundCommand(cmd, stdout_path, stderr_path, env=None):
 
 
 @Retry()
-def IssueRetryableCommand(cmd, env=None):
+def IssueRetryableCommand(cmd, env=None, retry_on_stderr=False):
   """Tries running the provided command until it succeeds or times out.
 
   Args:
     cmd: A list of strings such as is given to the subprocess.Popen()
         constructor.
+    env: An alternate environment to pass to the Popen command.
+    retry_on_stderr: If True, getting any output on stderr will be treated
+        like a non-zero return code (i.e. the command will be retried).
 
   Returns:
     A tuple of stdout and stderr from running the provided command.
@@ -447,6 +450,10 @@ def IssueRetryableCommand(cmd, env=None):
   if retcode:
     raise errors.VmUtil.CalledProcessException(
         'Command returned a non-zero exit code.\n')
+  if retry_on_stderr and stderr:
+    raise errors.VmUtil.CalledProcessException(
+        'The "retry_on_stderr" option was set and the command '
+        'had output on stderr:\n%s' % stderr)
   return stdout, stderr