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.

Sign up for GitHub

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

Jump to bottom

Feat:(AWS) Adds support for Ephemeral Storage #2304

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Feat:(AWS) Adds support for Ephemeral Storage #2304

merged 3 commits into from
Feb 28, 2023

Conversation

robh007
Copy link
Contributor

@robh007 robh007 commented Feb 21, 2023 •
edited

Should close #2138

@robh007
Copy link
Contributor Author

robh007 commented Feb 21, 2023

@aliscott The build seems to be failing because it can't access AWS.

@aliscott aliscott self-assigned this Feb 22, 2023
@aliscott
Copy link
Member

@robh007 sorry for the delay, I've been struck by the virus. Those errors look expected. It looks like the sync usage stuff which is okay for the tests. The test failures seem to be because the usage data was updated. Did you have AWS credentials exported in your shell when running the update the tests?

@robh007
Copy link
Contributor Author

robh007 commented Feb 24, 2023 •
edited

@robh007 sorry for the delay, I've been struck by the virus. Those errors look expected. It looks like the sync usage stuff which is okay for the tests. The test failures seem to be because the usage data was updated. Did you have AWS credentials exported in your shell when running the update the tests?

Not that I'm aware of, I've ran the updates again & nothing new as generated. Do I need to do anything else?

@aliscott
Copy link
Member

@robh007 I hope you don't mind I just pushed a commit to this branch to update the tests to see if that helps. I ran the following:

unset AWS_PROFILE
unset AWS_ACCESS_KEY_ID
unset AWS_SECRET_ACCESS_KEY
make test_update_cmd
make test_update_usage

@aliscott
Copy link
Member

@robh007 the failing tests are now fixed in master, so this so pass the tests when rebased.

@robh007 robh007 requested a review from a team as a code owner February 27, 2023 10:23
aliscott
aliscott reviewed Feb 27, 2023
View reviewed changes
Copy link
Member

@aliscott aliscott left a comment

Choose a reason for hiding this comment

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

Thanks @robh007 this is awesome! I added a couple of comments. It also looks like the rebase went a bit weird.

internal/resources/aws/lambda_function.go Outdated

func (a *LambdaFunction) storageCostComponent(quantity *decimal.Decimal, storageType string) *schema.CostComponent {
return &schema.CostComponent{
Name: "Ephemeral Storage",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Name: "Ephemeral Storage",
Name: "Ephemeral storage",

internal/resources/aws/lambda_function.go Outdated
Comment on lines 97 to 99
storageGBSeconds = decimalPtr(calculateStorageGBSeconds(storageSize, *gbSeconds))

costComponents = append(costComponents, a.storageCostComponent(storageGBSeconds, storageType))
Copy link
Member

Choose a reason for hiding this comment

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

I think we can move this logic down to be in the same block that we add the duration cost component, since we're already calculating monthlyRequests and gbSeconds there.

internal/resources/aws/lambda_function.go Outdated

costComponents = append(costComponents, a.storageCostComponent(storageGBSeconds, storageType))
} else {
costComponents = append(costComponents, a.storageCostComponent(decimalPtr(decimal.NewFromInt(0)), storageType))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
costComponents = append(costComponents, a.storageCostComponent(decimalPtr(decimal.NewFromInt(0)), storageType))
costComponents = append(costComponents, a.storageCostComponent(nil, storageType))

I think this means it will show the pricing instead of a zero cost, which is what we want if there's no usage data added.

@robh007
Copy link
Contributor Author

robh007 commented Feb 27, 2023

Finally!!! Hopefully!!! @aliscott

@aliscott aliscott merged commit 5e68043 into infracost:master Feb 28, 2023
@aliscott
Copy link
Member

Thanks @robh007!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aliscott aliscott aliscott approved these changes

Assignees

@aliscott aliscott

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AWS Lambda - missing cost configurations
2 participants
@robh007 @aliscott