[build-hooks] Fix wget cache skip for trafficmanager.net URLs#25505
[build-hooks] Fix wget cache skip for trafficmanager.net URLs#25505rustiqly wants to merge 1 commit intosonic-net:masterfrom
Conversation
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in the wget build hook that broke slave image builds on the 202505 branch after download URLs changed to use the trafficmanager.net domain. The fix changes how URL_PREFIX is computed to prevent legitimate downloads from being incorrectly skipped.
Changes:
- Modified
URL_PREFIXassignment in buildinfo_base.sh to use the fullPACKAGE_URL_PREFIXinstead of extracting only the hostname - Added comprehensive Copilot instructions documentation file (unrelated to the bug fix)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/sonic-build-hooks/scripts/buildinfo_base.sh | Fixed URL_PREFIX to use full path instead of hostname-only, preventing incorrect skipping of downloads from /public/ paths |
| .github/copilot-instructions.md | Added comprehensive project documentation for GitHub Copilot (unrelated to bug fix) |
|
|
||
| ``` | ||
| sonic-buildimage/ | ||
| ├── device/ # Platform-specific device configurations and plugins | ||
| ├── dockers/ # Dockerfile definitions for all SONiC containers | ||
| ├── files/ # Configuration files, scripts, and templates | ||
| ├── installer/ # ONIE installer scripts | ||
| ├── platform/ # Platform-specific build rules and configurations | ||
| ├── rules/ # Makefile rules for building individual components | ||
| ├── scripts/ # Build helper scripts | ||
| ├── sonic-slave-*/ # Build environment container definitions (per Debian version) | ||
| ├── src/ # Source code and submodules for SONiC components | ||
| ├── .azure-pipelines/ # CI/CD pipeline definitions | ||
| ├── Makefile # Top-level build entry point | ||
| └── .github/ # GitHub Actions and PR templates | ||
| ``` | ||
|
|
||
| ### Key Concepts | ||
| - **Rules system**: Each component has a `.mk` file in `rules/` defining how to build it | ||
| - **Docker containers**: SONiC services run in Docker containers defined in `dockers/` | ||
| - **Platform abstraction**: `device/` and `platform/` directories abstract hardware differences | ||
| - **Build slaves**: Builds run inside Debian-versioned containers (bookworm, bullseye, etc.) | ||
| - **Submodules**: Most SONiC components are git submodules under `src/` | ||
|
|
||
| ## Language & Style | ||
|
|
||
| - **Primary languages**: Makefile, Shell (bash), Python, Jinja2 templates | ||
| - **Makefile style**: Use tabs for indentation in Makefiles (GNU Make requirement) | ||
| - **Shell scripts**: Use `#!/bin/bash`, 4-space indentation | ||
| - **Python**: Follow PEP 8, 4-space indentation | ||
| - **Naming**: Use snake_case for variables and functions in shell/Python; UPPER_CASE for Make variables | ||
|
|
||
| ## Build Instructions | ||
|
|
||
| ```bash | ||
| # Clone with submodules | ||
| git clone --recurse-submodules https://github.com/sonic-net/sonic-buildimage.git | ||
| cd sonic-buildimage | ||
|
|
||
| # Initialize build environment | ||
| make init | ||
|
|
||
| # Configure for a specific platform | ||
| make configure PLATFORM=vs # Virtual Switch for testing | ||
| # Other platforms: broadcom, mellanox, marvell-teralynx, etc. | ||
|
|
||
| # Build the image | ||
| make SONIC_BUILD_JOBS=4 target/sonic-vs.img.gz | ||
|
|
||
| # Build specific component | ||
| make target/debs/bookworm/swss_1.0.0_amd64.deb | ||
| ``` | ||
|
|
||
| ### Build Environment Requirements | ||
| - Multiple CPU cores, 8+ GiB RAM, 300+ GiB disk | ||
| - Docker installed and running | ||
| - KVM virtualization support (for some builds) | ||
|
|
||
| ## Testing | ||
|
|
||
| - **VS (Virtual Switch)** platform is the primary testing platform | ||
| - CI runs on Azure Pipelines (`.azure-pipelines/`) | ||
| - Test images are built with `PLATFORM=vs` | ||
| - Integration tests run against VS images in sonic-mgmt repo | ||
| - Use `pytest.ini` at the root for Python test configuration | ||
|
|
||
| ## PR Guidelines | ||
|
|
||
| - **Commit format**: `[component/folder]: Description of changes` | ||
| - **Signed-off-by**: All commits MUST include `Signed-off-by: Your Name <email>` (DCO requirement) | ||
| - **CLA**: Sign the Linux Foundation EasyCLA before contributing | ||
| - **Single logical change per PR**: Isolate each commit to one component/bugfix/feature | ||
| - **Submodule updates**: When updating a submodule, reference the PR in the submodule repo | ||
| - **PR description**: Include what changed, why, and how to test | ||
|
|
||
| ## Common Patterns | ||
|
|
||
| - **Adding a new package**: Create a `.mk` file in `rules/`, add source in `src/` | ||
| - **Adding a Docker container**: Create Dockerfile in `dockers/`, add build rule in `rules/` | ||
| - **Platform support**: Add platform config in `device/<vendor>/`, build rules in `platform/` | ||
| - **Version pinning**: Dependencies are version-pinned in rules files | ||
| - **Build flags**: Use `ENABLE_*` and `INCLUDE_*` variables to toggle features | ||
|
|
||
| ## Dependencies | ||
|
|
||
| - All SONiC repos are submodules (sonic-swss, sonic-sairedis, sonic-utilities, etc.) | ||
| - Debian base system (bookworm/bullseye) | ||
| - Docker for containerized builds | ||
| - Azure Pipelines for CI/CD | ||
|
|
||
| ## Gotchas | ||
|
|
||
| - **Build times**: Full builds take 2-6 hours; use `SONIC_BUILD_JOBS` to parallelize | ||
| - **Disk space**: Builds require 100+ GiB; clean with `make clean` or `make reset` | ||
| - **Submodule versions**: Always check that submodule pins are correct before building | ||
| - **Docker cache**: Build uses Docker layer caching; `make clean` to force rebuild | ||
| - **Branch compatibility**: Component branches must match buildimage branch (e.g., master ↔ master) | ||
| - **Make variables**: Many build options are controlled by variables in `rules/config` | ||
| - **Platform differences**: Some features are platform-specific; check `rules/config` for `ENABLE_*` flags | ||
| - **Do NOT modify files in `src/` directly**: Changes should go to the respective submodule repos |
There was a problem hiding this comment.
This PR appears to contain two unrelated changes: (1) fixing the wget cache skip issue and (2) adding Copilot instructions documentation. According to SONiC PR guidelines, each PR should contain a single logical change. Consider splitting the Copilot instructions into a separate PR for better change tracking and review focus.
The URL_PREFIX variable was computed by stripping the path from PACKAGE_URL_PREFIX, keeping only the scheme and hostname (e.g. https://packages.trafficmanager.net/). This caused the wget hook's download_packages() to skip any URL matching that hostname prefix via 'continue', bypassing both the download and version tracking entirely. After commit 805e128 changed download URLs from sonicstorage.blob.core.windows.net to packages.trafficmanager.net, the download URLs (under /public/) now share the same hostname as PACKAGE_URL_PREFIX (under /packages/), triggering the skip condition. wget silently produces no output and the expected files are never created, breaking slave image builds. Fix by using the full PACKAGE_URL_PREFIX as URL_PREFIX instead of only the hostname. This ensures only URLs that are actual proxy redirects (under /packages/) are skipped, while URLs under other paths (e.g. /public/) are downloaded normally. Fixes: sonic-net#23352 Signed-off-by: Rustiqly <[email protected]>
0edae77 to
fd8692d
Compare
|
Local verification: # Old behavior — strips URL to hostname only:
PACKAGE_URL_PREFIX="https://packages.trafficmanager.net/public/packages"
echo "${PACKAGE_URL_PREFIX}" | sed -E "s#(//[^/]*/)\.*#\1#"
# Output: https://packages.trafficmanager.net/
# New behavior — uses full prefix path:
# Output: https://packages.trafficmanager.net/public/packagesThe old regex strips everything after the hostname, so |
|
Closing this PR — the root cause was addressed upstream by #24510 (merged), which replaced |
Description
The wget hook's
download_packages()function inbuildinfo_base.shcomputesURL_PREFIXby stripping the path fromPACKAGE_URL_PREFIX, keeping only the scheme and hostname (e.g.,https://packages.trafficmanager.net/). Any URL matching this prefix is skipped entirely viacontinue, bypassing both download and version tracking.After commit 805e128 changed download URLs from
sonicstorage.blob.core.windows.nettopackages.trafficmanager.net, the download URLs (under/public/) now share the same hostname asPACKAGE_URL_PREFIX(under/packages/). This triggers the skip condition — wget silently produces no output and files are never created, breaking slave image builds on the 202505 branch.Root Cause
Fix
Use the full
PACKAGE_URL_PREFIXasURL_PREFIXso only actual proxy redirect URLs (under/packages/) are skipped, while download URLs under other paths (e.g.,/public/) proceed normally.Fixes #23352