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

feature: support GCP cloud storage #17

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

lmello
Copy link
Member

@lmello lmello commented Sep 9, 2024

This changes leverage the contribution by cklingspor, which added a storage factory and azure support to add support for gcp.

This is a work in progress and need more testing.

@lmello lmello changed the title feature: support GCP cloud storage WIP: DO NOT MERGE - feature: support GCP cloud storage Sep 9, 2024
@lmello
Copy link
Member Author

lmello commented Sep 9, 2024

I need help to test this in gcp. please let me know if you have access to it.

I will have access to a gcp environment in a project that will use this code in a few weeks, I would appreciate if anyone could help me test it out before that.

@lmello lmello force-pushed the add_support_for_gcp branch from f110851 to fa0df7b Compare September 9, 2024 22:31
@pvlkov
Copy link

pvlkov commented Oct 18, 2024

Apart from my comment above, the only thing I noticed while testing this in a GCP Kubernetes cluster was, that the Dockerfile does not produce a working container. You are not copying the src/storage directory into the final runtime-image container and the storage_factory.py file is also missing, which makes the imports fail.

COPY src/opencost_parquet_exporter.py /app/opencost_parquet_exporter.py

I would suggest replacing the copy directives to a single COPY src/ /app/. Also you might consider moving the storage_factory into the storage module as well.

With these changes I managed to get a running container in a GCP cluster which was able to query the Opencost endpoint and upload the result to a GCS bucket using a Kubernetes service account with workload identity.

I have not tested specifying OPENCOST_PARQUET_GCP_CREDENTIALS, neither have I tested using the default GOOGLE_APPLICATION_CREDENTIALS environment. I might get around to doing that at some later point but I cannot promise that.

@pvlkov
Copy link

pvlkov commented Oct 18, 2024

Update: Tested from a non-GCP cluster with OPENCOST_PARQUET_CREDENTIALS_JSON and it also works.

@lmello
Copy link
Member Author

lmello commented Nov 14, 2024

Apart from my comment above, the only thing I noticed while testing this in a GCP Kubernetes cluster was, that the Dockerfile does not produce a working container. You are not copying the src/storage directory into the final runtime-image container and the storage_factory.py file is also missing, which makes the imports fail.

COPY src/opencost_parquet_exporter.py /app/opencost_parquet_exporter.py

I would suggest replacing the copy directives to a single COPY src/ /app/. Also you might consider moving the storage_factory into the storage module as well.

With these changes I managed to get a running container in a GCP cluster which was able to query the Opencost endpoint and upload the result to a GCS bucket using a Kubernetes service account with workload identity.

I have not tested specifying OPENCOST_PARQUET_GCP_CREDENTIALS, neither have I tested using the default GOOGLE_APPLICATION_CREDENTIALS environment. I might get around to doing that at some later point but I cannot promise that.

@pvlkov thanks for testing and providing feedback. we fixed the docker error on the main branch. I am going to rebase this changes on it so the docker would be fixed.

@lmello
Copy link
Member Author

lmello commented Nov 14, 2024

r with OPENCOST_PARQUET_CREDENTIALS_JSON and it also works.

Thank you.

@lmello lmello changed the title WIP: DO NOT MERGE - feature: support GCP cloud storage feature: support GCP cloud storage Nov 14, 2024
This changes leverage the contribution by cklingspor, which added a
storage factory and azure support to add support for gcp.

This is a work in progress and need more testing.
Include the main requirements file instead of duplicating the versions
inside requirements-dev.txt
@lmello lmello force-pushed the add_support_for_gcp branch from fa0df7b to a1da04b Compare November 14, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants