Fix aws on windows

perfkitbenchmarker/aws/aws_disk.py

@@ -88,7 +88,7 @@ def _Exists(self):
         'describe-volumes',
         '--region=%s' % self.region,
         '--filter=Name=volume-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = util.IssueRetryableCommand(describe_cmd)
     response = json.loads(stdout)
     volumes = response['Volumes']
     assert len(volumes) < 2, 'Too many volumes.'
@@ -121,7 +121,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)
+    util.IssueRetryableCommand(attach_cmd)
 
   def Detach(self):
     """Detaches the disk from a VM."""
@@ -131,7 +131,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)
+    util.IssueRetryableCommand(detach_cmd)
 
     with self._lock:
       assert self.attached_vm_id in AwsDisk.vm_devices

perfkitbenchmarker/aws/aws_network.py

@@ -75,9 +75,9 @@ def AllowPort(self, vm, port):
           '--group-id=%s' % vm.group_id,
           '--port=%s' % port,
           '--cidr=0.0.0.0/0']
-      vm_util.IssueRetryableCommand(
+      util.IssueRetryableCommand(
           authorize_cmd + ['--protocol=tcp'])
-      vm_util.IssueRetryableCommand(
+      util.IssueRetryableCommand(
           authorize_cmd + ['--protocol=udp'])
       self.firewall_set.add(entry)
 
@@ -114,7 +114,7 @@ def _Exists(self):
         'describe-vpcs',
         '--region=%s' % self.region,
         '--filter=Name=vpc-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = util.IssueRetryableCommand(describe_cmd)
     response = json.loads(stdout)
     vpcs = response['Vpcs']
     assert len(vpcs) < 2, 'Too many VPCs.'
@@ -136,7 +136,7 @@ def _EnableDnsHostnames(self):
         '--enable-dns-hostnames',
         '{ "Value": true }']
 
-    vm_util.IssueRetryableCommand(enable_hostnames_command)
+    util.IssueRetryableCommand(enable_hostnames_command)
 
   def _Delete(self):
     """Delete's the VPC."""
@@ -191,7 +191,7 @@ def _Exists(self):
         'describe-subnets',
         '--region=%s' % self.region,
         '--filter=Name=subnet-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = util.IssueRetryableCommand(describe_cmd)
     response = json.loads(stdout)
     subnets = response['Subnets']
     assert len(subnets) < 2, 'Too many subnets.'
@@ -235,7 +235,7 @@ 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, _ = util.IssueRetryableCommand(describe_cmd)
     response = json.loads(stdout)
     internet_gateways = response['InternetGateways']
     assert len(internet_gateways) < 2, 'Too many internet gateways.'
@@ -251,7 +251,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)
+      util.IssueRetryableCommand(attach_cmd)
       self.attached = True
 
   def Detach(self):
@@ -263,7 +263,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)
+      util.IssueRetryableCommand(detach_cmd)
       self.attached = False
 
 
@@ -297,7 +297,7 @@ 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, _ = util.IssueRetryableCommand(describe_cmd)
     response = json.loads(stdout)
     self.id = response['RouteTables'][0]['RouteTableId']
 
@@ -310,7 +310,7 @@ 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)
+    util.IssueRetryableCommand(create_cmd)
 
 
 class AwsPlacementGroup(resource.BaseResource):
@@ -358,7 +358,7 @@ 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, _ = util.IssueRetryableCommand(describe_cmd)
     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)
+      util.IssueRetryableCommand(import_cmd)
       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)
+      util.IssueRetryableCommand(delete_cmd)
       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,7 @@ 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, _ = util.IssueRetryableCommand(describe_cmd)
     response = json.loads(stdout)
     instance = response['Reservations'][0]['Instances'][0]
     self.ip_address = instance['PublicIpAddress']
@@ -248,7 +248,7 @@ def _Exists(self):
         'describe-instances',
         '--region=%s' % self.region,
         '--filter=Name=instance-id,Values=%s' % self.id]
-    stdout, _ = vm_util.IssueRetryableCommand(describe_cmd)
+    stdout, _ = util.IssueRetryableCommand(describe_cmd)
     response = json.loads(stdout)
     reservations = response['Reservations']
     assert len(reservations) < 2, 'Too many reservations.'

perfkitbenchmarker/aws/util.py

@@ -14,6 +14,7 @@
 
 """Utilities for working with Amazon Web Services resources."""
 
+from perfkitbenchmarker import errors
 from perfkitbenchmarker import flags
 from perfkitbenchmarker import vm_util
 
@@ -41,7 +42,7 @@ def AddTags(resource_id, region, **kwargs):
       '--tags']
   for key, value in kwargs.iteritems():
     tag_cmd.append('Key={0},Value={1}'.format(key, value))
-  vm_util.IssueRetryableCommand(tag_cmd)
+  IssueRetryableCommand(tag_cmd)
 
 
 def AddDefaultTags(resource_id, region):
@@ -57,3 +58,29 @@ def AddDefaultTags(resource_id, region):
   """
   tags = {'owner': FLAGS.owner, 'perfkitbenchmarker-run': FLAGS.run_uri}
   AddTags(resource_id, region, **tags)
+
+
+@vm_util.Retry()
+def IssueRetryableCommand(cmd, env=None):
+  """Tries running the provided command until it succeeds or times out.
+
+  On Windows, the AWS CLI doesn't correctly set the return code when it
+  has an error (at least on version 1.7.28). By retrying the command if
+  we get output on stderr, we can work around this issue.
+
+  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.
+
+  Returns:
+    A tuple of stdout and stderr from running the provided command.
+  """
+  stdout, stderr, retcode = vm_util.IssueCommand(cmd, env=env)
+  if retcode:
+    raise errors.VmUtil.CalledProcessException(
+        'Command returned a non-zero exit code.\n')
+  if stderr:
+    raise errors.VmUtil.CalledProcessException(
+        'The command had output on stderr:\n%s' % stderr)
+  return stdout, stderr

perfkitbenchmarker/vm_util.py

@@ -439,6 +439,7 @@ def IssueRetryableCommand(cmd, env=None):
   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.
 
   Returns:
     A tuple of stdout and stderr from running the provided command.

tests/aws_test.py

@@ -22,16 +22,16 @@
 
 from perfkitbenchmarker import disk
 from perfkitbenchmarker import virtual_machine
-from perfkitbenchmarker import vm_util
 from perfkitbenchmarker.aws import aws_disk
 from perfkitbenchmarker.aws import aws_network
 from perfkitbenchmarker.aws import aws_virtual_machine
+from perfkitbenchmarker.aws import util
 
 
 class AwsVolumeExistsTestCase(unittest.TestCase):
 
   def setUp(self):
-    self.p = mock.patch('perfkitbenchmarker.vm_util.IssueRetryableCommand')
+    self.p = mock.patch('perfkitbenchmarker.aws.util.IssueRetryableCommand')
     self.p.start()
     self.disk = aws_disk.AwsDisk(disk.BaseDiskSpec(None, None, None), 'zone-a')
     self.disk.id = 'vol-foo'
@@ -44,22 +44,22 @@ def testVolumePresent(self):
                 '"CreateTime": "2015-05-04T23:47:31.726Z","VolumeId":'
                 '"vol-5859691f","AvailabilityZone":"us-east-1a","VolumeType":'
                 '"standard","State":"creating"}]}')
-    vm_util.IssueRetryableCommand.side_effect = [(response, None)]
+    util.IssueRetryableCommand.side_effect = [(response, None)]
     self.assertTrue(self.disk._Exists())
 
   def testVolumeDeleted(self):
     response = ('{"Volumes":[{"VolumeId":"vol-e45b6ba3","Size": 500,'
                 '"AvailabilityZone": "us-east-1a","CreateTime":'
                 '"2015-05-04T23:53:42.952Z","Attachments":[],"State":'
                 '"deleting","SnapshotId":null,"VolumeType": "standard"}]}')
-    vm_util.IssueRetryableCommand.side_effect = [(response, None)]
+    util.IssueRetryableCommand.side_effect = [(response, None)]
     self.assertFalse(self.disk._Exists())
 
 
 class AwsVpcExistsTestCase(unittest.TestCase):
 
   def setUp(self):
-    self.p = mock.patch('perfkitbenchmarker.vm_util.IssueRetryableCommand')
+    self.p = mock.patch('perfkitbenchmarker.aws.util.IssueRetryableCommand')
     self.p.start()
     self.vpc = aws_network.AwsVpc('region')
     self.vpc.id = 'vpc-foo'
@@ -69,23 +69,23 @@ def tearDown(self):
 
   def testVpcDeleted(self):
     response = '{"Vpcs": [] }'
-    vm_util.IssueRetryableCommand.side_effect = [(response, None)]
+    util.IssueRetryableCommand.side_effect = [(response, None)]
     self.assertFalse(self.vpc._Exists())
 
   def testVpcPresent(self):
     response = ('{"Vpcs":[{"InstanceTenancy":"default","CidrBlock":'
                 '"10.0.0.0/16","IsDefault":false,"DhcpOptionsId":'
                 '"dopt-59b12f38","State":"available","VpcId":"vpc-2289a647"'
                 '}]}')
-    vm_util.IssueRetryableCommand.side_effect = [(response, None)]
+    util.IssueRetryableCommand.side_effect = [(response, None)]
     self.assertTrue(self.vpc._Exists())
 
 
 
 class AwsVirtualMachineExistsTestCase(unittest.TestCase):
 
   def setUp(self):
-    self.p = mock.patch('perfkitbenchmarker.vm_util.IssueRetryableCommand')
+    self.p = mock.patch('perfkitbenchmarker.aws.util.IssueRetryableCommand')
     self.p.start()
     self.vm = aws_virtual_machine.AwsVirtualMachine(
         virtual_machine.BaseVirtualMachineSpec(
@@ -100,14 +100,14 @@ def tearDown(self):
     self.p.stop()
 
   def testInstancePresent(self):
-    vm_util.IssueRetryableCommand.side_effect = [(self.response, None)]
+    util.IssueRetryableCommand.side_effect = [(self.response, None)]
     self.assertTrue(self.vm._Exists())
 
   def testInstanceDeleted(self):
     response = json.loads(self.response)
     state = response['Reservations'][0]['Instances'][0]['State']
     state['Name'] = 'shutting-down'
-    vm_util.IssueRetryableCommand.side_effect = [(json.dumps(response), None)]
+    util.IssueRetryableCommand.side_effect = [(json.dumps(response), None)]
     self.assertFalse(self.vm._Exists())
 
 if __name__ == '__main__':