-
Notifications
You must be signed in to change notification settings - Fork 2.3k
ci(workflows): harden linux-gcc-cmake-odbc apt/dpkg installs with retries and diagnostics #5072
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -811,36 +811,86 @@ jobs: | |||||
| - 1433:1433 | ||||||
| steps: | ||||||
| - uses: actions/checkout@v4 | ||||||
| - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev alien libaio1 gnupg2 curl odbcinst1debian2 libodbc1 odbcinst # libmysqlclient-dev mysql-client odbc-postgresql | ||||||
| # - name: Setup MySQL ODBC connector | ||||||
| # run: | | ||||||
| # wget https://dev.mysql.com/get/Downloads/Connector-ODBC/8.2/mysql-connector-odbc_8.2.0-1ubuntu22.04_amd64.deb | ||||||
| # wget https://dev.mysql.com/get/Downloads/MySQL-8.2/mysql-community-client-plugins_8.2.0-1ubuntu22.04_amd64.deb | ||||||
| # sudo dpkg -i mysql-community-client-plugins_8.2.0-1ubuntu22.04_amd64.deb mysql-connector-odbc_8.2.0-1ubuntu22.04_amd64.deb | ||||||
|
|
||||||
| - name: Free disk space | ||||||
|
|
||||||
| - name: Pre-check runner disk & cleanup | ||||||
| run: | | ||||||
| sudo df | ||||||
| sudo apt autopurge | ||||||
| sudo apt clean | ||||||
| sudo rm -rf /usr/share/dotnet | ||||||
| sudo npm cache clean --force | ||||||
| sudo docker system prune -a --force | ||||||
| sudo df | ||||||
| - name: Setup Oracle ODBC connector | ||||||
| echo "Disk usage before cleanup:" | ||||||
| df -h . | ||||||
| free -h | ||||||
| sudo rm -f /var/lib/dpkg/lock-frontend /var/cache/apt/archives/lock || true | ||||||
| sudo apt-get -y autoremove --purge || true | ||||||
| sudo apt-get -y clean || true | ||||||
| sudo rm -rf /usr/share/dotnet || true | ||||||
| sudo npm cache clean --force || true | ||||||
| sudo docker system prune -a --force || true | ||||||
| echo "Disk usage after cleanup:" | ||||||
| df -h . | ||||||
|
|
||||||
| - name: Install basic system dependencies (robust) | ||||||
| env: | ||||||
| DEBIAN_FRONTEND: noninteractive | ||||||
| run: | | ||||||
| set -euxo pipefail | ||||||
| sudo rm -f /var/lib/dpkg/lock-frontend /var/cache/apt/archives/lock || true | ||||||
| sudo apt-get update -o Acquire::Retries=3 | ||||||
| for i in 1 2 3; do | ||||||
| sudo apt-get install -y --no-install-recommends \ | ||||||
| -o Dpkg::Options::="--force-confdef" \ | ||||||
| -o Dpkg::Options::="--force-confold" \ | ||||||
| libssl-dev unixodbc-dev alien libaio1 gnupg2 curl \ | ||||||
| odbcinst1debian2 libodbc1 odbcinst || { | ||||||
| echo "apt-get install attempt $i failed; retrying in 10s..." | ||||||
| sleep 10 | ||||||
| continue | ||||||
| } | ||||||
| break | ||||||
| done | ||||||
|
|
||||||
| - name: Setup Oracle ODBC connector (.deb) and fix deps | ||||||
| env: | ||||||
| DEBIAN_FRONTEND: noninteractive | ||||||
| run: | | ||||||
| set -euxo pipefail | ||||||
| wget https://www.devart.com/odbc/oracle/devartodbcoracle_amd64.deb | ||||||
|
||||||
| wget https://www.devart.com/odbc/oracle/devartodbcoracle_amd64.deb | |
| wget --tries=3 --timeout=30 https://www.devart.com/odbc/oracle/devartodbcoracle_amd64.deb |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Oracle ODBC dpkg command should also have explicit configuration options to avoid interactive prompts, similar to the Microsoft packages step. While || true allows it to fail, adding dpkg options would make the non-interactive intent clearer and match the hardening approach:
sudo DEBIAN_FRONTEND=noninteractive dpkg -i --force-confdef --force-confold devartodbcoracle_amd64.deb || trueThis ensures dpkg won't prompt for configuration file conflicts.
| sudo dpkg -i devartodbcoracle_amd64.deb || true | |
| sudo dpkg -i --force-confdef --force-confold devartodbcoracle_amd64.deb || true |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The curl command lacks retry logic and timeout configuration. Given the PR's goal of hardening against transient failures, consider adding retry options:
curl --retry 3 --retry-delay 5 --max-time 60 -sSL -O https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.debThis aligns with the retry philosophy applied to apt-get operations throughout the PR.
| curl -sSL -O https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.deb | |
| curl --retry 3 --retry-delay 5 --max-time 60 -sSL -O https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.deb |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing DEBIAN_FRONTEND=noninteractive environment variable on the dpkg command. While the env is set at the step level, being explicit when calling dpkg can prevent potential interactive prompts if the environment isn't properly inherited.
Consider adding it explicitly to the dpkg command:
sudo DEBIAN_FRONTEND=noninteractive dpkg -i packages-microsoft-prod.debThis is especially important since this step had the -o Dpkg::Options:: flags for confdef/confold in the system dependencies step but those options are not applicable to dpkg command directly (they're apt-get options).
| sudo dpkg -i packages-microsoft-prod.deb | |
| sudo DEBIAN_FRONTEND=noninteractive dpkg -i packages-microsoft-prod.deb |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using set -euxo pipefail with a diagnostics step that should tolerate failures is problematic. The -e flag will cause the script to exit on the first command that returns a non-zero exit status, even though all commands have || true to prevent failures. While the || true will prevent individual command failures from stopping the script, the set -e is unnecessary here since this is a diagnostic step that should never fail.
Consider using set -uxo pipefail instead (without -e) for this diagnostic step, or remove set entirely since all commands already have || true fallbacks.
| set -euxo pipefail | |
| set -uxo pipefail |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The artifact upload step uses a hardcoded path ci-apt-logs instead of $GITHUB_WORKSPACE/ci-apt-logs which is used in the diagnostic capture step. While this may work if the working directory is $GITHUB_WORKSPACE, it's inconsistent and could fail if the working directory changes.
For consistency and clarity, use the full path:
path: ${{ github.workspace }}/ci-apt-logsor ensure the path matches what's created in line 883.
| path: ci-apt-logs | |
| path: ${{ github.workspace }}/ci-apt-logs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The retry logic has a subtle bug: if the last attempt (attempt 3) fails, the script will exit with a non-zero status due to
set -e, but the loop will have already completed all iterations. This means the retry won't actually fail the step - it will silently succeed.The issue is that after the loop completes all 3 iterations without success, there's no explicit check to verify if the installation actually succeeded. Consider adding a verification after the loop: