Skip to content

Conversation

@clincoln8
Copy link
Contributor

@clincoln8 clincoln8 commented Jan 15, 2026

Changes

  1. Separate out mixer specific overrides into website_mixer_*.yaml files and use those files to override the /mixer/deploy/helm_charts/envs/*.yaml values which override the /mixer/deploy/helm_charts/mixer/values.yaml values.

  2. Remove the redundant setValues overrides in the skaffold.yaml now that the override values are provided in the second valuesFiles array.

  3. In /deploy/helm_charts/dc_website/values.yaml, remove the unused serviceGroup workload configs, keeping the routing configs for ingrese. Changes to the mixer workload configs should be made in the website_mixer_*.yaml files.

  4. Set githash parameters in skaffold.yaml as setValueTemplates. These are overriden by the --deploy-parameters in the cloud deploy commands like in scripts/deploy_website_cloud_deploy.sh.

  5. Disable ingress for mixer in datcom-website-* deployments.

Things for reviewer to look for:

  1. That website_hash is set for every website profile in skaffold.yaml and mixer_hash is set for every mixer profile.
  2. go env by env and check:
    • for datcom-mixer-* does mixer/deploy/helm_charts/envs/mixer_*.yaml cover the deleted values in skaffold
    • for datcom-website-*
      • website: does deploy/helm_charts/envs/website_*.yaml cover the deleted values in skaffold
      • mixer: does deploy/helm_charts/envs/website_mixer_*.yaml + mixer/deploy/helm_charts/envs/mixer_*.yaml + mixers' values.yaml cover the deleted values in skaffold
  3. Are the overriden servicegroup configs the desired value in each website_mixer_*.yaml file?
  4. Are okay to disable mixer-ingress in datcom-website-* deployments?
  5. For each valueFiles skaffold that the website_mixer_*.yaml files come after the mixer_*.yaml files so that they properly override the values.

Testing

Pushed to datcom-website-dev. Replica counts reflect the changes. However noticed that the recon service group is still present but was missing the latest revision; we may need to separately remove that workload as a one-off from each datcom-website-* env.

In datcom-mixer-dev, I see that MIXER_GITHASH is propagated correctly.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @clincoln8, 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 refactors and cleans up the deployment configurations, primarily focusing on how mixer-related settings are managed within the Helm charts and Skaffold profiles. The changes aim to improve the clarity, maintainability, and consistency of deployment configurations by centralizing mixer-specific overrides, removing redundant parameters, and clarifying the purpose of various configuration files. This streamlines the deployment process and reduces potential for configuration errors.

Highlights

  • Mixer Configuration Refactoring: Mixer-specific deployment overrides have been extracted from environment-specific envs/*.yaml files into new, dedicated website_mixer_*.yaml files for better organization and clarity.
  • Skaffold.yaml Simplification: Redundant setValues entries, previously used for mixer configurations, have been removed from skaffold.yaml profiles, as these values are now sourced from the new website_mixer_*.yaml files.
  • ServiceGroup Workload Cleanup: Workload-specific configurations (like replicas, resources) for serviceGroups have been removed from deploy/helm_charts/dc_website/values.yaml, clarifying that this file is solely for routing configurations.
  • Githash Parameter Streamlining: Redundant githash parameters have been removed from skaffold.yaml and cloud deploy scripts, as these are now consistently managed via deploy-parameters.
  • Mixer Ingress Disabled: Ingress for mixer deployments has been explicitly disabled across datcom-website-* environments, aligning with the intended deployment architecture.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request does a great job of cleaning up the deployment configurations by separating mixer-specific overrides into their own files and removing redundant values from skaffold.yaml. The changes largely follow the intent described in the pull request description.

I've found a few critical issues that need to be addressed:

  • The nl server configuration has been accidentally removed or broken in the prod.yaml and staging.yaml environment files.
  • The serviceGroups configuration, which includes replica counts, is missing from the new website_mixer_prod.yaml file.
  • In skaffold.yaml, there's a duplicate setValues key for one of the profiles, and a required mixer.githash is missing for another.

I've left detailed comments with suggestions on how to fix these issues. Addressing them will ensure the deployments remain stable and correctly configured.

@clincoln8
Copy link
Contributor Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively refactors the deployment configuration by separating mixer-specific overrides into dedicated website_mixer_*.yaml files. This greatly improves the clarity and maintainability of the Skaffold and Helm configurations. The redundant setValues in skaffold.yaml have been cleaned up, and unused workload configurations have been removed from the base values.yaml. The changes align well with the goals outlined in the description.

I've found a critical indentation issue in skaffold.yaml that would break deployments, along with a few medium-severity issues related to code style and consistency. Please address these to finalize the refactoring.

@clincoln8
Copy link
Contributor Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively cleans up the deployment configuration by separating mixer-specific overrides into dedicated website_mixer_*.yaml files. It successfully removes redundant setValues from skaffold.yaml and consolidates serviceGroup workload configurations, improving clarity and maintainability. The changes also correctly disable ingress for mixer in datcom-website-* deployments and streamline githash parameter handling in deployment scripts. The ordering of valuesFiles in skaffold.yaml ensures proper overriding, as intended.

@clincoln8 clincoln8 changed the title Deployment config cleanup Streamline deployment configs Jan 15, 2026
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.

1 participant