-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add OCI Helm registry support for agent deployments #255
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
base: main
Are you sure you want to change the base?
Conversation
Add support for deploying agents using OCI-based Helm registries (e.g., Google Artifact Registry) as an alternative to classic Helm repositories. Changes: - Add `helm_oci_registry` and `helm_chart_version` fields to AgentEnvironmentConfig - Implement auto-login to Google Artifact Registry using gcloud credentials - Add `--use-latest-chart` CLI flag to fetch the latest chart version from OCI registry - Support both classic Helm repo mode and OCI registry mode based on environment config
|
|
||
|
|
||
| def get_chart_reference( | ||
| use_oci: bool, |
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.
do we need this use_oi ? isn't this implicitly true if oci_registry is not true?
| helm_repository_name = agent_env_config.helm_repository_name | ||
| helm_repository_url = agent_env_config.helm_repository_url | ||
| # Determine if using OCI or classic helm repo mode | ||
| use_oci = agent_env_config.uses_oci_registry() if agent_env_config else False |
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.
can you make this a helper function that returns chart_reference, chart_version just so its clear that it is always set? Feel like that gets kind of lost here and can cause helm deploy issues
| description="Helm repository url for the environment", | ||
| description="Helm repository url for the environment (classic mode)" | ||
| ) | ||
| helm_oci_registry: str | None = Field( |
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.
what about making an oci_registry map and then having these as nested values? just so it looks more like an optional config?
Summary
This PR adds support for deploying agents using OCI-based Helm registries (such as Google Artifact Registry) as an alternative to classic Helm repositories.
Key Changes
helm_oci_registryandhelm_chart_versiontoAgentEnvironmentConfigfor configuring OCI-based deploymentsgcloud auth print-access-token--use-latest-chartCLI flag that queries GAR for the most recent chart tagUsage
OCI mode (new) - Set
helm_oci_registryin your environment config:Fetch latest chart version:
Classic mode (unchanged) - Continue using
helm_repository_nameandhelm_repository_urlas before.Test plan
--use-latest-chartflag fetches correct version from GAR