-
Notifications
You must be signed in to change notification settings - Fork 94
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
CI: Add Infracost to GHA CI for infra cost tracking #1316
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for working on this @HarshCasper! I appreciate the comments you left as well :)
I raised a few items that we might want to discuss before merging though, let me know what you think!
36efdb9
to
b31f226
Compare
e5cff1f
to
5e96eca
Compare
a1dbf56
to
288c62f
Compare
a0af065
to
cc7af7f
Compare
405fa61
to
c74f479
Compare
💰 Infracost estimate: monthly cost will decrease by $0.57 (-1%) 📉
7 projects have no cost estimate changes. Infracost output
This comment will be updated when the cost estimate changes. Is this comment useful? Yes, No |
on: | ||
pull_request: | ||
paths: | ||
- 'qhub/template/stages/02-infrastructure' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we targetting only the 02-infrastructure
stage here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are some resources created during other stages (such as the loadbalancer), does infracost run over all the stages or only the initial 01 and 02?
It runs over all the stages, not just the initial 01 and 02. However, the workflow doesn't detect changes in the infrastructure not mentioned on the filters
. Can you just let me know about them so that I can add them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we targetting only the 02-infrastructure stage here?
It's as per the comment by @costrouc. If you want, I can direct it to target changes made on any change in the infrastructure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it's okay for me. Could you point me out to this comment as well? no need to change the config. Thanks @HarshCasper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@viniciusdc It was over a private message. @costrouc can verify that.
qhub/template/stages/01-terraform-state/aws/modules/terraform-state/main.tf
Outdated
Show resolved
Hide resolved
122cd2d
to
c74f479
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💰 Infracost estimate: monthly cost will decrease by $0.57 (-1%) 📉
Project Previous New Diff
Quansight/qhub/data/stages/01-terraform-state/aws $0.57 $0 -$0.57
All projects $73.57 $73.00 -$0.57 (-1%)
7 projects have no cost estimate changes.Infracost output
This comment will be updated when the cost estimate changes.
Thanks for all the updates @HarshCasper! This is looking real sweet 😎
Let me just verify I understand these changes too:
- the above
Infracost output
whenever we make changes (PR) to stages 01-02 (seefilter
) - and CI outputs these costs as an artifact
LGTM!
Yes! Whenever the infrastructure is changed for either cloud service provider, the Infracost PR report is generated.
Yes! It is also posted as a PR comment (for the particular cloud service provider). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @iameskild commented, this LGTM as well. If anything we can update this later if needed.
277f757
to
d65037f
Compare
Partially-resolves #1315
Changes introduced in this PR:
Types of changes
What types of changes does your PR introduce?
Put an
x
in the boxes that applyTesting
Requires testing
In case you checked yes, did you write tests?