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

build: cleanup disk and increase timeout for deploy multi-arch images #2289

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Jun 25, 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

Motivation and Context

The bulk of multi-arch images are built at the same time, take up a huge amount of space (they might reach the disk limit of the runner), and take a longer time. Adding a pre-step to do advanced cleanup and increase timeout of step

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

Bug fix, Enhancement


Description

  • Added a step to free disk space using jlumbroso/free-disk-space@main action in both deploy.yml and nightly.yml workflows.
  • Increased the timeout for building multi-arch images from 45 to 90 minutes in both deploy.yml and nightly.yml workflows.

Changes walkthrough 📝

Relevant files
Enhancement
deploy.yml
Add disk cleanup and increase timeout for deploy workflow

.github/workflows/deploy.yml

  • Added a step to free disk space before deploying Docker images.
  • Increased the timeout for building multi-arch images from 45 to 90
    minutes.
  • +6/-1     
    nightly.yml
    Add disk cleanup and increase timeout for nightly workflow

    .github/workflows/nightly.yml

  • Added a step to free disk space before nightly builds.
  • Increased the timeout for building multi-arch images from 45 to 90
    minutes.
  • +6/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    VietND96 added 3 commits June 25, 2024 14:25
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 2
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The use of jlumbroso/free-disk-space@main in both deploy.yml and nightly.yml without specifying a version could lead to potential issues if the action is updated with breaking changes. It's safer to use a specific version of a GitHub Action.
    Configuration Check:
    Ensure that the increase in timeout_minutes from 45 to 90 is justified by the expected increase in build times and that it does not unnecessarily tie up CI resources.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add a conditional check to ensure the disk space freeing action runs only on Ubuntu runners

    Consider adding a conditional check to ensure the jlumbroso/free-disk-space action runs
    only on Ubuntu runners. This will prevent potential issues if the workflow is run on a
    different OS.

    .github/workflows/deploy.yml [40-44]

     - name: Free Disk Space (Ubuntu)
    +  if: runner.os == 'Linux'
       uses: jlumbroso/free-disk-space@main
       with:
         tool-cache: false
         large-packages: false
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a conditional check for the OS is a good practice to ensure compatibility and prevent errors if the workflow is run on a different OS. The suggestion is relevant and improves the robustness of the workflow.

    7
    Add a conditional check to ensure the disk space freeing action runs only on Ubuntu runners in the nightly workflow

    Similar to the deploy workflow, consider adding a conditional check to ensure the
    jlumbroso/free-disk-space action runs only on Ubuntu runners in the nightly workflow.

    .github/workflows/nightly.yml [28-32]

     - name: Free Disk Space (Ubuntu)
    +  if: runner.os == 'Linux'
       uses: jlumbroso/free-disk-space@main
       with:
         tool-cache: false
         large-packages: false
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Similar to the first suggestion, adding a conditional OS check for the nightly workflow is beneficial for the same reasons, ensuring that the action is compatible only with the intended OS.

    7

    @VietND96 VietND96 merged commit e128fd4 into SeleniumHQ:trunk Jun 25, 2024
    14 checks passed
    @VietND96 VietND96 added this to the 4.23 milestone Jul 19, 2024
    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