Skip to content

K8s: Bump KEDA patch version #2575

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 11, 2025
Merged

K8s: Bump KEDA patch version #2575

merged 1 commit into from
Jan 11, 2025

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Jan 10, 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, Tests


Description

  • Added support for Windows platform in Selenium Grid configurations.

  • Updated KEDA patch version and related configurations.

  • Enhanced test setup to support multiple platforms dynamically.

  • Adjusted persistent volume claim policy to Delete for better resource management.


Changes walkthrough 📝

Relevant files
Tests
chart_test.sh
Adjust local PVC handling in test script                                 

tests/charts/make/chart_test.sh

  • Commented out a kubectl delete command for local PVC.
  • Adjusted local PVC handling and directory permissions.
  • +1/-1     
    __init__.py
    Enhance multi-platform support in Selenium tests                 

    tests/SeleniumTests/init.py

  • Added windows to the list of supported platforms.
  • Updated platform name assignment dynamically in test setup.
  • Enhanced multi-platform testing capabilities.
  • +4/-4     
    Configuration changes
    Makefile
    Update KEDA and Firefox configurations                                     

    Makefile

  • Updated Firefox download URL to a newer version.
  • Adjusted KEDA-related variables to reflect the new patch version.
  • +3/-3     
    local-pvc.yaml
    Update FTP server and PVC configurations                                 

    tests/charts/ci/local-pvc.yaml

  • Reorganized and updated FTP server deployment configuration.
  • Changed persistent volume reclaim policy to Delete.
  • +36/-36 
    Enhancement
    multiple-nodes-platform.yaml
    Add Windows platform configurations to Selenium Grid         

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

  • Added Windows platform configurations for Chrome, Firefox, and Edge
    nodes.
  • Enhanced cross-browser platform support in Selenium Grid.
  • +12/-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: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Platform Compatibility

    The change from 'Linux' to lowercase 'linux' in LIST_PLATFORMS may cause compatibility issues with existing platform detection logic. Verify that platform name casing is consistent across the codebase.

    LIST_PLATFORMS = ['linux', None, 'windows']
    Resource Cleanup

    Commenting out the kubectl delete command for local PVC could lead to resource leaks if PVCs are not properly cleaned up between test runs.

    #  kubectl delete -n ${SELENIUM_NAMESPACE} -f ${LOCAL_PVC_YAML} --ignore-not-found=true

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Standardize platform name capitalization to match Selenium Grid conventions

    The platform name 'windows' in LIST_PLATFORMS should be capitalized to 'Windows' to
    maintain consistency with the platform naming convention used in the Selenium Grid
    configuration, where platform names are capitalized.

    tests/SeleniumTests/init.py [39]

    -LIST_PLATFORMS = ['linux', None, 'windows']
    +LIST_PLATFORMS = ['linux', None, 'Windows']
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies a critical inconsistency between the platform names in the test code and the Selenium Grid configuration. This mismatch could cause platform targeting issues in tests.

    8
    Ensure consistent capitalization in platform names across the codebase

    The platform name 'linux' in LIST_PLATFORMS should be capitalized to 'Linux' to
    maintain consistency with the platform naming convention used in the Selenium Grid
    configuration and other parts of the code.

    tests/SeleniumTests/init.py [39]

    -LIST_PLATFORMS = ['linux', None, 'windows']
    +LIST_PLATFORMS = ['Linux', None, 'Windows']
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion complements the first one by addressing both platform names' capitalization. The inconsistency could cause test failures as 'Linux' is used elsewhere in the code.

    8
    Possible issue
    Align platform name setting logic across all browser configurations

    The platform name setting in the first code block should be conditional on
    browser_version being present, similar to the other two code blocks, to maintain
    consistent behavior across all browser configurations.

    tests/SeleniumTests/init.py [170-172]

     if browser_version:
         options.set_capability('browserVersion', browser_version)
    -options.set_capability('platformName', LIST_PLATFORMS[0])
    +    options.set_capability('platformName', LIST_PLATFORMS[0])
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion identifies an important inconsistency in the platform name setting logic that could affect test behavior. Making it conditional on browser_version would align with other similar code blocks.

    7

    Signed-off-by: Viet Nguyen Duc <[email protected]>
    Copy link

    qodo-merge-pro bot commented Jan 10, 2025

    CI Failure Feedback 🧐

    (Checks updated until commit 151ab92)

    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.

    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: 12714124030
    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: 12714124030
    127:  RERUN_FAILED_ONLY: true
    ...
    
    173:  0 upgraded, 0 newly installed, 0 to remove and 55 not upgraded.
    174:  ##[group]Run echo "$GH_CLI_TOKEN_PR" | gh auth login --with-token
    175:  �[36;1mecho "$GH_CLI_TOKEN_PR" | gh auth login --with-token�[0m
    176:  shell: /usr/bin/bash -e {0}
    177:  env:
    178:  GH_CLI_TOKEN: ***
    179:  GH_CLI_TOKEN_PR: ***
    180:  RUN_ID: 12714124030
    181:  RERUN_FAILED_ONLY: true
    182:  RUN_ATTEMPT: 1
    183:  ##[endgroup]
    184:  error validating token: missing required scope 'read:org'
    185:  ##[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 e0c3beb into trunk Jan 11, 2025
    34 of 36 checks passed
    @VietND96 VietND96 deleted the scaler branch January 11, 2025 05:44
    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