Repository: nchammas/flintrock (python)

Issue:
Adding custom tags
First off, thank you for this awesome package. I've been using it extensively for the last couple of months and it's been wonderful.

At my work, we label launched ec2 instances with additional tags for book-keeping purposes. Is there any interest to add that functionality to flintrock? Right now, the only tags flintrock uses are flintrock-role and Name and there's no way to append custom tags. I'm happy to take a stab if you think it's a worthwhile addition.


Pull request description:
add option to tag ec2 instances
This PR allows users to label their EC2 instances with custom tags. An EC2 tag is added with the --ec2-tag option, where each tag is a Key,Value pair separated by a comma. The ec2-tag can be specified multiple times.

I tested this both for launching new clusters and appending slaves to existing clusters. As far as I can tell, it works as expected.

Speaking of the code itself, there's a common formatting step that takes the tags as supplied on the command line and transforms them into a dictionary that EC2 expects. This piece of code is replicated for launching and adding slaves as I didn't know the proper place to put it if we wanted to wrap it as a function (the ec2.py file?). This would be trivial to do if desired.

Cheers,
Luke

Fixes #182 




Patch:
diff --git a/CHANGES.md b/CHANGES.md
index cc7b8d7..52f6b0c 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -2,10 +2,44 @@
 
 ## [Unreleased]
 
-[Unreleased]: https://github.com/nchammas/flintrock/compare/v0.7.0...master
+[Unreleased]: https://github.com/nchammas/flintrock/compare/v0.8.0...master
 
 Nothing notable yet.
 
+## [0.8.0] - 2017-02-11
+
+[0.8.0]: https://github.com/nchammas/flintrock/compare/v0.7.0...v0.8.0
+
+### Added
+
+* [#180]: Accessing data on S3 from your Flintrock cluster is now much
+  easier! Just configure Flintrock to use Hadoop 2.7+ (which is the
+  default) and an appropriate IAM role, and you'll be able to access
+  paths on S3 using the new `s3a://` prefix. [Check the README] for
+  more information.
+* [#176], [#187]: Flintrock now supports users with non-standard home
+  directories.
+
+[#180]: https://github.com/nchammas/flintrock/pull/180
+[#176]: https://github.com/nchammas/flintrock/pull/176
+[#187]: https://github.com/nchammas/flintrock/pull/187
+[Check the README]: https://github.com/nchammas/flintrock/tree/v0.8.0#accessing-data-on-s3
+
+### Changed
+
+* [#168]: Flintrock now does a better job of cleaning up after
+  interrupted operations.
+* [#179], [#184]: Flintrock can now clean up malformed Flintrock
+  clusters.
+* [`6b426ae`]: We fixed an issue affecting some users of Flintrock's
+  standalone package that caused Flintrock to intermittently throw
+  `ImportError`s.
+
+[#168]: https://github.com/nchammas/flintrock/pull/168
+[#179]: https://github.com/nchammas/flintrock/pull/179
+[#184]: https://github.com/nchammas/flintrock/pull/184
+[`6b426ae`]: https://github.com/nchammas/flintrock/commit/6b426aedc7e92b434021cc09c6e7eb181fca7eef
+
 ## [0.7.0] - 2016-11-15
 
 [0.7.0]: https://github.com/nchammas/flintrock/compare/v0.6.0...v0.7.0
diff --git a/README.md b/README.md
index abf3393..2d67e24 100644
--- a/README.md
+++ b/README.md
@@ -21,7 +21,7 @@ flintrock launch test-cluster \
     --spark-version 2.1.0 \
     --ec2-key-name key_name \
     --ec2-identity-file /path/to/key.pem \
-    --ec2-ami ami-9be6f38c \
+    --ec2-ami ami-0b33d91d \
     --ec2-user ec2-user
 ```
 
@@ -125,7 +125,7 @@ unzip it to a location of your choice, and run the `flintrock` executable inside
 For example:
 
 ```sh
-flintrock_version="0.7.0"
+flintrock_version="0.8.0"
 
 curl --location --remote-name "https://github.com/nchammas/flintrock/releases/download/v$flintrock_version/Flintrock-$flintrock_version-standalone-OSX-x86_64.zip"
 unzip -q -d flintrock "Flintrock-$flintrock_version-standalone-OSX-x86_64.zip"
@@ -231,7 +231,7 @@ providers:
     identity-file: /path/to/.ssh/key.pem
     instance-type: m3.medium
     region: us-east-1
-    ami: ami-9be6f38c
+    ami: ami-0b33d91d
     user: ec2-user
 ```
 
diff --git a/flintrock/__init__.py b/flintrock/__init__.py
index 11c50e6..071b765 100644
--- a/flintrock/__init__.py
+++ b/flintrock/__init__.py
@@ -1,2 +1,2 @@
 # See: https://packaging.python.org/en/latest/distributing/#standards-compliance-for-interoperability
-__version__ = '0.8.0.dev0'
+__version__ = '0.9.0.dev0'
diff --git a/flintrock/config.yaml.template b/flintrock/config.yaml.template
index 29edbe6..ccaa062 100644
--- a/flintrock/config.yaml.template
+++ b/flintrock/config.yaml.template
@@ -24,7 +24,7 @@ providers:
     instance-type: m3.medium
     region: us-east-1
     # availability-zone: <name>
-    ami: ami-9be6f38c   # Amazon Linux, us-east-1
+    ami: ami-0b33d91d   # Amazon Linux, us-east-1
     user: ec2-user
     # ami: ami-61bbf104   # CentOS 7, us-east-1
     # user: centos
@@ -36,6 +36,10 @@ providers:
     #   - group-name1
     #   - group-name2
     # instance-profile-name:
+    # tags:
+    #   - key1,value1
+    #   - key2, value2  # leading/trailing spaces are trimmed
+    #   - key3,  # value will be empty
     tenancy: default  # default | dedicated
     ebs-optimized: no  # yes | no
     instance-initiated-shutdown-behavior: terminate  # terminate | stop
diff --git a/flintrock/core.py b/flintrock/core.py
index 20dc79b..594695f 100644
--- a/flintrock/core.py
+++ b/flintrock/core.py
@@ -140,8 +140,8 @@ class FlintrockCluster:
             manifest_raw = ssh_check_output(
                 client=master_ssh_client,
                 command="""
-                    cat /home/{u}/.flintrock-manifest.json
-                """.format(u=shlex.quote(user)))
+                    cat "$HOME/.flintrock-manifest.json"
+                """)
             # TODO: Would it be better if storage (ephemeral and otherwise) was
             #       implemented as a Flintrock service and tracked in the manifest?
             ephemeral_dirs_raw = ssh_check_output(
@@ -570,10 +570,10 @@ def setup_node(
         command="""
             set -e
 
-            echo {private_key} > ~/.ssh/id_rsa
-            echo {public_key} >> ~/.ssh/authorized_keys
+            echo {private_key} > "$HOME/.ssh/id_rsa"
+            echo {public_key} >> "$HOME/.ssh/authorized_keys"
 
-            chmod 400 ~/.ssh/id_rsa
+            chmod 400 "$HOME/.ssh/id_rsa"
         """.format(
             private_key=shlex.quote(cluster.ssh_key_pair.private),
             public_key=shlex.quote(cluster.ssh_key_pair.public)))
@@ -644,11 +644,11 @@ def provision_cluster(
         ssh_check_output(
             client=master_ssh_client,
             command="""
-                echo {m} > /home/{u}/.flintrock-manifest.json
-                chmod go-rw /home/{u}/.flintrock-manifest.json
+                echo {m} > "$HOME/.flintrock-manifest.json"
+                chmod go-rw "$HOME/.flintrock-manifest.json"
             """.format(
-                m=shlex.quote(json.dumps(manifest, indent=4, sort_keys=True)),
-                u=shlex.quote(user)))
+                m=shlex.quote(json.dumps(manifest, indent=4, sort_keys=True))
+            ))
 
         for service in services:
             service.configure_master(
@@ -861,4 +861,4 @@ def copy_file_node(
 # core.py and services.py. I've thought about how to remove this circular dependency,
 # but for now this seems like what we need to go with.
 # Flintrock modules
-from .services import HDFS, Spark  # Used by start_cluster()
+from .services import HDFS, Spark  # Used by start_cluster() # noqa
diff --git a/flintrock/ec2.py b/flintrock/ec2.py
index 6be0deb..7a2a61a 100644
--- a/flintrock/ec2.py
+++ b/flintrock/ec2.py
@@ -4,7 +4,6 @@ import sys
 import time
 import urllib.request
 import base64
-import os
 from collections import namedtuple
 from datetime import datetime
 
@@ -234,6 +233,7 @@ class EC2Cluster(FlintrockCluster):
             identity_file: str,
             num_slaves: int,
             spot_price: float,
+            tags: list,
             assume_yes: bool):
         security_group_ids = [
             group['GroupId']
@@ -287,15 +287,17 @@ class EC2Cluster(FlintrockCluster):
                 instance_initiated_shutdown_behavior=instance_initiated_shutdown_behavior,
                 user_data=user_data)
 
+            slave_tags = [
+                {'Key': 'flintrock-role', 'Value': 'slave'},
+                {'Key': 'Name', 'Value': '{c}-slave'.format(c=self.name)}]
+            slave_tags += tags
+
             (ec2.instances
                 .filter(
                     Filters=[
                         {'Name': 'instance-id', 'Values': [i.id for i in new_slave_instances]}
                     ])
-                .create_tags(
-                    Tags=[
-                        {'Key': 'flintrock-role', 'Value': 'slave'},
-                        {'Key': 'Name', 'Value': '{c}-slave'.format(c=self.name)}]))
+                .create_tags(Tags=slave_tags))
 
             existing_slaves = {i.public_ip_address for i in self.slave_instances}
 
@@ -817,7 +819,8 @@ def launch(
         tenancy='default',
         ebs_optimized=False,
         instance_initiated_shutdown_behavior='stop',
-        user_data):
+        user_data,
+        tags):
     """
     Launch a cluster.
     """
@@ -898,24 +901,29 @@ def launch(
         master_instance = cluster_instances[0]
         slave_instances = cluster_instances[1:]
 
+        master_tags = [
+            {'Key': 'flintrock-role', 'Value': 'master'},
+            {'Key': 'Name', 'Value': '{c}-master'.format(c=cluster_name)}]
+        master_tags += tags
+
         (ec2.instances
             .filter(
                 Filters=[
                     {'Name': 'instance-id', 'Values': [master_instance.id]}
                 ])
-            .create_tags(
-                Tags=[
-                    {'Key': 'flintrock-role', 'Value': 'master'},
-                    {'Key': 'Name', 'Value': '{c}-master'.format(c=cluster_name)}]))
+            .create_tags(Tags=master_tags))
+
+        slave_tags = [
+            {'Key': 'flintrock-role', 'Value': 'slave'},
+            {'Key': 'Name', 'Value': '{c}-slave'.format(c=cluster_name)}]
+        slave_tags += tags
+
         (ec2.instances
             .filter(
                 Filters=[
                     {'Name': 'instance-id', 'Values': [i.id for i in slave_instances]}
                 ])
-            .create_tags(
-                Tags=[
-                    {'Key': 'flintrock-role', 'Value': 'slave'},
-                    {'Key': 'Name', 'Value': '{c}-slave'.format(c=cluster_name)}]))
+            .create_tags(Tags=slave_tags))
 
         cluster = EC2Cluster(
             name=cluster_name,
@@ -1005,6 +1013,30 @@ def get_clusters(*, cluster_names: list=[], region: str, vpc_id: str) -> list:
     return clusters
 
 
+def cli_validate_tags(ctx, param, value):
+    return validate_tags(value)
+
+
+def validate_tags(value):
+    """
+    Validate and parse optional EC2 tags.
+    """
+    err_msg = ("Tags need to be specified as 'Key,Value' pairs "
+               "separated by a single comma. Key cannot be empty "
+               "or be made up entirely of whitespace.")
+    tags = value
+    result = []
+    for tag in tags:
+        if tag.count(',') != 1:
+            raise click.BadParameter(err_msg)
+        key, value = [word.strip() for word in tag.split(',', maxsplit=1)]
+        if not key:
+            raise click.BadParameter(err_msg)
+        result.append({'Key': key, 'Value': value})
+
+    return result
+
+
 def _get_cluster_name(instance: 'boto3.resources.factory.ec2.Instance') -> str:
     """
     Given an EC2 instance, get the name of the Flintrock cluster it belongs to.
diff --git a/flintrock/flintrock.py b/flintrock/flintrock.py
index 58cb9d6..25721d6 100644
--- a/flintrock/flintrock.py
+++ b/flintrock/flintrock.py
@@ -236,6 +236,11 @@ def cli(cli_context, config, provider):
 @click.option('--ec2-user-data',
               type=click.File(mode='r', encoding='utf-8'),
               help="Path to EC2 user data script that will run on instance launch.")
+@click.option('--ec2-tag', 'ec2_tags',
+              callback=ec2.cli_validate_tags,
+              multiple=True,
+              help="Additional tags (e.g. 'Key,Value') to assign to the instances. "
+                   "You can specify this option multiple times.")
 @click.pass_context
 def launch(
         cli_context,
@@ -266,7 +271,8 @@ def launch(
         ec2_tenancy,
         ec2_ebs_optimized,
         ec2_instance_initiated_shutdown_behavior,
-        ec2_user_data):
+        ec2_user_data,
+        ec2_tags):
     """
     Launch a new cluster.
     """
@@ -356,7 +362,8 @@ def launch(
             tenancy=ec2_tenancy,
             ebs_optimized=ec2_ebs_optimized,
             instance_initiated_shutdown_behavior=ec2_instance_initiated_shutdown_behavior,
-            user_data=ec2_user_data)
+            user_data=ec2_user_data,
+            tags=ec2_tags)
     else:
         raise UnsupportedProviderError(provider)
 
@@ -619,6 +626,11 @@ def stop(cli_context, cluster_name, ec2_region, ec2_vpc_id, assume_yes):
 @click.option('--ec2-user')
 @click.option('--ec2-spot-price', type=float)
 @click.option('--assume-yes/--no-assume-yes', default=False)
+@click.option('--ec2-tag', 'ec2_tags',
+              callback=ec2.cli_validate_tags,
+              multiple=True,
+              help="Additional tags (e.g. 'Key,Value') to assign to the instances. "
+                   "You can specify this option multiple times.")
 @click.pass_context
 def add_slaves(
         cli_context,
@@ -629,6 +641,7 @@ def add_slaves(
         ec2_identity_file,
         ec2_user,
         ec2_spot_price,
+        ec2_tags,
         assume_yes):
     """
     Add slaves to an existing cluster.
@@ -656,6 +669,7 @@ def add_slaves(
         identity_file = ec2_identity_file
         provider_options = {
             'spot_price': ec2_spot_price,
+            'tags': ec2_tags
         }
     else:
         raise UnsupportedProviderError(provider)
diff --git a/flintrock/scripts/setup-ephemeral-storage.py b/flintrock/scripts/setup-ephemeral-storage.py
index b246c1c..dcb8876 100644
--- a/flintrock/scripts/setup-ephemeral-storage.py
+++ b/flintrock/scripts/setup-ephemeral-storage.py
@@ -12,6 +12,7 @@ The resulting structure we create is as follows:
         /root: The instance's root volume.
         /ephemeral[0-N]: Instance store volumes.
         /persistent[0-N]: EBS volumes.
+        /tmp: A temporary directory with lots of space.
 
 WARNING: Be conscious about what this script prints to stdout, as that
          output is parsed by Flintrock.
@@ -97,7 +98,7 @@ def format_devices(devices):
 
     for p in format_processes:
         stdout_raw, stderr_raw = p.communicate()
-        stdout, stderr = stdout_raw.decode('utf-8'), stderr_raw.decode('utf-8')
+        stdout, stderr = stdout_raw.decode('utf-8'), stderr_raw.decode('utf-8')  # noqa
         return_code = p.returncode
         if return_code != 0:
             raise Exception(
@@ -131,7 +132,7 @@ def mount_devices(devices):
                 device.name,
                 device.mount_point,
                 'ext4',
-                'defaults,users,noatime,nodiratime',
+                'defaults,users,noatime',
                 '0',
                 '0'])),
             shell=True)
@@ -159,6 +160,19 @@ def create_root_dir():
     return path
 
 
+def create_tmp_dir(target):
+    """
+    Create a folder that services can use as a temporary directory for big files.
+    """
+    path = '/media/tmp'
+    subprocess.check_output([
+        'sudo', 'ln', '-s', target, path])
+    subprocess.check_output(
+        'sudo chown "$(logname):$(logname)" {p}'.format(p=path),
+        shell=True)
+    return path
+
+
 if __name__ == '__main__':
     if sys.version_info < (2, 7) or ((3, 0) <= sys.version_info < (3, 4)):
         raise Exception(
@@ -182,6 +196,11 @@ if __name__ == '__main__':
     mount_devices(ephemeral_devices)
 
     root_dir = create_root_dir()
+    if ephemeral_devices:
+        tmp_dir = ephemeral_devices[0].mount_point
+    else:
+        tmp_dir = '/tmp'
+    create_tmp_dir(tmp_dir)
 
     print(json.dumps(
         {
diff --git a/flintrock/services.py b/flintrock/services.py
index f850d3e..48f36a5 100644
--- a/flintrock/services.py
+++ b/flintrock/services.py
@@ -192,7 +192,7 @@ class HDFS(FlintrockService):
         hdfs_master_ui = 'http://{m}:50070/webhdfs/v1/?op=GETCONTENTSUMMARY'.format(m=master_host)
 
         try:
-            hdfs_ui_info = json.loads(
+            hdfs_ui_info = json.loads(  # noqa
                 urllib.request.urlopen(hdfs_master_ui).read().decode('utf-8'))
         except Exception as e:
             # TODO: Catch a more specific problem.
diff --git a/flintrock/ssh.py b/flintrock/ssh.py
index ba0469e..ad0049c 100644
--- a/flintrock/ssh.py
+++ b/flintrock/ssh.py
@@ -129,7 +129,7 @@ def ssh(*, user: str, host: str, identity_file: str):
     """
     SSH into a host for interactive use.
     """
-    ret = subprocess.call(
+    subprocess.call(
         [
             'ssh',
             '-o', 'StrictHostKeyChecking=no',
diff --git a/flintrock/templates/hadoop/conf/hadoop-env.sh b/flintrock/templates/hadoop/conf/hadoop-env.sh
index 39805ba..e872b48 100644
--- a/flintrock/templates/hadoop/conf/hadoop-env.sh
+++ b/flintrock/templates/hadoop/conf/hadoop-env.sh
@@ -1,2 +1,2 @@
-export HADOOP_HOME="/home/$(logname)/hadoop"
+export HADOOP_HOME="$HOME/hadoop"
 export HADOOP_SSH_OPTS="-o StrictHostKeyChecking=no -o ConnectTimeout=5"
diff --git a/flintrock/templates/spark/conf/spark-env.sh b/flintrock/templates/spark/conf/spark-env.sh
index 77bd7f8..968e659 100644
--- a/flintrock/templates/spark/conf/spark-env.sh
+++ b/flintrock/templates/spark/conf/spark-env.sh
@@ -9,7 +9,7 @@ export SPARK_WORKER_CORES="$(nproc)"
 export SPARK_MASTER_HOST="{master_host}"
 
 # TODO: Make this dependent on HDFS install.
-export HADOOP_CONF_DIR="/home/$(logname)/hadoop/conf"
+export HADOOP_CONF_DIR="$HOME/hadoop/conf"
 
 # TODO: Make this non-EC2-specific.
 # Bind Spark's web UIs to this machine's public EC2 hostname
diff --git a/generate-standalone-package.py b/generate-standalone-package.py
index c142035..0e33d25 100644
--- a/generate-standalone-package.py
+++ b/generate-standalone-package.py
@@ -13,7 +13,8 @@ if __name__ == '__main__':
         operating_system = 'OSX'
     machine_type = platform.machine()
 
-    subprocess.run([
+    subprocess.run(
+        [
             'pyinstaller',
             '--noconfirm',
             '--clean',
@@ -23,7 +24,8 @@ if __name__ == '__main__':
             # We won't need this when this issue is resolved:
             # https://github.com/pyinstaller/pyinstaller/issues/1844
             '--hidden-import', 'html.parser',
-            'standalone.py'],
+            'standalone.py'
+        ],
         check=True)
 
     shutil.make_archive(
diff --git a/hook-flintrock.py b/hook-flintrock.py
index 5478cf8..38b8908 100644
--- a/hook-flintrock.py
+++ b/hook-flintrock.py
@@ -1,4 +1,4 @@
-datas=[
+datas = [
     ('flintrock/scripts', './scripts'),
     ('flintrock/templates', './templates'),
     ('flintrock/config.yaml.template', './'),
diff --git a/make-release.sh b/make-release.sh
index 7419b5b..ae51d0b 100644
--- a/make-release.sh
+++ b/make-release.sh
@@ -5,6 +5,7 @@
 #   - Default Amazon Linux AMI
 # Update CHANGES
 #   - Check: https://github.com/nchammas/flintrock/pulls?q=is%3Apr+is%3Aclosed+label%3A%22needs+changelog%22
+#   - Update "Unreleased" section. "Nothing notable yet."
 # Tag release on GitHub
 
 trash dist/ build/ Flintrock.egg-info/
diff --git a/requirements/developer.pip b/requirements/developer.pip
index ef0a99c..2ec3050 100644
--- a/requirements/developer.pip
+++ b/requirements/developer.pip
@@ -1,5 +1,5 @@
 -r user.pip
-pycodestyle == 2.2.0
-pytest >= 3.0.4
+pytest >= 3.0.6
 pytest-cov >= 2.4.0
+flake8 == 3.3.0
 # PyYAML  # requirement already covered by setup.py
diff --git a/setup.cfg b/setup.cfg
new file mode 100644
index 0000000..9f5236f
--- /dev/null
+++ b/setup.cfg
@@ -0,0 +1,8 @@
+[tool:pytest]
+norecursedirs = venv
+addopts = --verbose --cov flintrock --cov-report html
+
+[flake8]
+max-line-length = 100
+exclude = venv, build, dist
+ignore = E501
diff --git a/setup.py b/setup.py
index c045bd7..3ccead7 100644
--- a/setup.py
+++ b/setup.py
@@ -49,10 +49,10 @@ setuptools.setup(
     # totally break Flintrock.
     # For example: https://github.com/paramiko/paramiko/issues/615
     install_requires=[
-        'boto3 == 1.4.3',
-        'botocore == 1.4.91',
-        'click == 6.6',
-        'paramiko == 2.0.2',
+        'boto3 == 1.4.4',
+        'botocore == 1.5.10',
+        'click == 6.7',
+        'paramiko == 2.1.1',
         'PyYAML == 3.12',
         # This is to ensure that PyInstaller works. dateutil is an
         # indirect dependency of Flintrock, and PyInstaller chokes on
@@ -63,7 +63,7 @@ setuptools.setup(
         # of Flintrock intermittently fail due to an out-of-date version
         # of Cryptography being used.
         # See: https://github.com/nchammas/flintrock/issues/169
-        'cryptography >= 1.7.1',
+        'cryptography >= 1.7.2',
     ],
 
     entry_points={
diff --git a/tox.ini b/tox.ini
deleted file mode 100644
index 3bb4d36..0000000
--- a/tox.ini
+++ /dev/null
@@ -1,7 +0,0 @@
-[pycodestyle]
-max-line-length=100
-ignore=E501,E402
-
-[pytest]
-norecursedirs=venv
-addopts=--verbose --cov flintrock --cov-report html


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 36fa8a5..48298f8 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -5,7 +5,7 @@ import uuid
 from collections import OrderedDict
 
 # Flintrock
-from flintrock.core import FlintrockCluster, StorageDirs
+from flintrock.core import StorageDirs
 
 # External
 import pytest
diff --git a/tests/test_ec2.py b/tests/test_ec2.py
new file mode 100644
index 0000000..fd60eb7
--- /dev/null
+++ b/tests/test_ec2.py
@@ -0,0 +1,32 @@
+import pytest
+import click
+from flintrock.ec2 import validate_tags
+
+
+def test_validate_tags():
+    # List of test cases; each test case is a tuple, with first element
+    # the input and the second element the expected output
+    positive_test_cases = [
+        # basic case
+        (['k1,v1'], [{'Key': 'k1', 'Value': 'v1'}]),
+        # strips whitespace?
+        (['k2, v2 '], [{'Key': 'k2', 'Value': 'v2'}]),
+        # empty Value
+        (['k3,'], [{'Key': 'k3', 'Value': ''}]),
+        # multiple tags
+        (['k4,v4', 'k5,v5'],
+         [{'Key': 'k4', 'Value': 'v4'}, {'Key': 'k5', 'Value': 'v5'}])]
+
+    for test_case in positive_test_cases:
+        ec2_tags = validate_tags(test_case[0])
+        assert(isinstance(ec2_tags, list))
+        for i, ec2_tag in enumerate(ec2_tags):
+            expected_dict = test_case[1][i]
+            for k in expected_dict:
+                assert k in ec2_tag
+                assert ec2_tag[k] == expected_dict[k]
+
+    negative_test_cases = [["k1"], ["k2,v2,"], ["k3,,v3"], [",v4"]]
+    for test_case in negative_test_cases:
+        with pytest.raises(click.BadParameter):
+            validate_tags(test_case)
diff --git a/tests/test_static.py b/tests/test_static.py
index e5024b6..fde3cff 100644
--- a/tests/test_static.py
+++ b/tests/test_static.py
@@ -1,8 +1,8 @@
 import compileall
 import os
+import subprocess
 
 # External modules
-import pycodestyle
 import yaml
 
 FLINTROCK_ROOT_DIR = (
@@ -31,11 +31,9 @@ def test_code_compiles():
         assert result == 1
 
 
-def test_style():
-    style = pycodestyle.StyleGuide(
-        config_file=os.path.join(FLINTROCK_ROOT_DIR, 'tox.ini'))
-    result = style.check_files(TEST_PATHS)
-    assert result.total_errors == 0
+def test_flake8():
+    ret = subprocess.call(['flake8'], cwd=FLINTROCK_ROOT_DIR)
+    assert ret == 0
 
 
 def test_config_template_is_valid():