-
Notifications
You must be signed in to change notification settings - Fork 1
Add Helm Overrides for AGC #23
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
Conversation
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
| "'arcConfigEndpoint' doesn't exist under 'dataplaneEndpoints' in the ARM metadata." | ||
| ) | ||
|
|
||
| # Add overrides for AGC Scenario |
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.
will it not pass cmd_helm_install.extend with both the public and azc given the public ones are above this. would it be better to do if else..
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.
The above if condition does not execute in AGC so this should be fine
| # Add overrides for AGC Scenario | ||
| if cloud_name.lower() == "ussec" or cloud_name.lower() == "usnat": | ||
| logger.debug("Adding AGC scenario overrides.") | ||
|
|
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 we refactor the override to a different method?
|
|
||
| cmd_helm_install.extend( | ||
| [ | ||
| "--set", |
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 we detect these config values at a single location?
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.
Per discussion with Harshil, this would not be feasible
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.
listing the urls would not be feasible but the logic can be moved to a common place. please correct me if not so
Currently, there is no logic to handle the required Helm overrides for AGC. We need to implement functionality that adds these overrides when the AG scenario is detected.