merge dev to main (#38)

* Split normalize_config into two functions (#27) * Split normalize_config into two functions * Add test cases for normalize_config and parse_additional_config * Bug Fix: on test_parse_additional_config * Update test_parse_additional_config * Added new badges to the Readme (#30) * Move EC2 pricing calls to single function. (#29) * Added more clear SSH error message for improper credentials * Updated changelog * Fixed changelog * Updated SSH credential error message * Add tests for ssh.py module. * Add coverage as a test dependency. * Update changelog and fix style. * Add unittests for rsync module. (#33) * Add tests for yaml_loader.py to increase coverage (#34) * Add tests for yaml_loader.py to increase coverage * remove redundant imports Co-authored-by: ali <[email protected]> * moved function outside for better testing (#35) Co-authored-by: ali <[email protected]> * Added venv to .gitignore * bump version (#37) * Added venv to .gitignore * bump version bump version so we can merge with main * Update CHANGELOG.md update link for unreleased * Update CHANGELOG.md update link for unreleased Co-authored-by: Gabriele A. Ron <[email protected]> Co-authored-by: Gabe Ron <[email protected]> Co-authored-by: Heshanthaka <[email protected]> Co-authored-by: Joao Moreira <[email protected]> Co-authored-by: Gabriele A. Ron <[email protected]> Co-authored-by: Mohammed Ali Zubair <[email protected]> Co-authored-by: ali <[email protected]>
carsdotcom · Oct 27, 2022 · 05bbfdf · 05bbfdf
1 parent ba14db9
commit 05bbfdf
Show file tree

Hide file tree

Showing 20 changed files with 767 additions and 158 deletions.
diff --git a/.bumpversion.cfg b/.bumpversion.cfg
@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 1.0.1
+current_version = 1.0.2
 commit = True
 tag = False
 parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)

diff --git a/.gitignore b/.gitignore
@@ -15,3 +15,5 @@ __pycache__/
 *.egg-info/
 build/
 dist/
+.idea/
+venv/
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,9 +6,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ## [Unreleased]
 
+
+## [1.0.2] - 2022-10-27
+
 ### Added
+- **Tests** - Test for `ssh.py` and `rsync.py` module.
 - **GitHub** - Workflow to run unittests on every PR and push to main.
 
+### Changed
+- **SSH** - Add error to show when SSH credentials are invalid.
+- **Dependencies** - Add `coverage` as a test dependency.
+- **Readme** - Add new badges to the Readme.
+- **Common** - Move EC2 pricing calls to single function in `common.py`.
+
 
 ## [1.0.1] - 2022-09-28
 
@@ -23,12 +33,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - **GitHub** - Update action to build and publish package only when version is bumped.
 - **Forge** - Added automatic tag `forge-name` to allow `Name` tag to be changed.
 
-
 ## [1.0.0] - 2022-09-27
 
 ### Added
 - **Initial commit** - Forge source code, unittests, docs, pyproject.toml, README.md, and LICENSE files.
 
-[unreleased]: https://github.com/carsdotcom/cars-forge/compare/v1.0.1...HEAD
+[unreleased]: https://github.com/carsdotcom/cars-forge/compare/v1.0.2...HEAD
+[1.0.2]: https://github.com/carsdotcom/cars-forge/compare/v1.0.1...v1.0.2
 [1.0.1]: https://github.com/carsdotcom/cars-forge/compare/v1.0.0...v1.0.1
 [1.0.0]: https://github.com/carsdotcom/cars-forge/releases/tag/v1.0.0
diff --git a/README.md b/README.md
@@ -1,5 +1,11 @@
-<p align="center"><img src="docs/logo.jpg"></p>
-
+<p align="center"><img src="https://github.com/carsdotcom/cars-forge/blob/main/docs/logo.jpg?raw=true"></p>
+
+[![GitHub license](https://img.shields.io/github/license/carsdotcom/cars-forge?color=navy&label=License&logo=License&style=flat-square)](https://github.com/carsdotcom/cars-forge/blob/main/LICENSE)
+[![PyPI](https://img.shields.io/pypi/v/cars-forge?color=navy&style=flat-square)](https://pypi.org/project/cars-forge/)
+![hacktoberfest](https://img.shields.io/github/issues/carsdotcom/cars-forge?color=orange&label=Hacktoberfest%202022&style=flat-square&?labelColor=black)
+![PyPI - Downloads](https://img.shields.io/pypi/dm/cars-forge?color=navy&style=flat-square)
+![GitHub Workflow Status (branch)](https://img.shields.io/github/workflow/status/carsdotcom/cars-forge/Publish%20Package/main?color=navy&style=flat-square)
+![GitHub contributors](https://img.shields.io/github/contributors/carsdotcom/cars-forge?color=navy&style=flat-square)
 ---
 
 ## About

diff --git a/pyproject.toml b/pyproject.toml
@@ -44,6 +44,7 @@ dependencies = [
 [project.optional-dependencies]
 test = [
     "pytest~=7.1.0",
+    "pytest-cov~=4.0"
 ]
 dev = [
     "bump2version~=1.0",
@@ -72,3 +73,10 @@ packages = ["src/forge"]
 
 [tool.hatch.version]
 path = "src/forge/__init__.py"
+
+###
+# Pytest settings
+###
+[tool.pytest.ini_options]
+# Show coverage report with missing lines when running `pytest`
+addopts = "--cov=forge --cov-report term-missing"
diff --git a/src/forge/__init__.py b/src/forge/__init__.py
@@ -1,4 +1,4 @@
-__version__ = "1.0.1"
+__version__ = "1.0.2"
 
 # Default values for forge's essential arguments
 DEFAULT_ARG_VALS = {

diff --git a/src/forge/common.py b/src/forge/common.py
@@ -7,6 +7,7 @@
 import tempfile
 import sys
 import os
+from datetime import datetime
 from numbers import Number
 
 import boto3
@@ -290,10 +291,9 @@ def _parse_list(option):
     return option
 
 
-def normalize_config(config, additional_config=None):
+def normalize_config(config):
     """normalizes the Forge configuration data
 
-    If additional_config is present, normalize_config will purely parse additional_config and add it to config.
     If it detects an environmental config data (determined by a lack of ram or cpu data), it processes the ratio and
     updates DEFAULT_ARG_VALS['ratio']. If it detects a user configuration option, it will parse the ram, cpu, ration,
     and market data so that it conforms to Forge's expectation. In either scenario, if it detects aws_az it will update
@@ -303,8 +303,6 @@ def normalize_config(config, additional_config=None):
     ----------
     config : dict
         Forge configuration data
-    additional_config : dict
-        Additional Forge use configuration options
 
     Notes
     -----
@@ -319,12 +317,6 @@ def normalize_config(config, additional_config=None):
     """
     config = dict(config)
 
-    if additional_config:
-        additional_config = {x['name']: x['default'] for x in additional_config if x['default']}
-        config = {**config, **additional_config}
-
-        return config
-
     if config.get('aws_az'):
         config['region'] = config['aws_az'][:-1]
 
@@ -346,6 +338,29 @@ def normalize_config(config, additional_config=None):
     return config
 
 
+def parse_additional_config(config, additional_config):
+    """parse additional configuration data
+
+    Parameters
+    ----------
+    config : dict
+        Forge configuration data
+    additional_config : dict
+        Additional Forge use configuration options
+
+    Returns
+    -------
+    dict
+        The additional Forge configuration data
+    """
+    config = dict(config)
+
+    additional_config = {x['name']: x['default'] for x in additional_config if x['default']}
+    config = {**config, **additional_config}
+
+    return config
+
+
 def set_boto_session(region, profile=None):
     """set the default Boto3 session
 
@@ -458,3 +473,59 @@ def user_accessible_vars(config, **kwargs):
     user_vars.update({k: v for k, v in config.items() if k in ADDITIONAL_KEYS})
 
     return user_vars
+
+
+def get_ec2_pricing(ec2_type, market, config):
+    """Get the hourly spot or on-demand price of given EC2 instance type.
+
+    Parameters
+    ----------
+    ec2_type : str
+        EC2 type to get pricing for.
+    market : str
+        Whether EC2 is a `'spot'` or `'on-demand'` instance.
+    config : dict
+        Forge configuration data.
+
+    Returns
+    -------
+    float
+        Hourly price of given EC2 type in given market.
+    """
+    region = config.get('region')
+    az = config.get('aws_az')
+
+    if market == 'spot':
+        client = boto3.client('ec2')
+        response = client.describe_spot_price_history(
+            StartTime=datetime.utcnow(),
+            ProductDescriptions=['Linux/UNIX (Amazon VPC)'],
+            AvailabilityZone=az,
+            InstanceTypes=[ec2_type]
+        )
+        price = float(response['SpotPriceHistory'][0]['SpotPrice'])
+
+    elif market == 'on-demand':
+        client = boto3.client('pricing', region_name='us-east-1')
+
+        long_region = get_regions()[region]
+        op_sys = 'Linux'
+
+        filters = [
+            {'Field': 'tenancy', 'Value': 'shared', 'Type': 'TERM_MATCH'},
+            {'Field': 'operatingSystem', 'Value': op_sys, 'Type': 'TERM_MATCH'},
+            {'Field': 'preInstalledSw', 'Value': 'NA', 'Type': 'TERM_MATCH'},
+            {'Field': 'location', 'Value': long_region, 'Type': 'TERM_MATCH'},
+            {'Field': 'capacitystatus', 'Value': 'Used', 'Type': 'TERM_MATCH'},
+            {'Field': 'instanceType', 'Value': ec2_type, 'Type': 'TERM_MATCH'}
+        ]
+        response = client.get_products(ServiceCode='AmazonEC2', Filters=filters)
+
+        results = response['PriceList']
+        product = json.loads(results[0])
+        od = product['terms']['OnDemand']
+        price_details = list(od.values())[0]['priceDimensions']
+        price = list(price_details.values())[0]['pricePerUnit']['USD']
+        price = float(price)
+
+    return price
diff --git a/src/forge/create.py b/src/forge/create.py
@@ -4,15 +4,15 @@
 import sys
 import os
 import time
-import json
 from datetime import datetime, timedelta
 
 import boto3
 from botocore.exceptions import ClientError
 
 from . import DEFAULT_ARG_VALS, REQUIRED_ARGS
 from .parser import add_basic_args, add_job_args, add_env_args, add_general_args
-from .common import ec2_ip, get_regions, destroy_hook, set_boto_session, user_accessible_vars, FormatEmpty
+from .common import (ec2_ip, destroy_hook, set_boto_session,
+                     user_accessible_vars, FormatEmpty, get_ec2_pricing)
 from .destroy import destroy
 
 logger = logging.getLogger(__name__)
@@ -251,59 +251,32 @@ def pricing(n, config, fleet_id):
 
     set_boto_session(region, profile)
 
-    az = config.get('aws_az')
     region = config.get('region')
     ec2_client = boto3.client('ec2')
-    pricing_client = boto3.client('pricing', region_name='us-east-1')
-    # get list of EC2s
-    ec2_list = []
+
+    # Get list of active fleet EC2s
+    fleet_types = []
     fleet_request_configs = ec2_client.describe_fleet_instances(FleetId=fleet_id)
-    active_instances_list = fleet_request_configs.get('ActiveInstances')
-    if active_instances_list is None:  # Consider changing to if not active_instances_list:
-        return None
+    for i in fleet_request_configs.get('ActiveInstances', []):
+        fleet_types.append(i['InstanceType'])
 
-    for i in active_instances_list:
-        ec2_list.append(i['InstanceType'])
+    if not fleet_types:
+        return
 
-    # on-demand pricing
-    long_region = get_regions()[region]
-    op_sys = 'Linux'
+    # Get on-demand prices regardless of market
     total_on_demand_cost = 0
-    for ec2 in ec2_list:
-        filters = [
-            {'Field': 'tenancy', 'Value': 'shared', 'Type': 'TERM_MATCH'},
-            {'Field': 'operatingSystem', 'Value': op_sys, 'Type': 'TERM_MATCH'},
-            {'Field': 'preInstalledSw', 'Value': 'NA', 'Type': 'TERM_MATCH'},
-            {'Field': 'location', 'Value': long_region, 'Type': 'TERM_MATCH'},
-            {'Field': 'capacitystatus', 'Value': 'Used', 'Type': 'TERM_MATCH'},
-            {'Field': 'instanceType', 'Value': ec2, 'Type': 'TERM_MATCH'}
-        ]
-        response = pricing_client.get_products(ServiceCode='AmazonEC2', Filters=filters)
-        results = response['PriceList']
-        product = json.loads(results[0])
-        od = product['terms']['OnDemand']
-        price_details = list(od.values())[0]['priceDimensions']
-        on_demand_price = list(price_details.values())[0]['pricePerUnit']['USD']
-        on_demand_price = float(on_demand_price)
-        total_on_demand_cost = total_on_demand_cost + on_demand_price
-    total_on_demand_cost = round(total_on_demand_cost, 2)
-    # get spot pricing
+    for ec2_type in fleet_types:
+        total_on_demand_cost += get_ec2_pricing(ec2_type, 'on-demand', config)
+
+    # If using spot instances get spot pricing to show savings over on-demand
     if market == 'spot':
         total_spot_cost = 0
-        for ec2 in ec2_list:
-            describe_result = ec2_client.describe_spot_price_history(
-                StartTime=datetime.utcnow(),
-                ProductDescriptions=['Linux/UNIX (Amazon VPC)'],
-                AvailabilityZone=az,
-                InstanceTypes=[ec2]
-            )
-            spot_price = float(describe_result['SpotPriceHistory'][0]['SpotPrice'])
-            total_spot_cost = total_spot_cost + spot_price
-        total_spot_cost = round(total_spot_cost, 2)
-        saving = round(100 * (1 - (total_spot_cost / total_on_demand_cost)), 2)
-        logger.info(f'Hourly price is ${total_spot_cost}. Savings of {saving}%')
+        for ec2_type in fleet_types:
+            total_spot_cost += get_ec2_pricing(ec2_type, market, config)
+        saving = 100 * (1 - (total_spot_cost / total_on_demand_cost))
+        logger.info('Hourly price is $%.2f. Savings of %.2f%%', total_spot_cost, saving)
     elif market == 'on-demand':
-        logger.info(f'Hourly price is ${total_on_demand_cost}')
+        logger.info('Hourly price is $%.2f', total_on_demand_cost)
 
 
 def create_template(n, config, task):

diff --git a/src/forge/destroy.py b/src/forge/destroy.py
@@ -8,7 +8,7 @@
 
 from . import DEFAULT_ARG_VALS, REQUIRED_ARGS
 from .parser import add_basic_args, add_general_args, add_env_args
-from .common import ec2_ip, get_regions, set_boto_session
+from .common import ec2_ip, set_boto_session, get_ec2_pricing
 
 logger = logging.getLogger(__name__)
 
@@ -44,17 +44,11 @@ def pricing(detail, config, market):
         The market the instance was created in
     """
     logger.debug('config is %s', config)
-    env = config.get('forge_env')
     profile = config.get('aws_profile')
-    az = config.get('aws_az')
     region = config.get('region')
 
     set_boto_session(region, profile)
 
-    ec2_client = boto3.client('ec2')
-    pricing_client = boto3.client('pricing', region_name='us-east-1')
-    total_spot_cost = 0
-    total_on_demand_cost = 0
     total_cost = 0
     now = datetime.now(timezone.utc)
     dif = timedelta()
@@ -65,48 +59,15 @@ def pricing(detail, config, market):
             dif = (now - launch_time)
             if dif > max_dif:
                 max_dif = dif
-            ec2 = e['instance_type']
-            if market == 'spot':
-                describe_result = ec2_client.describe_spot_price_history(
-                    StartTime=datetime.utcnow(),
-                    ProductDescriptions=['Linux/UNIX (Amazon VPC)'],
-                    AvailabilityZone=az,
-                    InstanceTypes=[ec2]
-                )
-                spot_price = float(describe_result['SpotPriceHistory'][0]['SpotPrice'])
-                total_cost = total_cost + spot_price
-                total_cost = round(total_cost, 2)
-            elif market == 'on-demand':
-                long_region = get_regions()[region]
-                op_sys = 'Linux'
-                filters = [
-                    {'Field': 'tenancy', 'Value': 'shared', 'Type': 'TERM_MATCH'},
-                    {'Field': 'operatingSystem', 'Value': op_sys, 'Type': 'TERM_MATCH'},
-                    {'Field': 'preInstalledSw', 'Value': 'NA', 'Type': 'TERM_MATCH'},
-                    {'Field': 'location', 'Value': long_region, 'Type': 'TERM_MATCH'},
-                    {'Field': 'capacitystatus', 'Value': 'Used', 'Type': 'TERM_MATCH'},
-                    {'Field': 'instanceType', 'Value': ec2, 'Type': 'TERM_MATCH'}
-                ]
-                response = pricing_client.get_products(ServiceCode='AmazonEC2', Filters=filters)
-                results = response['PriceList']
-                product = json.loads(results[0])
-                instance = (product['product']['attributes']['instanceType'])
-                od = product['terms']['OnDemand']
-                price = float(
-                    od[list(od)[0]]['priceDimensions'][list(od[list(od)[0]]['priceDimensions'])[0]]['pricePerUnit'][
-                        'USD'])
-                ip = [instance, price]
-                on_demand_price = float(ip[1])
-                total_cost = total_cost + on_demand_price
-                total_cost = round(total_cost, 2)
+            ec2_type = e['instance_type']
+            total_cost = get_ec2_pricing(ec2_type, market, config)
 
     if total_cost > 0:
         time_d_float = max_dif.total_seconds()
-        d = {"days": max_dif.days}
-        d['hours'], rem = divmod(int(max_dif.total_seconds()), 3600)
-        d["minutes"], d["seconds"] = divmod(rem, 60)
+        hours, rem = divmod(int(time_d_float), 3600)
+        minutes = rem // 60
         cost = round(total_cost * (time_d_float / 60 / 60), 2)
-        time_diff = "{hours} hours and {minutes} minutes".format(**d)
+        time_diff = f"{hours} hours and {minutes} minutes"
         logger.info('Total run time was %s. Total cost was $%s', time_diff, cost)