Skip to content

Allow adjutment of router replica count #2600

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

Merged
merged 4 commits into from
Jan 21, 2025

Conversation

joshfng
Copy link
Contributor

@joshfng joshfng commented Jan 20, 2025

User description

Description

This enables HA for the router deployment

Motivation and Context

Allows the charts to be deployed without the router services going offline, breaking any in-flight selenium runs with a http error

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR Type

Enhancement


Description

  • Added support for configurable router replica count.

  • Updated router-deployment.yaml to use dynamic replica values.

  • Introduced a new replicas field in values.yaml.


Changes walkthrough 📝

Relevant files
Enhancement
router-deployment.yaml
Use dynamic replica count in router deployment                     

charts/selenium-grid/templates/router-deployment.yaml

  • Changed hardcoded replica count to a dynamic value.
  • Utilized .Values.components.router.replicas for replica configuration.
  • +1/-1     
    values.yaml
    Add replicas configuration in values.yaml                               

    charts/selenium-grid/values.yaml

  • Added a new replicas field under components.router.
  • Set default replica count to 1.
  • +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any question about Qodo Merge usage.
  • Check out the documentation for more information.
  • @CLAassistant
    Copy link

    CLAassistant commented Jan 20, 2025

    CLA assistant check
    All committers have signed the CLA.

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    Copy link

    qodo-merge-pro bot commented Jan 20, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Validate deployment replica count input

    Add input validation to ensure router replicas value is a positive integer. Without
    validation, setting negative or zero replicas could break the deployment.

    charts/selenium-grid/templates/router-deployment.yaml [17]

    -replicas: {{ .Values.components.router.replicas }}
    +replicas: {{ max 1 (.Values.components.router.replicas | int) }}
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion adds crucial input validation to prevent deployment failures by ensuring the replica count is always a positive integer, which is essential for maintaining service availability and preventing misconfiguration issues.

    8

    @VietND96
    Copy link
    Member

    Hi, thank you for your contribution. Do you foresee any component else that needs to do the same?

    @joshfng
    Copy link
    Contributor Author

    joshfng commented Jan 20, 2025

    Hi, thank you for your contribution. Do you foresee any component else that needs to do the same?

    I'm not familiar enough with each component to be able to say for sure, but it might not hurt to make each of them configurable.

    I can run through and change them all if you'd like

    @VietND96
    Copy link
    Member

    Yeah, go ahead with your changes. I think this is a chance to move all replicas configurable from values

    @VietND96 VietND96 merged commit e10551d into SeleniumHQ:trunk Jan 21, 2025
    19 of 26 checks passed
    @VietND96
    Copy link
    Member

    @joshfng, thank you! I added for the rest and will release a new chart

    @joshfng joshfng deleted the router-replicas branch January 21, 2025 12:26
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants