[FLPATH-3323] Add AWS self-hosted/on-prem support#5918
Conversation
github-actions
Bot
added
the
smokes-required
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances Koku's capabilities by introducing self-hosted PostgreSQL support for AWS. This change allows on-premises deployments to process AWS cost and usage data without relying on Trino or Hive, providing greater flexibility and control over data infrastructure. The implementation involves new Django models for AWS line items, a generalized framework for writing and deleting data in PostgreSQL, and a suite of AWS-specific SQL queries tailored for the self-hosted environment. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively adds self-hosted PostgreSQL support for the AWS provider, aligning it with the existing OCP implementation. The changes are well-structured, including the introduction of new Django models for AWS line items, a corresponding database migration, and new PostgreSQL-compatible SQL scripts for data summarization. The refactoring of ReportParquetProcessorBase to create a generic write_to_self_hosted_table method is a significant improvement, centralizing common logic and simplifying provider-specific processors. I've identified a potential issue in one of the new SQL scripts related to tag matching that needs to be addressed.
ydayagi
force-pushed
the
trino2pgaws
branch
4 times, most recently
from
cfc4e11 to
b5c72b8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5918 +/- ##
=======================================
- Coverage 94.4% 94.4% -0.0%
=======================================
Files 362 364 +2
Lines 31988 32323 +335
Branches 3513 3526 +13
=======================================
+ Hits 30185 30501 +316
- Misses 1168 1185 +17
- Partials 635 637 +2 🚀 New features to boost your workflow:
|
ydayagi
force-pushed
the
trino2pgaws
branch
4 times, most recently
from
9cdc204 to
d66e3e6
Compare
ydayagi
added
aws-smoke-tests
dchorvat1
added
on-prem-processing
|
/retest |
1 similar comment
|
/retest |
ydayagi
force-pushed
the
trino2pgaws
branch
2 times, most recently
from
901d1a1 to
7f60de5
Compare
|
/retest |
1 similar comment
|
/retest |
|
/retest |
ydayagi
force-pushed
the
trino2pgaws
branch
2 times, most recently
from
cf14ee6 to
f46c61c
Compare
|
/retest |
ydayagi
force-pushed
the
trino2pgaws
branch
3 times, most recently
from
75b327c to
1cf3547
Compare
|
@dchorvat1 can you run our integration tests on these to confirm functionality, then move it out of draft. |
| @@ -0,0 +1,154 @@ | |||
| CREATE TABLE IF NOT EXISTS {{schema | sqlsafe}}.managed_aws_openshift_daily_temp | |||
There was a problem hiding this comment.
What is the migration strategy for these tables in the on premise flow? Cause it doesn't appear like we have one at all from my perspective.
There was a problem hiding this comment.
i will create django db models. is that ok?
|
Testing Instructions are required |
| """Return list of table names to delete from. Override in subclass if needed.""" | ||
| return [self._table_name] | ||
|
|
||
| def delete_day_postgres(self, start_date, reportnumhours=None): |
There was a problem hiding this comment.
Why are we calling this delete day when we delete the entire month?
There was a problem hiding this comment.
it is something i took from the existing trino impl. ocp deletes by day and the others dont. but they share a base class
| """Return list of table names to delete from. Override in subclass if needed.""" | ||
| return [self._table_name] | ||
|
|
||
| def delete_day_postgres(self, start_date, reportnumhours=None): |
There was a problem hiding this comment.
You pass in start_date here, but don't seem to use it anywhere.
There was a problem hiding this comment.
it is the base class. OCP impl uses it
| """Return list of table names to delete from. Override in subclass if needed.""" | ||
| return [self._table_name] | ||
|
|
||
| def delete_day_postgres(self, start_date, reportnumhours=None): |
There was a problem hiding this comment.
I highly recommend we follow the call chain for this
for csv_filename in file_list:
# set start date based on data in the file being processed:
if self.provider_type == Provider.PROVIDER_OCP:
self.start_date = self.ocp_files_to_process[csv_filename.stem]["meta_reportdatestart"]
self._delete_old_data(Path(csv_filename))
if self.provider_type == Provider.PROVIDER_OCP and self.report_type is None:
msg = "Unknown report type, skipping file processing"
LOG.warning(
log_json(
self.tracing_id,
msg=msg,
context=self.error_context,
filename=csv_filename,
)
)
return
Inside of _delete_old_data:
if settings.ONPREM:
self._delete_old_data_postgres(filename)
else:
self._delete_old_data_trino(filename)
def _delete_old_data_postgres(self, filename):
"""remove records with data older than the data in the file being processed"""
# Get reportnumhours for OCP (will be None for non-OCP)
reportnumhours = None
if self.ocp_files_to_process:
reportnumhours = int(self.ocp_files_to_process[filename.stem]["meta_reportnumhours"])
# Processor handles deleting from all relevant tables (raw and daily for OCP)
processor = self._get_report_processor(daily=False)
processor.delete_day_postgres(self.start_date, reportnumhours)
Are you deleting a whole month of data each time we process a csv?
There was a problem hiding this comment.
No — the delete runs once per manifest, not per CSV. The should_s3_parquet_be_cleared / get_s3_parquet_cleared guard in _delete_old_data ensures only the first file in a manifest triggers the delete. The manifestid-based approach deletes data from previous manifests for the same source/month, effectively replacing old data with the current manifest's data. See the guard logic at koku/masu/processor/parquet/parquet_report_processor.py lines 676-692.
i used the existing iqe tests for aws and aws on ocp. only thing i added was the ONPREM setting. please advise what other items you expect |
Did you run the test locally or did you rely on CI? I don't think CI has been set up to trigger those tests yet. |
there is a label for it and i used it in the PR. the labels changed a few times due to CI issues. I also executed the tests locally. all the aws_for/on_ocp and aws in general |
ydayagi
force-pushed
the
trino2pgaws
branch
7 times, most recently
from
62cec59 to
f5a7761
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 CI Triager — Diagnosis Check: Red Hat Konflux / koku-ci / koku Action: Re-trigger the Generated automatically. Review before applying. |
5 participants
Add self-hosted PostgreSQL support for AWS provider, following the pattern established for OCP. This enables running koku without Trino/Hive for on-premises deployments.
Changes:
Jira: https://issues.redhat.com/browse/FLPATH-3323
Jira Ticket
COST-####
Description
This change will ...
Testing
Release Notes