Merge pull request #136 from Yelp/paasta-validate-itests

Paasta validate itests

general_itests/fake_soa_configs_validate/fake_invalid_service/chronos-test-cluster.yaml

@@ -0,0 +1,5 @@
+# Invalid with validation error: You must specify a "schedule" in your configuration
+---
+chronos_job:
+  cpus: .1
+  mem: 100

general_itests/fake_soa_configs_validate/fake_invalid_service/marathon-test-cluster.yaml

@@ -0,0 +1,7 @@
+---
+main:
+    cpus: .1
+    mem: 100
+    instances: 1
+    env:
+      FOO: BAR

general_itests/fake_soa_configs_validate/fake_valid_service/chronos-test-cluster.yaml

@@ -0,0 +1,5 @@
+---
+chronos_job:
+  cpus: .1
+  mem: 100
+  schedule: 'R/2015-08-14T10:00:00+00:00/PT10M'

general_itests/fake_soa_configs_validate/fake_valid_service/marathon-test-cluster.yaml

@@ -0,0 +1,7 @@
+---
+main:
+    cpus: .1
+    mem: 100
+    instances: 1
+    env:
+      FOO: BAR

general_itests/steps/validate_steps.py

@@ -0,0 +1,57 @@
+# Copyright 2015 Yelp Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from behave import given, when, then
+
+from paasta_tools.cli.utils import x_mark
+from paasta_tools.utils import _run
+
+
+@given(u'a "{service_type}" service')
+@given(u'an "{service_type}" service')
+def given_service(context, service_type):
+    context.service = 'fake_%s_service' % service_type
+    context.soa_dir = 'fake_soa_configs_validate'
+
+
+@when(u'we run paasta validate')
+def run_paasta_validate(context):
+    validate_cmd = ("cli.py validate "
+                    "--yelpsoa-config-root %s "
+                    "--service %s " % (context.soa_dir, context.service))
+    context.validate_return_code, context.validate_output = _run(command=validate_cmd)
+    context.validate_output = context.validate_output.decode('utf-8')
+
+
+@then(u'it should have a return code of "{code:d}"')
+def see_expected_return_code(context, code):
+    print context.validate_output
+    print context.validate_return_code
+    print
+    assert context.validate_return_code == code
+
+
+@then(u'everything should pass')
+def validate_status_all_pass(context):
+    assert not context.validate_output or x_mark().decode('utf-8') not in context.validate_output
+
+
+@then(u'it should report an error in the output')
+def validate_status_something_fail(context):
+    assert x_mark().decode('utf-8') in context.validate_output
+
+
+@then(u'the output should contain \'{output_string}\'')
+def output_contains(context, output_string):
+    assert output_string in context.validate_output

general_itests/validate.feature

@@ -0,0 +1,15 @@
+Feature: paasta validate can be used
+
+  Scenario: Running paasta validate against a valid service
+    Given a "valid" service
+    When we run paasta validate
+    Then it should have a return code of "0"
+     And everything should pass
+     And the output should contain 'has a valid instance'
+
+  Scenario: Running paasta validate against an invalid service
+    Given an "invalid" service
+    When we run paasta validate
+    Then it should have a return code of "1"
+     And it should report an error in the output
+     And the output should contain 'You must specify a "schedule" in your configuration'

paasta_tools/cli/cmds/validate.py

@@ -16,6 +16,7 @@
 import json
 import os
 import pkgutil
+import sys
 import yaml
 
 from glob import glob
@@ -95,7 +96,7 @@ def validate_schema(file_path, file_type):
         config_file = get_file_contents(file_path)
     except IOError:
         print '%s: %s' % (FAILED_READING_FILE, file_path)
-        return
+        return 1
     if extension == '.yaml':
         config_file_object = yaml.load(config_file)
     elif extension == '.json':
@@ -107,8 +108,10 @@ def validate_schema(file_path, file_type):
     except ValidationError as e:
         print '%s: %s' % (SCHEMA_INVALID, file_path)
         print '  Validation Message: %s' % e.message
+        return 1
     else:
         print '%s: %s' % (SCHEMA_VALID, basename)
+        return 0
 
 
 def validate_all_schemas(service_path):
@@ -124,9 +127,13 @@ def validate_all_schemas(service_path):
         if os.path.islink(file_name):
             continue
         basename = os.path.basename(file_name)
+        returncode = 0
         for file_type in ['chronos', 'marathon']:
             if basename.startswith(file_type):
-                validate_schema(file_name, file_type)
+                tmp_returncode = validate_schema(file_name, file_type)
+                if tmp_returncode != 0:
+                    returncode = tmp_returncode
+        return returncode
 
 
 def add_subparser(subparsers):
@@ -155,13 +162,22 @@ def get_service_path(service, soa_dir):
     :param args: argparse.Namespace obj created from sys.args by cli
     """
     if service:
-        return os.path.join(soa_dir, service)
+        service_path = os.path.join(soa_dir, service)
     else:
         if soa_dir == os.getcwd():
-            return os.getcwd()
+            service_path = os.getcwd()
         else:
             print UNKNOWN_SERVICE
             return None
+    if not os.path.isdir(service_path):
+        print failure("%s is not a directory" % service_path,
+                      "http://paasta.readthedocs.org/en/latest/yelpsoa_configs.html")
+        return None
+    if not glob(os.path.join(service_path, "*.yaml")):
+        print failure("%s does not contain any .yaml files" % service_path,
+                      "http://paasta.readthedocs.org/en/latest/yelpsoa_configs.html")
+        return None
+    return service_path
 
 
 def path_to_soa_dir_service(service_path):
@@ -174,6 +190,7 @@ def validate_chronos(service_path):
     soa_dir, service = path_to_soa_dir_service(service_path)
     instance_type = 'chronos'
 
+    returncode = 0
     for cluster in list_clusters(service, soa_dir, instance_type):
         for instance in list_all_instances_for_service(
                 service=service, clusters=[cluster], instance_type=instance_type,
@@ -186,8 +203,10 @@ def validate_chronos(service_path):
 
             if not checks_passed:
                 print invalid_chronos_instance(cluster, instance, "\n  ".join(unique_check_msgs))
+                returncode = 1
             else:
                 print valid_chronos_instance(cluster, instance)
+    return returncode
 
 
 def paasta_validate(args):
@@ -201,7 +220,17 @@ def paasta_validate(args):
     service_path = get_service_path(service, soa_dir)
 
     if service_path is None:
-        return 1
+        sys.exit(1)
+
+    returncode = 0
+
+    tmp_returncode = validate_all_schemas(service_path)
+    if tmp_returncode != 0:
+        returncode = tmp_returncode
+
+    tmp_returncode = validate_chronos(service_path)
+    if tmp_returncode != 0:
+        returncode = tmp_returncode
 
-    validate_all_schemas(service_path)
-    validate_chronos(service_path)
+    if returncode != 0:
+        sys.exit(returncode)

tests/cli/test_cmds_validate.py

@@ -16,6 +16,7 @@
 import os
 
 from mock import patch
+from pytest import raises
 from StringIO import StringIO
 
 from paasta_tools.chronos_tools import ChronosJobConfig
@@ -42,6 +43,8 @@ def test_paasta_validate_calls_everything(
     # Ensure each check in 'paasta_validate' is called
 
     mock_get_service_path.return_value = 'unused_path'
+    mock_validate_all_schemas.return_value = 0
+    mock_validate_chronos.return_value = 0
 
     args = mock.MagicMock()
     args.service = None
@@ -72,10 +75,21 @@ def test_validate_unknown_service():
     args.service = None
     args.yelpsoa_config_root = 'unused'
 
-    assert paasta_validate(args) == 1
+    with raises(SystemExit) as excinfo:
+        paasta_validate(args)
 
+    assert excinfo.value.code == 1
+
+
+@patch('paasta_tools.cli.cmds.validate.os.path.isdir')
+@patch('paasta_tools.cli.cmds.validate.glob')
+def test_get_service_path_cwd(
+    mock_glob,
+    mock_isdir
+):
+    mock_isdir.return_value = True
+    mock_glob.return_value = ['something.yaml']
 
-def test_get_service_path_cwd():
     service = None
     soa_dir = os.getcwd()
 
@@ -84,7 +98,15 @@ def test_get_service_path_cwd():
     assert service_path == os.getcwd()
 
 
-def test_get_service_path_soa_dir():
+@patch('paasta_tools.cli.cmds.validate.os.path.isdir')
+@patch('paasta_tools.cli.cmds.validate.glob')
+def test_get_service_path_soa_dir(
+    mock_glob,
+    mock_isdir
+):
+    mock_isdir.return_value = True
+    mock_glob.return_value = ['something.yaml']
+
     service = 'some_service'
     soa_dir = 'some/path'
 
@@ -134,7 +156,7 @@ def test_marathon_validate_schema_list_hashes_good(
 """
     mock_get_file_contents.return_value = marathon_content
 
-    validate_schema('unused_service_path.yaml', 'marathon')
+    assert validate_schema('unused_service_path.yaml', 'marathon') == 0
 
     output = mock_stdout.getvalue()
 
@@ -155,7 +177,7 @@ def test_marathon_validate_schema_keys_outside_instance_blocks_bad(
     "page": false
 }
 """
-    validate_schema('unused_service_path.json', 'marathon')
+    assert validate_schema('unused_service_path.json', 'marathon') == 1
 
     output = mock_stdout.getvalue()
 
@@ -178,7 +200,7 @@ def test_chronos_validate_schema_list_hashes_good(
     }
 }
 """
-    validate_schema('unused_service_path.json', 'chronos')
+    assert validate_schema('unused_service_path.json', 'chronos') == 0
 
     output = mock_stdout.getvalue()
 
@@ -199,7 +221,7 @@ def test_chronos_validate_schema_keys_outside_instance_blocks_bad(
     "page": false
 }
 """
-    validate_schema('unused_service_path.json', 'chronos')
+    assert validate_schema('unused_service_path.json', 'chronos') == 1
 
     output = mock_stdout.getvalue()
 
@@ -248,7 +270,7 @@ def test_validate_chronos_missing_schedule(
     mock_list_all_instances_for_service.return_value = [fake_instance]
     mock_load_chronos_job_config.return_value = fake_chronos_job_config
 
-    validate_chronos('fake_service_path')
+    assert validate_chronos('fake_service_path') == 1
 
     output = mock_stdout.getvalue()
 
@@ -299,7 +321,7 @@ def test_validate_chronos_invalid_instance(
     mock_list_all_instances_for_service.return_value = [fake_instance]
     mock_load_chronos_job_config.return_value = fake_chronos_job_config
 
-    validate_chronos('fake_service_path')
+    assert validate_chronos('fake_service_path') == 1
 
     output = mock_stdout.getvalue()
 
@@ -351,7 +373,7 @@ def test_validate_chronos_valid_instance(
     mock_list_all_instances_for_service.return_value = [fake_instance]
     mock_load_chronos_job_config.return_value = fake_chronos_job_config
 
-    validate_chronos('fake_service_path')
+    assert validate_chronos('fake_service_path') == 0
 
     output = mock_stdout.getvalue()