Skip to content

K8s: Update template for service configs #2593

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 1 commit into from
Jan 17, 2025
Merged

K8s: Update template for service configs #2593

merged 1 commit into from
Jan 17, 2025

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Jan 17, 2025

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

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)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement, Documentation


Description

  • Added externalTrafficPolicy and sessionAffinity configurations for multiple Kubernetes services.

  • Simplified service hostnames by removing namespace suffixes in templates.

  • Updated documentation with troubleshooting steps for autoscaling issues.

  • Adjusted default tracing exporter endpoint configuration.


Changes walkthrough 📝

Relevant files
Documentation
2 files
CONFIGURATION.md
Document new traffic policy and session affinity options 
+23/-1   
README.md
Add troubleshooting section for autoscaling issues             
+16/-0   
Enhancement
17 files
selenium-grid.yaml
Simplify monitoring exporter target hostname                         
+1/-1     
chrome-node-service.yaml
Add traffic policy and session affinity to Chrome node service
+6/-0     
distributor-deployment.yaml
Simplify distributor service hostnames                                     
+3/-3     
distributor-service.yaml
Add traffic policy and session affinity to distributor service
+6/-0     
edge-node-service.yaml
Add traffic policy and session affinity to Edge node service
+6/-0     
event-bus-configmap.yaml
Simplify event bus hostname configuration                               
+1/-1     
event-bus-service.yaml
Add traffic policy and session affinity to Event Bus service
+6/-0     
firefox-node-service.yaml
Add traffic policy and session affinity to Firefox node service
+6/-0     
hub-service.yaml
Add traffic policy and session affinity to Hub service     
+6/-0     
monitoring-exporter-service.yaml
Add traffic policy and session affinity to monitoring exporter service
+6/-0     
node-configmap.yaml
Simplify node service hostnames                                                   
+3/-3     
relay-node-service.yaml
Add traffic policy and session affinity to Relay node service
+6/-0     
router-deployment.yaml
Simplify router service hostnames                                               
+3/-3     
router-service.yaml
Add traffic policy and session affinity to Router service
+6/-0     
session-map-service.yaml
Add traffic policy and session affinity to Session Map service
+6/-0     
session-queue-service.yaml
Add traffic policy and session affinity to Session Queue service
+6/-0     
values.yaml
Add configurable traffic policy and session affinity for services
+45/-1   
Tests
2 files
dummy.yaml
Simplify monitoring exporter target hostname in tests       
+1/-1     
dummy_solution.yaml
Simplify monitoring exporter target hostname in test solutions
+1/-1     

Need help?
  • Type /help how to ... in the comments thread for any question about Qodo Merge usage.
  • Check out the documentation for more information.
  • Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Service Discovery

    Removing namespace suffixes from service hostnames could affect service discovery in multi-namespace deployments. Verify that services can still communicate properly across namespaces.

      valueFrom:
        fieldRef:
          fieldPath: status.podIP
    - name: SE_DISTRIBUTOR_PORT
      value: {{ .Values.components.distributor.port | quote }}
    - name: SE_ROUTER_HOST
      value: '{{ template "seleniumGrid.router.fullname" . }}'
    - name: SE_ROUTER_PORT
      value: {{ .Values.components.router.port | quote }}
    - name: SE_SESSIONS_MAP_HOST
      value: '{{ template "seleniumGrid.sessionMap.fullname" . }}'
    - name: SE_SESSIONS_MAP_PORT
      value: {{ .Values.components.sessionMap.port | quote }}
    - name: SE_SESSION_QUEUE_HOST
      value: '{{ template "seleniumGrid.sessionQueue.fullname" . }}'
    - name: SE_SESSION_QUEUE_PORT
    Service Resolution

    The removal of namespace qualifiers from service hostnames may impact DNS resolution when services are deployed across different namespaces. Ensure proper service connectivity.

      valueFrom:
        fieldRef:
          fieldPath: status.podIP
    - name: SE_ROUTER_PORT
      value: {{ .Values.components.router.port | quote }}
    - name: SE_DISTRIBUTOR_HOST
      value: '{{ template "seleniumGrid.distributor.fullname" . }}'
    - name: SE_DISTRIBUTOR_PORT
      value: {{ .Values.components.distributor.port | quote }}
    - name: SE_SESSIONS_MAP_HOST
      value: '{{ template "seleniumGrid.sessionMap.fullname" . }}'
    - name: SE_SESSIONS_MAP_PORT
      value: {{ .Values.components.sessionMap.port | quote }}
    - name: SE_SESSION_QUEUE_HOST
      value: '{{ template "seleniumGrid.sessionQueue.fullname" . }}'
    - name: SE_SESSION_QUEUE_PORT
      value: {{ .Values.components.sessionQueue.port | quote }}

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Add validation for service session affinity configuration to prevent invalid values

    The service session affinity options should be validated to prevent invalid
    configurations. Add an enum validation to ensure only valid values (None, ClientIP,
    ClientIPString) are accepted.

    charts/selenium-grid/values.yaml [854-855]

     # -- Set session affinity to None, ClientIP or ClientIPString
    +# @enum ["", "None", "ClientIP", "ClientIPString"]
     sessionAffinity: ""
    • Apply this suggestion
    Suggestion importance[1-10]: 3

    Why: While the suggestion to add enum validation could help prevent configuration errors, it's a minor improvement to the documentation that doesn't address critical functionality or security concerns.

    3

    Copy link

    qodo-merge-pro bot commented Jan 17, 2025

    CI Failure Feedback 🧐

    (Checks updated until commit f7e662d)

    Action: Rerun workflow when failure

    Failed stage: Authenticate GitHub CLI for PR [❌]

    Failure summary:

    The GitHub Action failed because the provided authentication token (GH_CLI_TOKEN_PR) lacks the
    required 'read:org' permission scope. The GitHub CLI (gh) command attempted to authenticate but
    couldn't proceed due to insufficient permissions.

    Key points:

  • The error occurred during the gh auth login step
  • The token is missing the mandatory 'read:org' scope
  • The process exited with code 1 due to this permission issue

  • Relevant error logs:
    1:  ##[group]Operating System
    2:  Ubuntu
    ...
    
    28:  SecurityEvents: write
    29:  Statuses: write
    30:  ##[endgroup]
    31:  Secret source: Actions
    32:  Prepare workflow directory
    33:  Prepare all required actions
    34:  Getting action download info
    35:  Download action repository 'actions/checkout@main' (SHA:85e6279cec87321a52edac9c87bce653a07cf6c2)
    36:  Complete job name: Rerun workflow when failure
    ...
    
    48:  show-progress: true
    49:  lfs: false
    50:  submodules: false
    51:  set-safe-directory: true
    52:  env:
    53:  GH_CLI_TOKEN: ***
    54:  GH_CLI_TOKEN_PR: ***
    55:  RUN_ID: 12826635579
    56:  RERUN_FAILED_ONLY: true
    ...
    
    119:  ##[group]Run sudo apt update
    120:  �[36;1msudo apt update�[0m
    121:  �[36;1msudo apt install gh�[0m
    122:  shell: /usr/bin/bash -e {0}
    123:  env:
    124:  GH_CLI_TOKEN: ***
    125:  GH_CLI_TOKEN_PR: ***
    126:  RUN_ID: 12826635579
    127:  RERUN_FAILED_ONLY: true
    ...
    
    169:  0 upgraded, 0 newly installed, 0 to remove and 73 not upgraded.
    170:  ##[group]Run echo "$GH_CLI_TOKEN_PR" | gh auth login --with-token
    171:  �[36;1mecho "$GH_CLI_TOKEN_PR" | gh auth login --with-token�[0m
    172:  shell: /usr/bin/bash -e {0}
    173:  env:
    174:  GH_CLI_TOKEN: ***
    175:  GH_CLI_TOKEN_PR: ***
    176:  RUN_ID: 12826635579
    177:  RERUN_FAILED_ONLY: true
    178:  RUN_ATTEMPT: 1
    179:  ##[endgroup]
    180:  error validating token: missing required scope 'read:org'
    181:  ##[error]Process completed with exit code 1.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @VietND96 VietND96 merged commit 7a51fd2 into trunk Jan 17, 2025
    16 of 19 checks passed
    @VietND96 VietND96 deleted the update-svc branch January 17, 2025 11:16
    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.

    1 participant