Repository: nchammas/flintrock (language: python)

Issue:
Support spot request duration when adding slaves
This PR follows up on #315 adding the spot request duration to "add slaves".

I have ran the the tests and the only failure was flake8. I was not able to run the acceptance tests however (I think it's a configuration issue on my end but I don't have a lot of time to dig into it right now). I did manually launch / add slaves / remove slaves / destroy, and verified the valid until date of a spot requested cluster is correct in the EC2 dashboard.

Apologies I did not cover this in the first PR!


Discussion / hints:

Fix patch:
diff --git a/flintrock/ec2.py b/flintrock/ec2.py
index c6890a57..f84cc61c 100644
--- a/flintrock/ec2.py
+++ b/flintrock/ec2.py
@@ -5,7 +5,7 @@
 import urllib.request
 import base64
 import logging
-from datetime import datetime, timedelta, timezone
+from datetime import datetime
 
 # External modules
 import boto3
@@ -25,7 +25,7 @@
 )
 from .ssh import generate_ssh_key_pair
 from .services import SecurityGroupRule
-from .util import duration_to_timedelta
+from .util import duration_to_expiration
 
 logger = logging.getLogger('flintrock.ec2')
 
@@ -251,7 +251,7 @@ def add_slaves(
             num_slaves: int,
             java_version: int,
             spot_price: float,
-            spot_request_valid_until: str,
+            spot_request_duration: str,
             min_root_ebs_size_gb: int,
             tags: list,
             assume_yes: bool):
@@ -296,7 +296,7 @@ def add_slaves(
                 num_instances=num_slaves,
                 region=self.region,
                 spot_price=spot_price,
-                spot_request_valid_until=spot_request_valid_until,
+                spot_request_valid_until=duration_to_expiration(spot_request_duration),
                 ami=self.master_instance.image_id,
                 assume_yes=assume_yes,
                 key_name=self.master_instance.key_name,
@@ -873,17 +873,12 @@ def launch(
     else:
         user_data = ''
 
-    if spot_request_duration is None:
-        spot_request_valid_until = datetime.now(tz=timezone.utc) + timedelta(days=7)
-    else:
-        spot_request_valid_until = datetime.now(tz=timezone.utc) + duration_to_timedelta(spot_request_duration)
-
     try:
         cluster_instances = _create_instances(
             num_instances=num_instances,
             region=region,
             spot_price=spot_price,
-            spot_request_valid_until=spot_request_valid_until,
+            spot_request_valid_until=duration_to_expiration(spot_request_duration),
             ami=ami,
             assume_yes=assume_yes,
             key_name=key_name,
diff --git a/flintrock/flintrock.py b/flintrock/flintrock.py
index 0273e9e3..e50e4408 100644
--- a/flintrock/flintrock.py
+++ b/flintrock/flintrock.py
@@ -777,7 +777,7 @@ def add_slaves(
         provider_options = {
             'min_root_ebs_size_gb': ec2_min_root_ebs_size_gb,
             'spot_price': ec2_spot_price,
-            'spot_request_valid_until': ec2_spot_request_duration,
+            'spot_request_duration': ec2_spot_request_duration,
             'tags': ec2_tags
         }
     else:
diff --git a/flintrock/util.py b/flintrock/util.py
index e456f6dc..c7b94e92 100644
--- a/flintrock/util.py
+++ b/flintrock/util.py
@@ -1,6 +1,6 @@
 import os
 import sys
-from datetime import timedelta
+from datetime import datetime, timedelta, timezone
 from decimal import Decimal
 
 FROZEN = getattr(sys, 'frozen', False)
@@ -48,3 +48,14 @@ def duration_to_timedelta(duration_string):
             prev_num.append(character)
 
     return timedelta(seconds=float(total_seconds))
+
+
+def duration_to_expiration(duration_string):
+    default_duration = timedelta(days=7)
+
+    if not duration_string:
+        expiration = datetime.now(tz=timezone.utc) + default_duration
+    else:
+        expiration = datetime.now(tz=timezone.utc) + duration_to_timedelta(duration_string)
+
+    return expiration
diff --git a/requirements/developer.in b/requirements/developer.in
index 24c0f134..7e7c12e4 100644
--- a/requirements/developer.in
+++ b/requirements/developer.in
@@ -2,4 +2,5 @@
 pytest >= 3.5.0
 pytest-cov >= 2.5.1
 flake8 == 3.5.0
+freezegun == 1.1.0
 # PyYAML  # requirement already covered by setup.py
diff --git a/requirements/developer.pip b/requirements/developer.pip
index d1145a9d..09b34f8c 100644
--- a/requirements/developer.pip
+++ b/requirements/developer.pip
@@ -5,35 +5,117 @@
 #    pip-compile --output-file=requirements/developer.pip requirements/developer.in
 #
 -e file:.#egg=Flintrock
-attrs==19.3.0             # via pytest
+    # via -r requirements/user.pip
+attrs==19.3.0
+    # via pytest
 bcrypt==3.1.7
+    # via
+    #   -r requirements/user.pip
+    #   paramiko
 boto3==1.10.45
+    # via
+    #   -r requirements/user.pip
+    #   flintrock
 botocore==1.13.45
+    # via
+    #   -r requirements/user.pip
+    #   boto3
+    #   flintrock
+    #   s3transfer
 cffi==1.13.2
+    # via
+    #   -r requirements/user.pip
+    #   bcrypt
+    #   cryptography
+    #   pynacl
 click==7.0
-coverage==5.0.1           # via pytest-cov
+    # via
+    #   -r requirements/user.pip
+    #   flintrock
+coverage==5.0.1
+    # via pytest-cov
 cryptography==2.8
+    # via
+    #   -r requirements/user.pip
+    #   flintrock
+    #   paramiko
 docutils==0.15.2
+    # via
+    #   -r requirements/user.pip
+    #   botocore
 flake8==3.5.0
-importlib-metadata==1.3.0  # via pluggy, pytest
+    # via -r requirements/developer.in
+freezegun==1.1.0
+    # via -r requirements/developer.in
+importlib-metadata==1.3.0
+    # via
+    #   pluggy
+    #   pytest
 jmespath==0.9.4
-mccabe==0.6.1             # via flake8
-more-itertools==8.0.2     # via pytest, zipp
-packaging==19.2           # via pytest
+    # via
+    #   -r requirements/user.pip
+    #   boto3
+    #   botocore
+mccabe==0.6.1
+    # via flake8
+more-itertools==8.0.2
+    # via pytest
+packaging==19.2
+    # via pytest
 paramiko==2.7.1
-pluggy==0.13.1            # via pytest
-py==1.8.0                 # via pytest
-pycodestyle==2.3.1        # via flake8
+    # via
+    #   -r requirements/user.pip
+    #   flintrock
+pluggy==0.13.1
+    # via pytest
+py==1.8.0
+    # via pytest
+pycodestyle==2.3.1
+    # via flake8
 pycparser==2.19
-pyflakes==1.6.0           # via flake8
+    # via
+    #   -r requirements/user.pip
+    #   cffi
+pyflakes==1.6.0
+    # via flake8
 pynacl==1.3.0
-pyparsing==2.4.6          # via packaging
+    # via
+    #   -r requirements/user.pip
+    #   paramiko
+pyparsing==2.4.6
+    # via packaging
 pytest-cov==2.8.1
+    # via -r requirements/developer.in
 pytest==5.3.2
+    # via
+    #   -r requirements/developer.in
+    #   pytest-cov
 python-dateutil==2.8.1
+    # via
+    #   -r requirements/user.pip
+    #   botocore
+    #   freezegun
 pyyaml==5.2
+    # via
+    #   -r requirements/user.pip
+    #   flintrock
 s3transfer==0.2.1
+    # via
+    #   -r requirements/user.pip
+    #   boto3
 six==1.13.0
+    # via
+    #   -r requirements/user.pip
+    #   bcrypt
+    #   cryptography
+    #   packaging
+    #   pynacl
+    #   python-dateutil
 urllib3==1.25.7
-wcwidth==0.1.7            # via pytest
-zipp==0.6.0               # via importlib-metadata
+    # via
+    #   -r requirements/user.pip
+    #   botocore
+wcwidth==0.1.7
+    # via pytest
+zipp==0.6.0
+    # via importlib-metadata
diff --git a/requirements/maintainer.pip b/requirements/maintainer.pip
index ed08fb50..21c4e7a4 100644
--- a/requirements/maintainer.pip
+++ b/requirements/maintainer.pip
@@ -5,55 +5,190 @@
 #    pip-compile --output-file=requirements/maintainer.pip requirements/maintainer.in
 #
 -e file:.#egg=Flintrock
-altgraph==0.16.1          # via macholib, pyinstaller
+    # via -r requirements/developer.pip
+altgraph==0.17
+    # via
+    #   macholib
+    #   pyinstaller
 attrs==19.3.0
+    # via
+    #   -r requirements/developer.pip
+    #   pytest
 bcrypt==3.1.7
-bleach==3.1.0             # via readme-renderer
+    # via
+    #   -r requirements/developer.pip
+    #   paramiko
+bleach==3.3.0
+    # via readme-renderer
 boto3==1.10.45
+    # via
+    #   -r requirements/developer.pip
+    #   flintrock
 botocore==1.13.45
-certifi==2019.11.28       # via requests
+    # via
+    #   -r requirements/developer.pip
+    #   boto3
+    #   flintrock
+    #   s3transfer
+certifi==2020.12.5
+    # via requests
 cffi==1.13.2
-chardet==3.0.4            # via requests
+    # via
+    #   -r requirements/developer.pip
+    #   bcrypt
+    #   cryptography
+    #   pynacl
+chardet==4.0.0
+    # via requests
 click==7.0
+    # via
+    #   -r requirements/developer.pip
+    #   flintrock
 coverage==5.0.1
+    # via
+    #   -r requirements/developer.pip
+    #   pytest-cov
 cryptography==2.8
+    # via
+    #   -r requirements/developer.pip
+    #   flintrock
+    #   paramiko
 docutils==0.15.2
+    # via
+    #   -r requirements/developer.pip
+    #   botocore
+    #   readme-renderer
 flake8==3.5.0
-idna==2.8                 # via requests
+    # via -r requirements/developer.pip
+freezegun==1.1.0
+    # via -r requirements/developer.pip
+idna==2.10
+    # via requests
 importlib-metadata==1.3.0
+    # via
+    #   -r requirements/developer.pip
+    #   keyring
+    #   pluggy
+    #   pytest
 jmespath==0.9.4
+    # via
+    #   -r requirements/developer.pip
+    #   boto3
+    #   botocore
 keyring==20.0.1
-macholib==1.11            # via pyinstaller
+    # via -r requirements/maintainer.in
+macholib==1.14
+    # via pyinstaller
 mccabe==0.6.1
+    # via
+    #   -r requirements/developer.pip
+    #   flake8
 more-itertools==8.0.2
+    # via
+    #   -r requirements/developer.pip
+    #   pytest
+    #   zipp
 packaging==19.2
+    # via
+    #   -r requirements/developer.pip
+    #   bleach
+    #   pytest
 paramiko==2.7.1
-pkginfo==1.5.0.1          # via twine
+    # via
+    #   -r requirements/developer.pip
+    #   flintrock
+pkginfo==1.7.0
+    # via twine
 pluggy==0.13.1
+    # via
+    #   -r requirements/developer.pip
+    #   pytest
 py==1.8.0
+    # via
+    #   -r requirements/developer.pip
+    #   pytest
 pycodestyle==2.3.1
+    # via
+    #   -r requirements/developer.pip
+    #   flake8
 pycparser==2.19
+    # via
+    #   -r requirements/developer.pip
+    #   cffi
 pyflakes==1.6.0
-pygments==2.5.2           # via readme-renderer
+    # via
+    #   -r requirements/developer.pip
+    #   flake8
+pygments==2.7.4
+    # via readme-renderer
 pyinstaller==3.5
+    # via -r requirements/maintainer.in
 pynacl==1.3.0
+    # via
+    #   -r requirements/developer.pip
+    #   paramiko
 pyparsing==2.4.6
+    # via
+    #   -r requirements/developer.pip
+    #   packaging
 pytest-cov==2.8.1
+    # via -r requirements/developer.pip
 pytest==5.3.2
+    # via
+    #   -r requirements/developer.pip
+    #   pytest-cov
 python-dateutil==2.8.1
+    # via
+    #   -r requirements/developer.pip
+    #   botocore
+    #   freezegun
 pyyaml==5.2
-readme-renderer==24.0     # via twine
-requests-toolbelt==0.9.1  # via twine
-requests==2.22.0          # via requests-toolbelt, twine
+    # via
+    #   -r requirements/developer.pip
+    #   flintrock
+readme-renderer==28.0
+    # via twine
+requests-toolbelt==0.9.1
+    # via twine
+requests==2.25.1
+    # via
+    #   requests-toolbelt
+    #   twine
 s3transfer==0.2.1
+    # via
+    #   -r requirements/developer.pip
+    #   boto3
 six==1.13.0
-tqdm==4.41.0              # via twine
+    # via
+    #   -r requirements/developer.pip
+    #   bcrypt
+    #   bleach
+    #   cryptography
+    #   packaging
+    #   pynacl
+    #   python-dateutil
+    #   readme-renderer
+tqdm==4.56.0
+    # via twine
 twine==1.15.0
+    # via -r requirements/maintainer.in
 urllib3==1.25.7
+    # via
+    #   -r requirements/developer.pip
+    #   botocore
+    #   requests
 wcwidth==0.1.7
-webencodings==0.5.1       # via bleach
-wheel==0.33.6
+    # via
+    #   -r requirements/developer.pip
+    #   pytest
+webencodings==0.5.1
+    # via bleach
+wheel==0.36.2
+    # via -r requirements/maintainer.in
 zipp==0.6.0
+    # via
+    #   -r requirements/developer.pip
+    #   importlib-metadata
 
 # The following packages are considered to be unsafe in a requirements file:
 # setuptools


Test patch:
diff --git a/tests/test_util.py b/tests/test_util.py
index ed77529e..84256a3d 100644
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -1,5 +1,6 @@
-from datetime import timedelta
-from flintrock.util import duration_to_timedelta
+from datetime import datetime, timedelta, timezone
+from flintrock.util import duration_to_timedelta, duration_to_expiration
+from freezegun import freeze_time
 
 
 def test_duration_to_timedelta():
@@ -8,3 +9,8 @@ def test_duration_to_timedelta():
     assert duration_to_timedelta('4d 2h 1m 5s') == timedelta(days=4, hours=2, minutes=1, seconds=5)
     assert duration_to_timedelta('36h') == timedelta(hours=36)
     assert duration_to_timedelta('7d') == timedelta(days=7)
+
+
+@freeze_time("2012-01-14")
+def test_duration_to_expiration():
+    assert duration_to_expiration('5m') == datetime.now(tz=timezone.utc) + timedelta(minutes=5)