Address review comments

* Unfunc `instance_name` option
* Simplify `image_ready` command - now exits 0 on no matching image or
  matching, validated image (i.e. "ready" tag is "yes"), and exits 1 on
  matching, non-validated image

oct/cli/image_ready.py

@@ -27,48 +27,49 @@
 \b
 Examples:
   See if the latest RHEL "base" image (A.K.A. AMI) in AWS has passed acceptance tests
-  $ oct image_ready --provider aws --operating_system rhel --stage base
+  $ oct image_ready --provider aws --os rhel --stage base
 
   Check if the latest centos "build" image is *not* ready in AWS, suppress non-error output (e.g. for use in a script):
-  $ oct image_ready --provider aws --operating_system centos --stage build --needs-validation --quiet
+  $ oct image_ready --provider aws --os centos --stage build --quiet
 ''',
 )
 @operating_system_option
-@provider_option
 @stage_option
+@provider_option
 @ami_id_option
 @pass_context
-@option('--needs-validation', '-n', is_flag=True, help='Returns true (0) if the newest matching image is not tagged ready.')
 @option('--quiet', '--silent', '-q', 'quiet', is_flag=True, help='Suppress standard output')
-def image_ready(context, operating_system, provider, stage, ami_id, needs_validation, quiet):
+def image_ready(context, operating_system, stage, provider, ami_id, quiet):
+    """
+    Exit non-zero (raise exception) if most recent matching image is
+    not marked "ready"
+
+    Otherwise, exit 0 if no matching images are found, or one or more
+    matching images are found and the *newest* one is tagged *ready*
+
+    :param operating_system: operating system tag to match on image
+    :param stage:  image build stage to match on image
+    :param provider: cloud provider (e.g. aws)
+    :param ami_id: unique ID specifying AWS AMI; can't be used with
+                   operating_system and stage options
+    """
     if ami_id:
         if provider != Provider.aws:
             raise UsageError("AMI ID only makes sense with cloud provider AWS")
         if operating_system or stage:
             raise UsageError("AMI ID can't be used with search criteria like --operating-system or --stage")
     if provider == Provider.aws:
-        _aws_image_ready(operating_system, stage, ami_id, needs_validation, quiet)
+        _aws_image_ready(operating_system, stage, ami_id, quiet)
 
 
-def _aws_image_ready(operating_system, stage, ami_id, needs_validation, quiet):
+def _aws_image_ready(operating_system, stage, ami_id, quiet):
     """
-    Fail (exit non-zero) if no matching image is found
-
-    If we are asking if the newest matching image is *non-ready*
-    (i.e. `--needs_validation`), succeed (exit 0) if one or more
-    matching images are found and the *newest* one is not tagged
-    *ready*
-
-    Otherwise, return succeed (exit 0) if one or more matching
-    images are found and the *newest* one is tagged *ready*
+    Implement image_ready logic for AWS AMIs
 
     :param operating_system: operating system tag to match on AMI
     :param stage:  image build stage to match on AMI
     :param ami_id: unique ID specifying AWS AMI; can't be used with
                    operating_system and stage options
-    :param needs_validation: when True, check if the newest matching
-                   AMI is not yet tagged ready. When False, check
-                   instead if it has been tagged ready
     """
     filter = []
     if operating_system:
@@ -84,20 +85,11 @@ def _aws_image_ready(operating_system, stage, ami_id, needs_validation, quiet):
     # Sort the matching images from newest to oldest:
     images = sorted(res['Images'], key=lambda (x): datetime.strptime(x['CreationDate'], '%Y-%m-%dT%H:%M:%S.%fZ'), reverse=True)
     if not res['Images']:
-        raise ClickException(message="No image was found matching the provided tags")
-    newest = images[0]
-    ready = value_for_tag(newest['Tags'], 'ready').lower() == 'yes'
-    message = "{} created {} is".format(image_info(newest), newest['CreationDate'])
-    # handle behavior for needs_validation
-    if ready:
-        message += " ready"
-        if needs_validation:
-            raise ClickException(message=message)
-        else:
-            quiet_echo(quiet, message)
+        quiet_echo(quiet, "No image was found matching the provided tags")
     else:
-        message += " not ready"
-        if needs_validation:
-            quiet_echo(quiet, message)
+        newest = images[0]
+        # If image is validated, "ready" tag will be "yes"
+        if value_for_tag(newest['Tags'], 'ready').lower() == 'yes':
+            quiet_echo(quiet, "{} created {} is validated".format(image_info(newest), newest['CreationDate']))
         else:
-            raise ClickException(message=message)
+            raise ClickException("{} created {} is not yet validated".format(image_info(newest), newest['CreationDate']))

oct/cli/package/common_options.py

@@ -17,7 +17,10 @@ def package_options(func):
         '--stage',
         '-s',
         'upgrade_stage',
-        type=Choice(['current', 'next', 'fork', 'base', 'crio', 'ose-master', 'ose-enterprise-3.9', 'ose-enterprise-3.8', 'ose-enterprise-3.7', 'ose-enterprise-3.6']),
+        type=Choice([
+            'current', 'next', 'fork', 'base', 'crio', 'ose-master', 'ose-enterprise-3.9', 'ose-enterprise-3.8',
+            'ose-enterprise-3.7', 'ose-enterprise-3.6'
+        ]),
         help='Update the current stage, upgrade to next default stage, or choose a stage',
     )(func)
 

oct/cli/provision/remote/all_in_one.py

@@ -7,7 +7,6 @@
 from ...util.common_options import ansible_output_options
 from ...util.cloud_provider.image_options import Stage, operating_system_option, stage_option, ami_id_option
 from ...util.cloud_provider.common_options import Provider, provider_option
-from ...util.cloud_provider.instance_options import instance_name_option
 
 
 def destroy_callback(context, _, value):
@@ -52,7 +51,13 @@ def destroy_callback(context, _, value):
 @operating_system_option
 @provider_option
 @stage_option
-@instance_name_option
+@option(
+    '--name',
+    '-n',
+    metavar='NAME',
+    required=True,
+    help='VM instance name.',
+)
 @ami_id_option
 @option(
     '--destroy',

oct/cli/util/cloud_provider/image_options.py

@@ -48,7 +48,6 @@ def operating_system_option(func):
             OperatingSystem.centos,
             OperatingSystem.rhel,
         ]),
-        # default=OperatingSystem.fedora,
         show_default=True,
         metavar='NAME',
         help='VM operating system.',