Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate QHub Costs via infracost #1340

Merged
merged 9 commits into from
Jun 29, 2022
Merged
2 changes: 2 additions & 0 deletions qhub/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from pydantic.error_wrappers import ValidationError

from qhub.cli.cost import create_cost_subcommand
from qhub.cli.deploy import create_deploy_subcommand
from qhub.cli.destroy import create_destroy_subcommand
from qhub.cli.initialize import create_init_subcommand
Expand Down Expand Up @@ -37,6 +38,7 @@ def cli(args):
create_support_subcommand(subparser)
create_upgrade_subcommand(subparser)
create_keycloak_subcommand(subparser)
create_cost_subcommand(subparser)

args = parser.parse_args(args)

Expand Down
15 changes: 15 additions & 0 deletions qhub/cli/cost.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import logging

from qhub.cost import infracost_report

logger = logging.getLogger(__name__)


def create_cost_subcommand(subparser):
Copy link
Member

@iameskild iameskild Jun 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a little more info to the help command regarding what the different Resource Breakdown categories means?

subparser = subparser.add_parser("cost-estimate")
subparser.add_argument("-p", "--path", help="qhub stages path", required=False)
subparser.set_defaults(func=handle_cost_report)


def handle_cost_report(args):
infracost_report(args.path)
112 changes: 112 additions & 0 deletions qhub/cost.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import json
import logging
import os
import re
import subprocess

from rich.console import Console
from rich.table import Table

logger = logging.getLogger(__name__)


def _check_infracost():
"""
Check if infracost is installed
"""
try:
subprocess.check_output(["infracost", "--version"])
return True
except subprocess.CalledProcessError:
return False


def _check_infracost_api_key():
"""
Check if infracost API key is configured
"""
try:
subprocess.check_output(["infracost", "configure", "get", "api_key"])
return True
except subprocess.CalledProcessError:
return False


def _run_infracost(path):
"""
Run infracost on the given path and return the JSON output
"""
try:
process = subprocess.Popen(
["infracost", "breakdown", "--path", path, "--format", "json"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
stdout, stderr = process.communicate()
infracost_data = json.loads(
re.search("({.+})", stdout.decode("UTF-8"))
.group(0)
.replace("u'", '"')
.replace("'", '"')
)
return infracost_data
except subprocess.CalledProcessError:
return None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a small note here about No data found?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want a comment or an error message?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we explain what went wrong so perhaps an error message.



def infracost_report(path):
"""
Generate a report of the infracost cost of the given path
args:
path: path to the qhub stages directory
"""
if not path:
path = os.path.join(os.getcwd(), "stages")

if _check_infracost() and _check_infracost_api_key():
if not os.path.exists(path):
print("Deployment is not available")
else:
data = _run_infracost(path)

cost_table = Table(title="Cost Breakdown")
cost_table.add_column("Name", justify="right", style="cyan", no_wrap=True)
cost_table.add_column("Cost", justify="right", style="cyan", no_wrap=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cost ($) perhaps?

Copy link
Member

@iameskild iameskild Jun 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or can we change currencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iameskild We can change currencies! However, do we want to prioritise the feature right now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just adding (USD) is a good solution

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, adding a way to change currencies is not a priority but let's add a label. USD as Kim suggested sounds good to me :)

@HarshCasper, would you mind just creating an issue listing a few of the enhancement requests raised in this PR and any others that you might suggest? Thanks a lot!!


cost_table.add_row("Total Monthly Cost", data["totalMonthlyCost"])
cost_table.add_row("Total Hourly Cost", data["totalHourlyCost"])

resource_table = Table(title="Resource Breakdown")
resource_table.add_column(
"Name", justify="right", style="cyan", no_wrap=True
)
resource_table.add_column(
"Number", justify="right", style="cyan", no_wrap=True
)

resource_table.add_row(
"Total Detected Costs", str(data["summary"]["totalDetectedResources"])
)
resource_table.add_row(
"Total Supported Resources",
str(data["summary"]["totalSupportedResources"]),
)
resource_table.add_row(
"Total Un-Supported Resources",
str(data["summary"]["totalUnsupportedResources"]),
)
resource_table.add_row(
"Total Non-Priced Resources",
str(data["summary"]["totalNoPriceResources"]),
)
resource_table.add_row(
"Total Usage-Priced Resources",
str(data["summary"]["totalUsageBasedResources"]),
)

console = Console()
console.print(cost_table)
console.print(resource_table)
print(f"Access the dashboard here: {data['shareUrl']}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HarshCasper when I trying this out myself, I kept running into this issue:

Traceback (most recent call last):
  File "/Users/eskild/opt/miniconda3/envs/qhub-dev/bin/qhub", line 33, in <module>
    sys.exit(load_entry_point('qhub', 'console_scripts', 'qhub')())
  File "/Users/eskild/Documents/Quansight/qhub/qhub/__main__.py", line 7, in main
    cli(sys.argv[1:])
  File "/Users/eskild/Documents/Quansight/qhub/qhub/cli/__init__.py", line 55, in cli
    args.func(args)
  File "/Users/eskild/Documents/Quansight/qhub/qhub/cli/cost.py", line 15, in handle_cost_report
    infracost_report(args.path)
  File "/Users/eskild/Documents/Quansight/qhub/qhub/cost.py", line 110, in infracost_report
    print(f"Access the dashboard here: {data['shareUrl']}")
KeyError: 'shareUrl'

Would you mind having a look?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I was trying this locally, trying to get an estimate for ~/Documents/Quansight/qhub-deployment-quansight-beta

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @iameskild 👋

I tried running it and check what the error was. Fortunately it worked for me:

       Cost Breakdown        
┏━━━━━━━━━━━━━━━━━━━━┳━━━━━━┓
┃               Name ┃ Cost ┃
┡━━━━━━━━━━━━━━━━━━━━╇━━━━━━┩
│ Total Monthly Cost │   73 │
│  Total Hourly Cost │  0.1 │
└────────────────────┴──────┘
           Resource Breakdown            
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━┓
┃                         Name ┃ Number ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━┩
│         Total Detected Costs │     11 │
│    Total Supported Resources │      3 │
│ Total Un-Supported Resources │      1 │
│   Total Non-Priced Resources │      7 │
│ Total Usage-Priced Resources │      3 │
└──────────────────────────────┴────────┘
Access the dashboard here: https://dashboard.infracost.io/share/cuy3fqeeb3kqvcufrme7c9hlekcnuxa7

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please run these two commands and see what the output is:

infracost --version
infracost configure get api_key

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @iameskild — I took a look at it and found the root cause. The JSON payload that we received did not have dashboards enabled in it, hence QHub errored out.

To enable this, we have to enable the dashboards via:

infracost configure set enable_dashboard true

Can you run this and try it out once? Since we are giving the users control over their Infracost installation, I suggest we add it out in the docs to let the users know to enable dashboards. Or we can add it as a flag that the user needs to add in case they wish to use it. What do you think? cc: @viniciusdc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update @HarshCasper! I tried it with enable_dashboard true and it worked. The dashboard link is very neat!

I think you bring up a good point about how much of the infracost setup process we should leave to the user. I'm inclined to have this tool handle the setup for the user. Handling the install with a qhub cost-estimate subcommand might be a too much to add right now but adding the flag to enable dashboards sounds like a reasonable addition.

And like you said, either way, we should document this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @iameskild, we should let the user do the config by themselves which restricts the maintenance of this in the future.

else:
print("Infracost is not installed or the API key is not configured")