Skip to content
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

K8s: Add default values for multiple nodes platform and version #2543

Merged
merged 1 commit into from
Dec 28, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Dec 27, 2024

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

Scaler trigger configuration

From KEDA core v2.16.1+, the trigger metadata browserVersion, platformName is recommended to be set explicitly to have the correct scaling behavior (especially when your Grid includes autoscaling Nodes, non-autoscaling Nodes, relay Nodes, etc.). Besides that, in client binding, it is also recommended to set the browserVersion, platformName to align with the trigger metadata. Please see below examples for more details.

Understand list trigger parameters

  • url - Graphql url of your Selenium Grid. If endpoint requires authentication, you can use TriggerAuthentication to provide the credentials instead of embedding in the URL.
  • browserName - browserName should match with Node stereotype and request capability is scaled by this scaler. (Default: ``, Optional)
  • sessionBrowserName - sessionBrowserName if the browserName is different from the sessionBrowserName. (Default: ``, Optional)
  • browserVersion - browserVersion should match with Node stereotype and request capability is scaled by this scaler. (Default: ``, Optional)
  • platformName - platformName should match with Node stereotype and request capability is scaled by this scaler. (Default: ``, Optional)
  • unsafeSsl - Skip certificate validation when connecting over HTTPS. (Default: false, Optional)
  • activationThreshold - Target value for activating the scaler. Learn more about activation here. (Default: 0, Optional)
  • nodeMaxSessions - Number of maximum sessions that can run in parallel on a Node. Update this parameter align with node config --max-sessions (SE_NODE_MAX_SESSIONS) to have the correct scaling behavior. (Default: 1, Optional).

Understand list trigger authentication

  • username - Username for basic authentication in GraphQL endpoint instead of embedding in the URL. (Optional)
  • password - Password for basic authentication in GraphQL endpoint instead of embedding in the URL. (Optional)
  • authType - Type of authentication to be used. This can be set to Bearer or OAuth2 in case Selenium Grid behind an Ingress proxy with other authentication types. (Optional)
  • accessToken - Access token. This is required when authType is set a value. (Optional)

In each Node, trigger parameters value will be set under config key hpa. In template, those will be added spec of ScaledObject/ScaledJob.

In chart values, by default, browserName, sessionBrowserName are set for corresponding node browser. Parameters browserVersion, platformName are not set, leave them as empty by default. The default scaler metadata looks like

  triggers:
    - type: selenium-grid
      metadata:
        url: 'http://selenium-hub:4444/graphql'
        browserName: 'chrome'
        browserVersion: ''
        platformName: ''

In this case, the scaler will be triggered by below request (example in Python client, common use case that most users get started)

options = ChromeOptions()
driver = webdriver.Remote(options=options, command_executor=SELENIUM_GRID_URL)

With above script, the request is sent to Grid. Via GraphQL response, it looks like

{
  "data": {
    "grid": {
      "sessionCount": 0,
      "maxSession": 0,
      "totalSlots": 0
    },
    "nodesInfo": {
      "nodes": []
    },
    "sessionsInfo": {
      "sessionQueueRequests": [
        "{\"browserName\": \"chrome\"}"
      ]
    }
  }
}

Scaler will trigger to scale up the Node with stereotypes matched to pick up the request in the queue. Via GraphQL response, it looks like

{
  "data": {
    "grid": {
      "sessionCount": 0,
      "maxSession": 1,
      "totalSlots": 1
    },
    "nodesInfo": {
      "nodes": [
        {
          "id": "UUID",
          "status": "UP",
          "sessionCount": 0,
          "maxSession": 1,
          "slotCount": 1,
          "stereotypes": "[{\"slots\": 1, \"stereotype\": {\"browserName\": \"chrome\", \"browserVersion\": \"\", \"platformName\": \"\"}}]",
          "sessions": []
        }
      ]
    },
    "sessionsInfo": {
      "sessionQueueRequests": [
        "{\"browserName\": \"chrome\"}"
      ]
    }
  }
}

In Node deployment spec, there is environment variable SE_NODE_BROWSER_VERSION which is able to unset browserVersion in Node stereotypes (it is setting short browser build number by default e.g 131.0) or any custom value is up to you, which is expected to match with the request capabilities in queue and scaler trigger metadata.
Similarly, SE_NODE_PLATFORM_NAME is used to unset the platformName in Node stereotypes if needed. Noted, update to newer image tag if these 2 env variables doesn't take effect for you.

For another example, where your Grid with multiple scalers have different metadata, one of them looks like

  triggers:
    - type: selenium-grid
      metadata:
        url: 'http://selenium-hub:4444/graphql'
        browserName: 'chrome'
        browserVersion: '131.0'
        platformName: 'Linux'

The request to trigger this corresponds to the following Python script

options = ChromeOptions()
options.set_capability('platformName', 'Linux')
options.set_capability('browserVersion', '131.0')
driver = webdriver.Remote(options=options, command_executor=SELENIUM_GRID_URL)

Define multiple scalers with different trigger parameters.

When deploying the chart, you can define multiple scalers with different trigger parameters to scale up different Node stereotypes against different request capabilities.

Under config key crossBrowsers, in corresponding browser node, you can define array of item with structure same as that node, via nameOverride to set unique name for each scaler to avoid resources collision.

For example multiple-nodes-platform.yaml file, it defines 2 scalers per browser node to scale against requests with and without platformName capability.

For example multiple-nodes-platform-version.yaml file, it defines multiple scalers with platformName: 'Linux' and last few previous stable versions per browser node to scale against requests with browserVersion and platformName capabilities.

While deploying the chart, you can quickly use these extra values files by passing the file via --values flag to apply.

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

This PR enhances the Selenium Grid's platform and version handling capabilities with the following changes:

  • Added support for multiple platforms testing with a new TEST_MULTIPLE_PLATFORMS flag
  • Created new configuration file multiple-nodes-platform.yaml for platform-specific node settings
  • Updated existing multiple-nodes-platform-version.yaml with explicit platform configurations
  • Enhanced documentation with detailed examples and explanations for scaler trigger configuration
  • Modified test framework to support random platform selection during tests
  • Updated configuration descriptions to better reflect platform and version settings
  • Added comprehensive examples for different platform and version combinations in documentation

Changes walkthrough 📝

Relevant files
Configuration changes
4 files
bootstrap.sh
Update values file reference in bootstrap script                 

tests/charts/bootstrap.sh

  • Updated values file reference from cross-browsers-values.yaml to
    multiple-nodes-platform-version.yaml
  • +2/-2     
    Makefile
    Add multiple platforms flag to test targets                           

    Makefile

    • Added TEST_MULTIPLE_PLATFORMS flag to various test targets
    +5/-5     
    multiple-nodes-platform-version.yaml
    Add platform configuration for multiple nodes                       

    charts/selenium-grid/multiple-nodes-platform-version.yaml

  • Added platformName: 'Linux' to all node configurations
  • Updated file comments with example usage
  • +25/-0   
    values.yaml
    Update values file with platform configuration changes     

    charts/selenium-grid/values.yaml

  • Updated file references and descriptions for platform settings
  • Modified default platformName values and documentation
  • +19/-20 
    Enhancement
    3 files
    chart_test.sh
    Add multiple platforms testing support                                     

    tests/charts/make/chart_test.sh

  • Added TEST_MULTIPLE_PLATFORMS environment variable
  • Updated values file reference for multiple platforms testing
  • +7/-1     
    __init__.py
    Add multiple platforms support in Selenium tests                 

    tests/SeleniumTests/init.py

  • Added TEST_MULTIPLE_PLATFORMS flag
  • Added LIST_PLATFORMS constant with Linux and None options
  • Modified setUp method to support random platform selection
  • +14/-4   
    multiple-nodes-platform.yaml
    Add new platform-specific node configuration file               

    charts/selenium-grid/multiple-nodes-platform.yaml

  • Added new file for platform-specific node configurations
  • Defined configurations for Chrome, Firefox, and Edge nodes with
    platform settings
  • +39/-0   
    Documentation
    2 files
    CONFIGURATION.md
    Update configuration documentation for platform settings 

    charts/selenium-grid/CONFIGURATION.md

  • Updated documentation for browser capabilities and platform settings
  • Modified descriptions for browserName, browserVersion, and
    platformName parameters
  • +12/-12 
    README.md
    Add documentation for scaler trigger configuration             

    charts/selenium-grid/README.md

  • Added new section for scaler trigger configuration
  • Added detailed examples and explanations for platform and version
    settings
  • +123/-0 

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Configuration Error

    The Firefox node configuration uses incorrect name 'node-chrome-platform-any' instead of 'node-firefox-platform-any'

    - nameOverride: '{{ $.Release.Name }}-node-chrome-platform-any'
      hpa:
        platformName: ''
        browserVersion: ''
    Platform Handling

    The random platform selection may lead to inconsistent test behavior when some platforms are not supported by certain browser versions

    if TEST_MULTIPLE_PLATFORMS:
        platform_name = random.choice(LIST_PLATFORMS)
        if platform_name:
            options.set_capability('platformName', platform_name)

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fix incorrect node name reference in configuration file

    Fix incorrect nameOverride for Firefox node that currently uses 'chrome' instead of
    'firefox' in the name.

    charts/selenium-grid/multiple-nodes-platform.yaml [23-26]

    -- nameOverride: '{{ $.Release.Name }}-node-chrome-platform-any'
    +- nameOverride: '{{ $.Release.Name }}-node-firefox-platform-any'
       hpa:
         platformName: ''
         browserVersion: ''
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion fixes a critical configuration error where a Firefox node is incorrectly named as 'chrome', which could cause deployment and scaling issues in production.

    9

    Copy link

    qodo-merge-pro bot commented Dec 27, 2024

    CI Failure Feedback 🧐

    (Checks updated until commit 95a493c)

    Action: Rerun workflow when failure

    Failed stage: Authenticate GitHub CLI for PR [❌]

    Failure summary:

    The action failed because the GitHub authentication token lacks the required 'read:org' permission
    scope. The error occurred during the gh auth login command, which requires proper authentication
    permissions to access organizational resources.

    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:cbb722410c2e876e24abbe8de2cc27693e501dcb)
    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: 12521346858
    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: 12521346858
    127:  RERUN_FAILED_ONLY: true
    ...
    
    164:  0 upgraded, 0 newly installed, 0 to remove and 48 not upgraded.
    165:  ##[group]Run echo "$GH_CLI_TOKEN_PR" | gh auth login --with-token
    166:  �[36;1mecho "$GH_CLI_TOKEN_PR" | gh auth login --with-token�[0m
    167:  shell: /usr/bin/bash -e {0}
    168:  env:
    169:  GH_CLI_TOKEN: ***
    170:  GH_CLI_TOKEN_PR: ***
    171:  RUN_ID: 12521346858
    172:  RERUN_FAILED_ONLY: true
    173:  RUN_ATTEMPT: 1
    174:  ##[endgroup]
    175:  error validating token: missing required scope 'read:org'
    176:  ##[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 force-pushed the autoscaling-multiple-platforms branch 3 times, most recently from 1aa020c to 42a1466 Compare December 27, 2024 21:45
    @VietND96 VietND96 force-pushed the autoscaling-multiple-platforms branch from 42a1466 to 95a493c Compare December 27, 2024 22:30
    @VietND96 VietND96 merged commit 6f25d80 into trunk Dec 28, 2024
    28 of 36 checks passed
    @VietND96 VietND96 deleted the autoscaling-multiple-platforms branch December 28, 2024 00:02
    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