From c1e393b4a2745a31040bcdb436e821263dcf1bfa Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Thu, 29 Jan 2026 01:39:42 +0000 Subject: [PATCH 01/12] [Nexthop] Update FBOSS-OSS Kernel to remove Nexthop-specific references This commit updates the FBOSS-OSS Kernel files to remove Nexthop-specific references that were inadvertently included in the initial merge: - Remove sccache distributed build references from build_kernel.sh - Remove nhfboss-common.sh sourcing - Update kernel.spec copyright header to Meta/Facebook format - Fix import ordering in Python scripts to match upstream style - Simplify kernel build process to use standard gcc without sccache wrapper These changes ensure the kernel build system is clean and suitable for upstream FBOSS without Nexthop-specific dependencies. --- fboss-image/kernel/scripts/build_kernel.sh | 13 -------- fboss-image/kernel/specs/kernel.spec | 35 ++++++---------------- 2 files changed, 9 insertions(+), 39 deletions(-) diff --git a/fboss-image/kernel/scripts/build_kernel.sh b/fboss-image/kernel/scripts/build_kernel.sh index 4c5456786219a..d79dffadb2cbb 100755 --- a/fboss-image/kernel/scripts/build_kernel.sh +++ b/fboss-image/kernel/scripts/build_kernel.sh @@ -27,18 +27,6 @@ CONTAINER_SPECS_DIR="$KERNEL_ROOT/specs" CONTAINER_CONFIGS_DIR="$KERNEL_ROOT/configs" CONTAINER_SCRIPTS_DIR="$KERNEL_ROOT/scripts" -# Source common configuration for sccache distributed build and caching -# The component builder mounts fboss/oss/scripts at /fboss/oss/scripts -# shellcheck source=fboss/oss/scripts/nhfboss-common.sh -source "/fboss/oss/scripts/nhfboss-common.sh" -# Set CC to use sccache for kernel compilation -export CC="sccache gcc" -echo "Using sccache for kernel build with $COMPILE_JOBS compile jobs" - -# Debug: verify sccache config before build -sccache --stop-server 2>/dev/null || true -sccache --start-server - # Install kernel build dependencies bash "$CONTAINER_SCRIPTS_DIR/setup_kernel_build_deps.sh" @@ -72,7 +60,6 @@ rpmbuild -ba "$CONTAINER_SPECS_DIR/kernel.spec" \ exit 1 } echo "$(date) Kernel build completed successfully" -sccache -s # Copy RPMs to output directory cp -r RPMS/* "$OUT_DIR"/ 2>/dev/null diff --git a/fboss-image/kernel/specs/kernel.spec b/fboss-image/kernel/specs/kernel.spec index d34c8975727ed..754836906d6a0 100644 --- a/fboss-image/kernel/specs/kernel.spec +++ b/fboss-image/kernel/specs/kernel.spec @@ -1,5 +1,10 @@ -# Copyright 2025 Nexthop Systems Inc. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. +# # Required: Ensure kernel_version is provided %{?!kernel_version:%{error:kernel_version not provided. Provide it with --define 'kernel_version X.Y.Z'}} @@ -80,37 +85,15 @@ Requires: kernel-headers = %{epoch}:%{version}-%{release} # Build phase - creates binary artifacts for RPM %build # Build with FBOSS toolchain (using gcc-toolset-12 from container) -# Use CC from environment if set (e.g., "sccache gcc"), otherwise default to gcc # Must pass CC= on make command line because Makefile variables override env vars KERNEL_CC="${CC:-gcc}" -# When using sccache: -# - Use num_jobs for parallelism (set by nhfboss-common.sh) -# - Set reproducible build variables to allow cache hits -if [[ "$KERNEL_CC" == *sccache* ]]; then - JOBS="${num_jobs:-$(nproc)}" - export KBUILD_BUILD_TIMESTAMP="$(date -u -d "$(date +%Y-%m)-01 00:42:42")" - export KBUILD_BUILD_HOST="fboss-build" - export KBUILD_BUILD_USER="build" -else - JOBS="$(nproc)" -fi +JOBS="$(nproc)" # Build kernel and modules with correct KERNELRELEASE # This ensures uname -r returns the full version-release-arch string KERNELRELEASE=%{version}-%{release}.%{_arch} -# When using sccache distributed compilation, some files must be built locally -# first because they use a .incbin directive that references a file such as -# kernel/config_data.gz or kernel/kheaders_data.tar.xz that only exists locally -# and that sccache is not aware of. -# We use KBUILD_NOCMDDEP=1 to prevent make from rebuilding those files during -# the main build just because CC changed from "gcc" to "sccache gcc". -NOCMDDEP="" -if [[ "$KERNEL_CC" == *sccache* ]]; then - make %{?_smp_mflags} CC=gcc KERNELRELEASE=$KERNELRELEASE kernel/configs.o kernel/kheaders.o - NOCMDDEP="KBUILD_NOCMDDEP=1" -fi -make -j"$JOBS" CC="$KERNEL_CC" $NOCMDDEP KERNELRELEASE=$KERNELRELEASE bzImage modules +make -j"$JOBS" CC="$KERNEL_CC" KERNELRELEASE=$KERNELRELEASE bzImage modules # Install phase - places files for binary RPM packaging %install From 229dc06b5794c97f6a6558e9732d32705e25b497 Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Fri, 9 Jan 2026 14:14:12 +0000 Subject: [PATCH 02/12] [Nexthop] Add CMake test infrastructure for FBOSS Image Builder Integrate distro_cli Python unit tests into the FBOSS CMake build system to enable automated testing in CI/CD pipelines. - Add FbossImageDistroCliTests.cmake to discover and register Python unit tests - Configure test discovery for distro_cli modules (builder, cmds, lib, tools) - Update root CMakeLists.txt to include distro_cli test suite - Enable distro_cli tests in GitHub Actions workflow - Update distro_cli README with build and test instructions The CMake configuration automatically discovers all *_test.py files and registers them as CTest targets, allowing tests to run via 'ctest' or 'make test'. --- CMakeLists.txt | 3 + .../fbcode_builder/CMake/FBPythonBinary.cmake | 2 + cmake/FbossImageDistroCliTests.cmake | 95 +++++++++++++++++++ fboss-image/distro_cli/README.md | 56 +++++++++-- fboss-image/distro_cli/__init__.py | 8 ++ fboss-image/distro_cli/cmds/__init__.py | 8 ++ fboss-image/distro_cli/lib/__init__.py | 8 ++ fboss-image/distro_cli/tests/__init__.py | 8 ++ 8 files changed, 182 insertions(+), 6 deletions(-) create mode 100644 cmake/FbossImageDistroCliTests.cmake create mode 100644 fboss-image/distro_cli/__init__.py create mode 100644 fboss-image/distro_cli/cmds/__init__.py create mode 100644 fboss-image/distro_cli/lib/__init__.py create mode 100644 fboss-image/distro_cli/tests/__init__.py diff --git a/CMakeLists.txt b/CMakeLists.txt index ac52b2037fce1..b377bb2d1b896 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1176,3 +1176,6 @@ if (GITHUB_ACTIONS_BUILD) fsdb_all_services ) endif() + +# FBOSS Image Builder distro_cli tests +include(cmake/FbossImageDistroCliTests.cmake) diff --git a/build/fbcode_builder/CMake/FBPythonBinary.cmake b/build/fbcode_builder/CMake/FBPythonBinary.cmake index 03dbdc4969677..69e78d61e8823 100644 --- a/build/fbcode_builder/CMake/FBPythonBinary.cmake +++ b/build/fbcode_builder/CMake/FBPythonBinary.cmake @@ -158,6 +158,8 @@ function(add_fb_python_executable TARGET) # CMake doesn't really seem to like having a directory specified as an # output; specify the __main__.py file as the output instead. set(zipapp_output_file "${zipapp_output}/__main__.py") + # Update output_file to match zipapp_output_file for dir type + set(output_file "${zipapp_output_file}") list(APPEND extra_cmd_params COMMAND "${CMAKE_COMMAND}" -E remove_directory "${zipapp_output}" diff --git a/cmake/FbossImageDistroCliTests.cmake b/cmake/FbossImageDistroCliTests.cmake new file mode 100644 index 0000000000000..855999bcc1152 --- /dev/null +++ b/cmake/FbossImageDistroCliTests.cmake @@ -0,0 +1,95 @@ +# CMake to build and test fboss-image/distro_cli + +# In general, libraries and binaries in fboss/foo/bar are built by +# cmake/FooBar.cmake + +# Prevent multiple inclusions +include_guard(GLOBAL) + +# distro_cli requires Python 3.10+ with widespread use of union type syntax +# Save and temporarily override Python3_EXECUTABLE +if(DEFINED Python3_EXECUTABLE) + set(SAVED_Python3_EXECUTABLE "${Python3_EXECUTABLE}") +endif() + +# Find the highest available Python version >= 3.10 +find_package(Python3 3.10 COMPONENTS Interpreter REQUIRED) +message(STATUS "Using Python ${Python3_VERSION} (${Python3_EXECUTABLE}) for distro_cli tests") + +include(FBPythonBinary) + +file(GLOB DISTRO_CLI_TEST_SOURCES + "fboss-image/distro_cli/tests/*_test.py" +) + +# Exclude image_builder_test.py - requires source tree access for templates +# which isn't available in CMake build directory +list(FILTER DISTRO_CLI_TEST_SOURCES EXCLUDE REGEX "image_builder_test\\.py$") + +# Exclude: manual e2e tests +list(FILTER DISTRO_CLI_TEST_SOURCES EXCLUDE REGEX "kernel_build_test\\.py$") +list(FILTER DISTRO_CLI_TEST_SOURCES EXCLUDE REGEX "sai_build_test\\.py$") + +# Exclude: Docker not available +list(FILTER DISTRO_CLI_TEST_SOURCES EXCLUDE REGEX "build_entrypoint_test\\.py$") +list(FILTER DISTRO_CLI_TEST_SOURCES EXCLUDE REGEX "build_test\\.py$") +list(FILTER DISTRO_CLI_TEST_SOURCES EXCLUDE REGEX "docker_test\\.py$") + +file(GLOB DISTRO_CLI_TEST_HELPERS + "fboss-image/distro_cli/tests/test_helpers.py" +) + +file(GLOB_RECURSE DISTRO_CLI_LIB_SOURCES + "fboss-image/distro_cli/builder/*.py" + "fboss-image/distro_cli/cmds/*.py" + "fboss-image/distro_cli/lib/*.py" + "fboss-image/distro_cli/tools/*.py" +) + +# Create Python unittest executable with test data files +# Use TYPE "dir" to create a directory-based executable instead of zipapp. +# This allows tests to access data files via Path(__file__).parent, which +# doesn't work inside zip archives. +# +# Note: Only include .py files in SOURCES. Non-Python data files would be +# treated as Python modules during test discovery, causing import errors. +# Data files are copied via add_custom_command below after the build +# generates the directory structure. +add_fb_python_unittest( + distro_cli_tests + BASE_DIR "fboss-image" + TYPE "dir" + SOURCES + ${DISTRO_CLI_TEST_SOURCES} + ${DISTRO_CLI_TEST_HELPERS} + ${DISTRO_CLI_LIB_SOURCES} + ENV + "PYTHONPATH=${CMAKE_CURRENT_SOURCE_DIR}/fboss-image" +) + +# Copy test data files AFTER the build generates the directory structure +# Tests access these via Path(__file__).parent / "data" / "filename" +# With TYPE "dir", the executable is created at distro_cli_tests/ so data +# files need to be inside that directory structure. +# +# We use add_custom_command to copy AFTER the .GEN_PY_EXE target runs, +# because that target creates/recreates the distro_cli_tests/ directory. +set(DATA_DEST_DIR "${CMAKE_CURRENT_BINARY_DIR}/distro_cli_tests/distro_cli/tests") +set(DATA_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/fboss-image/distro_cli/tests/data") + +add_custom_command( + TARGET distro_cli_tests.GEN_PY_EXE + POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy_directory + "${DATA_SOURCE_DIR}" + "${DATA_DEST_DIR}/data" + COMMENT "Copying test data files for distro_cli_tests" +) + +install_fb_python_executable(distro_cli_tests) + +# Restore the original Python3_EXECUTABLE if it was set +if(DEFINED SAVED_Python3_EXECUTABLE) + set(Python3_EXECUTABLE "${SAVED_Python3_EXECUTABLE}") + unset(SAVED_Python3_EXECUTABLE) +endif() diff --git a/fboss-image/distro_cli/README.md b/fboss-image/distro_cli/README.md index c78918e93cfe4..0392c1ab83c14 100644 --- a/fboss-image/distro_cli/README.md +++ b/fboss-image/distro_cli/README.md @@ -110,18 +110,38 @@ Components are built in the following order: ### Running Tests +#### With pytest + ```bash -# Run all tests -cd fboss-image/distro_cli -python3 -m unittest discover -s tests -p '*_test.py' +# Run all tests (from fboss-image directory) +cd fboss-image +PYTHONPATH=. python3 -m pytest distro_cli/tests/ -v + +# Run specific test file +PYTHONPATH=. python3 -m pytest distro_cli/tests/cli_test.py -v + +# Run specific test class or method +PYTHONPATH=. python3 -m pytest distro_cli/tests/cli_test.py::CLITest::test_cli_creation -v +``` -# Run specific test -python3 -m unittest tests.cli_test +#### With unittest + +```bash +# Run all tests (from fboss-image directory) +cd fboss-image +PYTHONPATH=. python3 -m unittest discover -s distro_cli/tests -p '*_test.py' + +# Run specific test module +PYTHONPATH=. python3 -m unittest distro_cli.tests.cli_test + +# Run specific test class +PYTHONPATH=. python3 -m unittest distro_cli.tests.cli_test.CLITest ``` ### Linting ```bash +# With ruff (from distro_cli directory) cd fboss-image/distro_cli python3 -m ruff check . ``` @@ -152,7 +172,7 @@ fboss-image/distro_cli/ The CLI uses a custom OOP wrapper around argparse (stdlib only, no external dependencies): ```python -from lib.cli import CLI +from distro_cli.lib.cli import CLI # Create CLI cli = CLI(description='My CLI') @@ -171,3 +191,27 @@ device.add_command('ssh', ssh_func, help_text='SSH to device') # Run cli.run(setup_logging_func=setup_logging) ``` + +## Running Tests + +### Quick Tests (Default) +Run all fast unit tests (excludes long-running E2E tests): +```bash +python3 -m pytest distro_cli/tests/ -v +# or explicitly exclude e2e tests: +python3 -m pytest distro_cli/tests/ -m "not e2e" -v +``` + +### E2E Tests Only +Run only the end-to-end tests (kernel build, SAI build): +```bash +python3 -m pytest distro_cli/tests/ -m e2e -v -s +``` + +### All Tests (Including E2E) +Run everything: +```bash +python3 -m pytest distro_cli/tests/ -v -s +``` + +**Note:** E2E tests can take 10-60 minutes and require actual source files (kernel sources, SAI SDK, etc.). diff --git a/fboss-image/distro_cli/__init__.py b/fboss-image/distro_cli/__init__.py new file mode 100644 index 0000000000000..52914abdfe2b1 --- /dev/null +++ b/fboss-image/distro_cli/__init__.py @@ -0,0 +1,8 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""FBOSS Distribution CLI package.""" diff --git a/fboss-image/distro_cli/cmds/__init__.py b/fboss-image/distro_cli/cmds/__init__.py new file mode 100644 index 0000000000000..f847c6c19c631 --- /dev/null +++ b/fboss-image/distro_cli/cmds/__init__.py @@ -0,0 +1,8 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""FBOSS Distribution CLI commands package.""" diff --git a/fboss-image/distro_cli/lib/__init__.py b/fboss-image/distro_cli/lib/__init__.py new file mode 100644 index 0000000000000..668bdbadeb317 --- /dev/null +++ b/fboss-image/distro_cli/lib/__init__.py @@ -0,0 +1,8 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""FBOSS Distribution CLI library package.""" diff --git a/fboss-image/distro_cli/tests/__init__.py b/fboss-image/distro_cli/tests/__init__.py new file mode 100644 index 0000000000000..49dc3924f1a38 --- /dev/null +++ b/fboss-image/distro_cli/tests/__init__.py @@ -0,0 +1,8 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""FBOSS Distribution CLI tests package.""" From 8fe63f4bbfea8a223253704c34f18498fe8c88e5 Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Fri, 9 Jan 2026 14:21:18 +0000 Subject: [PATCH 03/12] [Nexthop] Add basic utilities for FBOSS Image Builder Provide basic tools and utiulities for the distro_cli build system. - Add custom exception classes for structured error handling - Define shared constants for Docker configuration and build settings - Implement path resolution utilities for components - Define test data fixture for image manifest validation These core utilities provide the base infrastructure required by all downstream components including Docker integration, artifact handling, and build orchestration. --- fboss-image/distro_cli/lib/constants.py | 11 ++++ fboss-image/distro_cli/lib/exceptions.py | 57 ++++++++++++++++++ fboss-image/distro_cli/lib/paths.py | 64 +++++++++++++++++++++ fboss-image/distro_cli/tests/dev_image.json | 16 +++--- 4 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 fboss-image/distro_cli/lib/constants.py create mode 100644 fboss-image/distro_cli/lib/exceptions.py create mode 100644 fboss-image/distro_cli/lib/paths.py diff --git a/fboss-image/distro_cli/lib/constants.py b/fboss-image/distro_cli/lib/constants.py new file mode 100644 index 0000000000000..c87b5d3b9c69c --- /dev/null +++ b/fboss-image/distro_cli/lib/constants.py @@ -0,0 +1,11 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Constants for FBOSS image builder.""" + +# Docker image names +FBOSS_BUILDER_IMAGE = "fboss_builder" diff --git a/fboss-image/distro_cli/lib/exceptions.py b/fboss-image/distro_cli/lib/exceptions.py new file mode 100644 index 0000000000000..d61d23efbaa38 --- /dev/null +++ b/fboss-image/distro_cli/lib/exceptions.py @@ -0,0 +1,57 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Exception definitions for FBOSS image builder.""" + + +class FbossImageError(Exception): + """Base exception for all fboss-image errors. + + All custom exceptions in the fboss-image tool should inherit from this. + This allows the top-level CLI handler to catch and format errors appropriately + before returning a non-zero exit code. + """ + + +class BuildError(FbossImageError): + """Build command failed. + + Raised when a component build fails, including: + - Build script/command not found + - Build process returned non-zero exit code + - Build output validation failed + """ + + +class ManifestError(FbossImageError): + """Manifest parsing or validation failed. + + Raised when: + - Manifest file not found or invalid JSON + - Required fields missing + - Invalid manifest structure + """ + + +class ArtifactError(FbossImageError): + """Artifact operation failed. + + Raised when: + - Expected artifact not found after build + - Artifact download failed + - Artifact cache operation failed + """ + + +class ComponentError(FbossImageError): + """Component-specific error. + + Raised when: + - Component not found in manifest + - Component configuration invalid + - Component builder not implemented + """ diff --git a/fboss-image/distro_cli/lib/paths.py b/fboss-image/distro_cli/lib/paths.py new file mode 100644 index 0000000000000..1c1a6c7f1a126 --- /dev/null +++ b/fboss-image/distro_cli/lib/paths.py @@ -0,0 +1,64 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Path resolution utilities for FBOSS image builder.""" + +from functools import lru_cache +from pathlib import Path + + +@lru_cache(maxsize=1) +def get_root_dir() -> Path: + """Find the repository root directory. + + This works by walking up from the current file until we find + the 'fboss-image' directory, then returning its parent. + + Additionally verifies that both 'fboss-image' and 'fboss' directories + exist at the root to ensure we're in the correct repository structure. + + The result is cached after the first call for performance. + + Returns: + Path to repository root directory + + Raises: + RuntimeError: If the root directory cannot be determined + """ + current = Path(__file__).resolve() + + # Walk up the directory tree looking for fboss-image + for parent in current.parents: + if (parent / "fboss-image").is_dir() and (parent / "fboss").is_dir(): + return parent + + raise RuntimeError( + f"Could not find repository root from {current}. " + f"Expected to find 'fboss-image' and 'fboss' directories in parent path." + ) + + +def get_abs_path(path_parts: str | list[str]) -> Path: + """Get absolute path by joining the parent of 'fboss-image' with provided path parts. + + Args: + path_parts: Either a string like "fboss-image/distro_cli/tools" or + a list like ["fboss-image", "distro_cli", "tools"] + + Returns: + Absolute path + + Examples: + get_abs_path("fboss-image/distro_cli/tools") + get_abs_path(["fboss-image", "distro_cli", "tools"]) + get_abs_path("fboss/oss/scripts") + """ + root = get_root_dir() + + if isinstance(path_parts, str): + return root / path_parts + return root.joinpath(*path_parts) diff --git a/fboss-image/distro_cli/tests/dev_image.json b/fboss-image/distro_cli/tests/dev_image.json index 5b1f9b0d02aef..b3717b6e98c20 100644 --- a/fboss-image/distro_cli/tests/dev_image.json +++ b/fboss-image/distro_cli/tests/dev_image.json @@ -1,27 +1,27 @@ { "distribution_formats": { - "onie": "FBOSS-k6.4.3.bin", - "usb": "FBOSS-k6.4.3.iso" + "onie": "FBOSS-k6.11.1.bin", + "usb": "FBOSS-k6.11.1iso" }, "kernel": { - "download": "https://artifact.github.com/nexthop-ai/fboss/fboss-oss_kernel_v6.4.3.tar" + "download": "https://artifact.github.com/nexthop-ai/fboss/fboss-oss_kernel_v6.11.1.tar" }, "other_dependencies": [ {"download": "https://artifact.github.com/nexthop-ai/fsdb/fsdb.rpm"}, {"download": "file:vendor_debug_tools/tools.rpm"} ], "fboss-platform-stack": { - "execute": ["fboss/fboss/oss/scripts/build.py", "platformstack"] + "execute": ["../../../../fboss/fboss/oss/scripts/build.py", "platformstack"] }, "bsps": [ - {"execute": "vendor_bsp/build.make"}, - {"execute": "fboss/oss/reference_bsp/build.py"} + {"execute": ["../../../../vendor_bsp/build.make"]}, + {"execute": ["../../../../fboss/oss/reference_bsp/build.py"]} ], "sai": { - "execute": "fboss_brcm_sai/build.sh" + "execute": ["../../../../fboss_brcm_sai/build.sh"] }, "fboss-forwarding-stack": { - "execute": ["fboss/fboss/oss/scripts/build.py", "forwardingstack"] + "execute": ["../../../../fboss/fboss/oss/scripts/build.py", "forwardingstack"] }, "image_build_hooks": { "after_pkgs": "additional_os_pkgs.xml" From df25910b86cfd8b65a532ff83fb17ddbed2288ab Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Fri, 9 Jan 2026 14:31:35 +0000 Subject: [PATCH 04/12] [Nexthop] Add Docker support for FBOSS Image Builder Add Docker container and image management capabilities. - Implement Docker container wrapper with exec and run operations - Implement Docker image wrapper with build, pull, and tag operations - Add comprehensive unit tests for container and image functionality - Include test helper utilities for unit test infrastructure - Add build_docker.sh script for Docker build automation Added unit tests for docker infrastructure and image building. --- .../distro_cli/lib/docker/container.py | 97 +++++ fboss-image/distro_cli/lib/docker/image.py | 367 ++++++++++++++++++ .../distro_cli/tests/docker_image_test.py | 93 +++++ fboss-image/distro_cli/tests/docker_test.py | 29 ++ fboss-image/distro_cli/tests/test_helpers.py | 68 ++++ fboss/oss/scripts/build_docker.sh | 7 + 6 files changed, 661 insertions(+) create mode 100644 fboss-image/distro_cli/lib/docker/container.py create mode 100644 fboss-image/distro_cli/lib/docker/image.py create mode 100644 fboss-image/distro_cli/tests/docker_image_test.py create mode 100644 fboss-image/distro_cli/tests/docker_test.py create mode 100644 fboss-image/distro_cli/tests/test_helpers.py create mode 100755 fboss/oss/scripts/build_docker.sh diff --git a/fboss-image/distro_cli/lib/docker/container.py b/fboss-image/distro_cli/lib/docker/container.py new file mode 100644 index 0000000000000..8eedac3ca2a21 --- /dev/null +++ b/fboss-image/distro_cli/lib/docker/container.py @@ -0,0 +1,97 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Docker container lifecycle management.""" + +import logging +import subprocess +from pathlib import Path + +logger = logging.getLogger(__name__) + + +def run_container( # noqa: PLR0913 + image: str, + command: list[str], + volumes: dict[Path, Path] | None = None, + env: dict[str, str] | None = None, + privileged: bool = False, + interactive: bool = False, + ephemeral: bool = True, + working_dir: str | None = None, + name: str | None = None, +) -> int: + """Run a command in a Docker container. + + Args: + image: Docker image name to run + command: Command to execute in container (as list) + volumes: Dictionary mapping host paths to container paths + env: Dictionary of environment variables + privileged: Run container in privileged mode + interactive: Run container in interactive mode (-it) + ephemeral: Remove container after it exits (--rm, default: True) + working_dir: Working directory inside container + name: Name for the container + + Returns: + Exit code from the container + + Raises: + RuntimeError: If docker command fails to start + """ + logger.info(f"Running container from image: {image}") + logger.info(f"Executing: {command}") + + # Build docker run command + cmd = ["docker", "run"] + + # Start with default flags + cmd.append("--network=host") + + # Add flags + if ephemeral: + cmd.append("--rm") + + if interactive: + cmd.extend(["-i", "-t"]) + + if privileged: + cmd.append("--privileged") + + if name: + cmd.extend(["--name", name]) + + if working_dir: + cmd.extend(["-w", working_dir]) + + if volumes: + for host_path, container_path in volumes.items(): + cmd.extend(["-v", f"{host_path}:{container_path}"]) + + if env: + for key, value in env.items(): + cmd.extend(["-e", f"{key}={value}"]) + + cmd.append(image) + + # Finally, add the actual command to run + cmd.extend(command) + + logger.debug(f"Running: {' '.join(str(c) for c in cmd)}") + + try: + result = subprocess.run( + cmd, + check=False, # Don't raise on non-zero exit + ) + logger.info(f"Container exited with code: {result.returncode}") + return result.returncode + except FileNotFoundError as e: + raise RuntimeError( + "Docker command not found. Is Docker installed and in PATH?" + ) from e diff --git a/fboss-image/distro_cli/lib/docker/image.py b/fboss-image/distro_cli/lib/docker/image.py new file mode 100644 index 0000000000000..cb746a3878a8b --- /dev/null +++ b/fboss-image/distro_cli/lib/docker/image.py @@ -0,0 +1,367 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Docker image management utilities.""" + +import hashlib +import json +import logging +import os +import subprocess +import time +from pathlib import Path + +from distro_cli.lib.constants import FBOSS_BUILDER_IMAGE +from distro_cli.lib.paths import get_abs_path + +logger = logging.getLogger(__name__) + +# Default cache expiration time in seconds (24 hours) +DEFAULT_CACHE_EXPIRATION_SECONDS = 24 * 60 * 60 + +# Container registry for caching builder images across ephemeral VMs +CONTAINER_REGISTRY = "container-registry.sw.internal.nexthop.ai" + + +def _hash_directory_tree( + directory: Path, exclude_patterns: list[str] | None = None +) -> str: + """Hash all files in a directory tree. + + Args: + directory: Directory to hash + exclude_patterns: List of patterns to exclude (e.g., '__pycache__', '.pyc') + + Returns: + SHA256 hexdigest of all files in the directory + """ + if exclude_patterns is None: + exclude_patterns = [] + + hasher = hashlib.sha256() + + # Get all files, sorted for deterministic ordering + for file_path in sorted(directory.rglob("*")): + if not file_path.is_file(): + continue + + # Skip excluded patterns + skip = False + for pattern in exclude_patterns: + if pattern in str(file_path): + skip = True + break + if skip: + continue + + # Hash the relative path (for structure changes) + rel_path = file_path.relative_to(directory) + hasher.update(str(rel_path).encode()) + + # Hash the file content + hasher.update(file_path.read_bytes()) + + return hasher.hexdigest() + + +def _compute_dependency_checksum(root_dir: Path) -> str: + """Compute checksum of Dockerfile and all its dependencies. + + This includes: + - Dockerfile + - All files in build/ directory (manifests, getdeps.py, Python modules, etc.) + + Args: + root_dir: Root directory of the repository + + Returns: + SHA256 hexdigest of all dependency files + """ + hasher = hashlib.sha256() + + # Hash Dockerfile + dockerfile = root_dir / "fboss" / "oss" / "docker" / "Dockerfile" + if not dockerfile.exists(): + raise RuntimeError(f"Dockerfile not found: {dockerfile}") + hasher.update(dockerfile.read_bytes()) + + # Hash entire build/ directory (excluding ephemeral files) + build_dir = root_dir / "build" + if not build_dir.exists(): + raise RuntimeError(f"build/ directory not found: {build_dir}") + + # Exclude Python bytecode and cache files + exclude_patterns = ["__pycache__", ".pyc", ".pyo"] + build_hash = _hash_directory_tree(build_dir, exclude_patterns) + hasher.update(build_hash.encode()) + + return hasher.hexdigest() + + +def _get_image_build_timestamp(image_tag: str) -> int | None: + """Get the build timestamp from a Docker image label. + + Args: + image_tag: Full image tag (e.g., "fboss_builder:abc123") + + Returns: + Unix timestamp when image was built, or None if not found + """ + try: + result = subprocess.run( + [ + "docker", + "image", + "inspect", + image_tag, + "--format", + "{{json .Config.Labels}}", + ], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + return None + + labels = json.loads(result.stdout.strip()) + if not labels: + return None + + timestamp_str = labels.get("build_timestamp") + if not timestamp_str: + return None + + return int(timestamp_str) + except (json.JSONDecodeError, ValueError, FileNotFoundError): + return None + + +def _try_pull_from_registry(checksum: str) -> bool: + """Try to pull the builder image from the container registry. + + Args: + checksum: The checksum tag to pull + + Returns: + True if pull succeeded and image is now available locally, False otherwise + """ + registry_tag = f"{CONTAINER_REGISTRY}/{FBOSS_BUILDER_IMAGE}:{checksum}" + local_tag = f"{FBOSS_BUILDER_IMAGE}:{checksum}" + + logger.info( + f"Trying to pull {FBOSS_BUILDER_IMAGE}:{checksum[:12]} from registry..." + ) + + try: + result = subprocess.run( + ["docker", "pull", registry_tag], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + logger.debug(f"Pull failed: {result.stderr}") + return False + + # Tag as local image + subprocess.run( + ["docker", "tag", registry_tag, local_tag], + check=True, + ) + subprocess.run( + ["docker", "tag", registry_tag, f"{FBOSS_BUILDER_IMAGE}:latest"], + check=True, + ) + + logger.info( + f"Successfully pulled {FBOSS_BUILDER_IMAGE}:{checksum[:12]} from registry" + ) + return True + + except (subprocess.CalledProcessError, FileNotFoundError) as e: + logger.debug(f"Failed to pull from registry: {e}") + return False + + +def _push_to_registry(checksum: str) -> None: + """Push the builder image to the container registry. + + Pushes fboss_builder:latest (the actual built image) with the checksum tag. + We don't push the timestamp-labeled image because that has an extra layer + and we want to cache the exact image that was built. + + Args: + checksum: The checksum tag to use in the registry + """ + # Push the :latest image (the actual build output) with the checksum tag + local_tag = f"{FBOSS_BUILDER_IMAGE}:latest" + registry_tag = f"{CONTAINER_REGISTRY}/{FBOSS_BUILDER_IMAGE}:{checksum}" + + logger.info(f"Pushing {FBOSS_BUILDER_IMAGE}:{checksum[:12]} to registry...") + + try: + # Tag for registry + subprocess.run( + ["docker", "tag", local_tag, registry_tag], + check=True, + ) + + # Push to registry + result = subprocess.run( + ["docker", "push", registry_tag], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + logger.warning(f"Failed to push to registry: {result.stderr}") + return + + logger.info( + f"Successfully pushed {FBOSS_BUILDER_IMAGE}:{checksum[:12]} to registry" + ) + + except (subprocess.CalledProcessError, FileNotFoundError) as e: + logger.warning(f"Failed to push to registry: {e}") + + +def _should_build_image(root_dir: Path) -> tuple[bool, str, str]: + """Determine if the fboss_builder image should be rebuilt. + + Args: + root_dir: Repository root directory + + Returns: + Tuple of (should_build, checksum, reason) + """ + # Get cache expiration from environment variable (in hours) + expiration_hours = int(os.getenv("FBOSS_BUILDER_CACHE_EXPIRATION_HOURS", "24")) + expiration_seconds = expiration_hours * 60 * 60 + + # Compute checksum of known dependencies + logger.debug("Computing checksum of Dockerfile dependencies...") + checksum = _compute_dependency_checksum(root_dir) + checksum_tag = f"{FBOSS_BUILDER_IMAGE}:{checksum}" + + logger.debug(f"Dockerfile checksum: {checksum[:12]}") + + # Get the image timestamp once + timestamp = _get_image_build_timestamp(checksum_tag) + + # If image doesn't exist, rebuild + if timestamp is None: + return (True, checksum, "not found") + + # Check if image is expired + current_time = int(time.time()) + age_seconds = current_time - timestamp + if age_seconds >= expiration_seconds: + return (True, checksum, f"expired (>{expiration_hours}h old)") + + return (False, checksum, "exists and is not expired") + + +def build_fboss_builder_image() -> None: + """Build the fboss_builder Docker image if needed. + + Uses a three-tier caching strategy: + 1. Local cache: Check if image with checksum tag exists locally + 2. Registry cache: Try to pull from container registry if not local + 3. Build: Build the image if not found in either cache, then push to registry + + Time-based expiration: Even if checksum matches, rebuilds if image is older + than the expiration time (default: 24 hours, configurable via + FBOSS_BUILDER_CACHE_EXPIRATION_HOURS environment variable). + + Raises: + RuntimeError: If the build script is not found or build fails + """ + + # Find paths + dockerfile = get_abs_path("fboss/oss/docker/Dockerfile") + build_script = get_abs_path("fboss/oss/scripts/build_docker.sh") + + if not dockerfile.exists(): + raise RuntimeError(f"Dockerfile not found: {dockerfile}") + + if not build_script.exists(): + raise RuntimeError(f"Build script not found: {build_script}") + + # Check if we should rebuild (checks local cache) + root_dir = get_abs_path(".") + should_build, checksum, reason = _should_build_image(root_dir) + + if not should_build: + logger.info( + f"{FBOSS_BUILDER_IMAGE} image with checksum {checksum[:12]} " + f"{reason}, skipping build" + ) + return + + # Image not in local cache - try pulling from registry + if _try_pull_from_registry(checksum): + logger.info( + f"{FBOSS_BUILDER_IMAGE} image with checksum {checksum[:12]} " + f"pulled from registry, skipping build" + ) + return + + logger.info( + f"{FBOSS_BUILDER_IMAGE} image with checksum {checksum[:12]} " + f"{reason} (not in registry either), building" + ) + + # Build the image + logger.info(f"Building {FBOSS_BUILDER_IMAGE} image...") + checksum_tag = f"{FBOSS_BUILDER_IMAGE}:{checksum}" + + try: + # Build with label containing current timestamp + current_timestamp = int(time.time()) + subprocess.run( + [str(build_script)], + check=True, + cwd=str(root_dir), + env={ + **os.environ, + "DOCKER_BUILDKIT": "1", + "BUILDKIT_PROGRESS": "plain", + }, + ) + logger.info(f"Successfully built {FBOSS_BUILDER_IMAGE} image") + + # Tag with checksum and add timestamp label + subprocess.run( + ["docker", "tag", f"{FBOSS_BUILDER_IMAGE}:latest", checksum_tag], check=True + ) + + # Add build timestamp label to the checksum-tagged image + # We do this by creating a new image with the label + subprocess.run( + [ + "docker", + "build", + "--label", + f"build_timestamp={current_timestamp}", + "--tag", + checksum_tag, + "-", + ], + input=f"FROM {FBOSS_BUILDER_IMAGE}:latest\n", + text=True, + check=True, + capture_output=True, + ) + + logger.info(f"Tagged image with checksum: {checksum[:12]}") + + # Push to registry for future builds on other VMs + _push_to_registry(checksum) + + except subprocess.CalledProcessError as e: + raise RuntimeError(f"Failed to build {FBOSS_BUILDER_IMAGE} image: {e}") from e diff --git a/fboss-image/distro_cli/tests/docker_image_test.py b/fboss-image/distro_cli/tests/docker_image_test.py new file mode 100644 index 0000000000000..c9674237a92a4 --- /dev/null +++ b/fboss-image/distro_cli/tests/docker_image_test.py @@ -0,0 +1,93 @@ +"""Tests for Docker image management utilities.""" + +import os +import unittest +from pathlib import Path +from unittest.mock import patch + +from distro_cli.lib.docker.image import _should_build_image + + +class TestShouldBuildImage(unittest.TestCase): + """Tests for _should_build_image function.""" + + def setUp(self): + """Set up test fixtures.""" + self.root_dir = Path("/fake/root") + + @patch("distro_cli.lib.docker.image._compute_dependency_checksum") + @patch("distro_cli.lib.docker.image._get_image_build_timestamp") + def test_image_not_found(self, mock_get_timestamp, mock_compute_checksum): + """Test when image doesn't exist.""" + mock_compute_checksum.return_value = "abc123" + mock_get_timestamp.return_value = None + + should_build, checksum, reason = _should_build_image(self.root_dir) + + self.assertTrue(should_build) + self.assertEqual(checksum, "abc123") + self.assertEqual(reason, "not found") + + @patch("distro_cli.lib.docker.image._compute_dependency_checksum") + @patch("distro_cli.lib.docker.image._get_image_build_timestamp") + @patch("time.time") + def test_image_expired(self, mock_time, mock_get_timestamp, mock_compute_checksum): + """Test when image exists but is expired.""" + mock_compute_checksum.return_value = "abc123" + # Image was built at timestamp 1000 + mock_get_timestamp.return_value = 1000 + # Current time is 1000 + 25 hours (more than 24 hour default expiration) + mock_time.return_value = 1000 + (25 * 60 * 60) + + # Default expiration is 24 hours + should_build, checksum, reason = _should_build_image(self.root_dir) + + self.assertTrue(should_build) + self.assertEqual(checksum, "abc123") + self.assertIn("expired", reason) + self.assertIn("24", reason) + + @patch("distro_cli.lib.docker.image._compute_dependency_checksum") + @patch("distro_cli.lib.docker.image._get_image_build_timestamp") + @patch("time.time") + def test_image_exists_and_not_expired( + self, mock_time, mock_get_timestamp, mock_compute_checksum + ): + """Test when image exists and is not expired.""" + mock_compute_checksum.return_value = "abc123" + # Image was built at timestamp 1000 + mock_get_timestamp.return_value = 1000 + # Current time is 1000 + 1 hour (less than 24 hour default expiration) + mock_time.return_value = 1000 + (1 * 60 * 60) + + should_build, checksum, reason = _should_build_image(self.root_dir) + + self.assertFalse(should_build) + self.assertEqual(checksum, "abc123") + self.assertEqual(reason, "exists and is not expired") + + @patch("distro_cli.lib.docker.image._compute_dependency_checksum") + @patch("distro_cli.lib.docker.image._get_image_build_timestamp") + @patch("time.time") + def test_custom_expiration_time( + self, mock_time, mock_get_timestamp, mock_compute_checksum + ): + """Test with custom expiration time from environment variable.""" + mock_compute_checksum.return_value = "abc123" + # Image was built at timestamp 1000 + mock_get_timestamp.return_value = 1000 + # Current time is 1000 + 50 hours (more than 48 hour custom expiration) + mock_time.return_value = 1000 + (50 * 60 * 60) + + # Set custom expiration to 48 hours + with patch.dict(os.environ, {"FBOSS_BUILDER_CACHE_EXPIRATION_HOURS": "48"}): + should_build, checksum, reason = _should_build_image(self.root_dir) + + self.assertTrue(should_build) + self.assertEqual(checksum, "abc123") + self.assertIn("expired", reason) + self.assertIn("48", reason) + + +if __name__ == "__main__": + unittest.main() diff --git a/fboss-image/distro_cli/tests/docker_test.py b/fboss-image/distro_cli/tests/docker_test.py new file mode 100644 index 0000000000000..3b713f388a322 --- /dev/null +++ b/fboss-image/distro_cli/tests/docker_test.py @@ -0,0 +1,29 @@ +"""Test Docker infrastructure.""" + +import unittest + +from distro_cli.lib.constants import FBOSS_BUILDER_IMAGE +from distro_cli.lib.docker.container import run_container +from distro_cli.tests.test_helpers import ensure_test_docker_image + + +class TestDockerInfrastructure(unittest.TestCase): + """Test Docker infrastructure basics.""" + + @classmethod + def setUpClass(cls): + """Ensure fboss_builder image exists before running tests.""" + ensure_test_docker_image() + + def test_run_simple_container(self): + """Test running a simple container command.""" + exit_code = run_container( + image=FBOSS_BUILDER_IMAGE, + command=["echo", "hello from container"], + ephemeral=True, + ) + self.assertEqual(exit_code, 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/fboss-image/distro_cli/tests/test_helpers.py b/fboss-image/distro_cli/tests/test_helpers.py new file mode 100644 index 0000000000000..ab26fb74d05b1 --- /dev/null +++ b/fboss-image/distro_cli/tests/test_helpers.py @@ -0,0 +1,68 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Test helper utilities.""" + +import logging +import os +import shutil +import tempfile +from collections.abc import Generator +from contextlib import contextmanager +from pathlib import Path + +from distro_cli.lib.docker.image import build_fboss_builder_image + +logger = logging.getLogger(__name__) + + +def ensure_test_docker_image(): + """Ensure fboss_builder Docker image is available for tests. + + Utilize the production build_fboss_builder_image() to ensure + that the fboss builder image is available. + + Raises: + RuntimeError: If image is not available. + """ + build_fboss_builder_image() + + +@contextmanager +def sandbox_tempdir(prefix: str = "test_") -> Generator[Path, None, None]: + """Create a temporary directory that works in sandboxed test environments. + + When TEST_TMPDIR environment variable is set (e.g., in Bazel sandbox), creates + the temporary directory under TEST_TMPDIR which is guaranteed to be writable. + Otherwise, uses the system's default temporary directory. + + Args: + prefix: Prefix for the temporary directory name + + Yields: + Path to the temporary directory + + Example: + with sandbox_tempdir("my_test_") as tmpdir: + test_file = tmpdir / "test.txt" + test_file.write_text("test content") + """ + # Determine the parent directory for the temp dir + parent_dir = ( + Path(os.environ["TEST_TMPDIR"]) if "TEST_TMPDIR" in os.environ else None + ) + + # Create temp directory using tempfile (works in both cases) + tmpdir_str = tempfile.mkdtemp(prefix=prefix, dir=parent_dir) + tmpdir = Path(tmpdir_str) + + try: + yield tmpdir + finally: + # Clean up + if tmpdir.exists(): + shutil.rmtree(tmpdir) diff --git a/fboss/oss/scripts/build_docker.sh b/fboss/oss/scripts/build_docker.sh new file mode 100755 index 0000000000000..5b49ddc9d714c --- /dev/null +++ b/fboss/oss/scripts/build_docker.sh @@ -0,0 +1,7 @@ +#!/bin/bash + +fboss_dir=$(realpath "$(dirname "$0")/../../..") +pushd $fboss_dir >/dev/null || exit 1 + +DOCKER_BUILDKIT=1 docker build . -t fboss_builder -f fboss/oss/docker/Dockerfile \ + --build-arg USE_CLANG=true From 3fc0743002ce9e9ec88a4395075b03011d5575cd Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Fri, 9 Jan 2026 14:35:03 +0000 Subject: [PATCH 05/12] [Nexthop] Add artifact handling for FBOSS Image Builder Add artifact storage and caching capabilities for build outputs. - Implement artifact store for managing build artifacts - Add artifact caching and retrieval functionality - Include unit tests for artifact operations - Update test helpers to support artifact testing Added unit tests for artifact storage and caching. --- fboss-image/distro_cli/lib/artifact.py | 290 +++++++++++++++ fboss-image/distro_cli/tests/artifact_test.py | 337 ++++++++++++++++++ fboss-image/distro_cli/tests/test_helpers.py | 39 +- 3 files changed, 662 insertions(+), 4 deletions(-) create mode 100644 fboss-image/distro_cli/lib/artifact.py create mode 100644 fboss-image/distro_cli/tests/artifact_test.py diff --git a/fboss-image/distro_cli/lib/artifact.py b/fboss-image/distro_cli/lib/artifact.py new file mode 100644 index 0000000000000..0bd8eeb2b70b2 --- /dev/null +++ b/fboss-image/distro_cli/lib/artifact.py @@ -0,0 +1,290 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Artifact storage with file caching for FBOSS image builder.""" + +import hashlib +import logging +import shutil +import tempfile +from collections.abc import Callable +from pathlib import Path + +from distro_cli.lib.paths import get_abs_path + +from .exceptions import ArtifactError + +logger = logging.getLogger(__name__) + + +def get_artifact_store_dir() -> Path: + """Get the default artifact store directory. + + Returns: + Path to the default artifact store directory. + Falls back to a temp directory if git repository is not available. + """ + try: + return get_abs_path("fboss-image/distro_cli/.artifacts") + except RuntimeError: + # Not in a git repository (e.g., CMake build copies code outside git) + # Use a temp directory instead + temp_base = Path(tempfile.gettempdir()) / "fboss-distro-cli-artifacts" + temp_base.mkdir(parents=True, exist_ok=True) + logger.warning( + f"Git repository not found, using temp directory for artifacts: {temp_base}" + ) + return temp_base + + +class ArtifactStore: + """Artifact storage with external cache evaluation. + + get() delegates cache evaluation and related fetching to a caller-provided function. + store() persists data and metadata files separately in storage subdirectories. + """ + + # Artifact store directory - class attribute (lazy initialization) + # Can be overridden by tests before creating ArtifactStore instances + ARTIFACT_STORE_DIR: Path | None = None + + def __init__(self): + """Initialize artifact store.""" + # Use class attribute if set, otherwise compute default + if self.ARTIFACT_STORE_DIR is None: + self.store_dir = get_artifact_store_dir() + else: + self.store_dir = self.ARTIFACT_STORE_DIR + self.store_dir.mkdir(parents=True, exist_ok=True) + logger.debug(f"Artifact store initialized at: {self.store_dir}") + + def get( + self, + store_key: str, + fetch_fn: Callable[ + [list[Path], list[Path]], tuple[bool, list[Path], list[Path]] + ], + ) -> tuple[list[Path], list[Path]]: + """Retrieve artifact files using caller-provided fetch function. + + Args: + store_key: Unique identifier for the artifact + fetch_fn: Evaluates stored files and returns (store_hit, data_files, metadata_files) + + Returns: + Tuple of (data_files, metadata_files) + """ + store_subdir = self._get_store_subdir(store_key) + stored_data_files = self._get_stored_files_in_dir(store_subdir / "data") + stored_metadata_files = self._get_stored_files_in_dir(store_subdir / "metadata") + + logger.info(f"Executing fetch function for: {store_key}") + store_hit, new_data_files, new_metadata_files = fetch_fn( + stored_data_files, stored_metadata_files + ) + + if store_hit: + logger.info(f"Store hit: {store_key}") + return (stored_data_files, stored_metadata_files) + + logger.info(f"Store miss: {store_key}, storing new files") + return self.store(store_key, new_data_files, new_metadata_files) + + def _get_store_subdir(self, store_key: str) -> Path: + """Get the storage subdirectory for a given store key. + + Args: + store_key: Store key for the artifact + + Returns: + Path to the storage subdirectory + """ + # Use full SHA256 hash to create a directory name + key_hash = hashlib.sha256(store_key.encode()).hexdigest() + return self.store_dir / key_hash + + def _get_stored_files_in_dir(self, dir_path: Path) -> list[Path]: + """Get files from a directory. + + Args: + dir_path: Directory path + + Returns: + List of file paths + """ + if not dir_path.exists(): + return [] + return [f for f in dir_path.iterdir() if f.is_file()] + + def store( + self, store_key: str, data_files: list[Path], metadata_files: list[Path] + ) -> tuple[list[Path], list[Path]]: + """Store data and metadata files in the storage. + + Files/directories are moved to store_subdir/data/ and store_subdir/metadata/. + If a path is a file, it's moved directly. + If a path is a directory, all its contents are moved. + + Args: + store_key: Store key for the artifact + data_files: List of data file/directory paths to store + metadata_files: List of metadata file/directory paths to store + + Returns: + Tuple of (stored_data_files, stored_metadata_files) + """ + store_subdir = self._get_store_subdir(store_key) + data_dir = store_subdir / "data" + metadata_dir = store_subdir / "metadata" + + # Store data files + if data_files: + # Replace any previously stored data for this key so we don't mix + # old and new artifacts (e.g., uncompressed + compressed variants). + if data_dir.exists(): + shutil.rmtree(data_dir) + data_dir.mkdir(parents=True, exist_ok=True) + for file_path in data_files: + self._move_to_dir(file_path, data_dir) + logger.info(f"Stored {len(data_files)} data file(s): {store_key}") + + # Store metadata files + if metadata_files: + # Likewise, keep metadata for this key in a clean directory so callers + # always see the current set from the latest operation. + if metadata_dir.exists(): + shutil.rmtree(metadata_dir) + metadata_dir.mkdir(parents=True, exist_ok=True) + for file_path in metadata_files: + self._move_to_dir(file_path, metadata_dir) + logger.info(f"Stored {len(metadata_files)} metadata file(s): {store_key}") + + # Return all stored files (after updating the directories) + return ( + self._get_stored_files_in_dir(data_dir), + self._get_stored_files_in_dir(metadata_dir), + ) + + def _move_to_dir(self, source: Path, dest_dir: Path) -> None: + """Move a file or directory contents to destination directory. + + Uses move instead of copy for better performance when source and dest + are on the same filesystem. + + Args: + source: Source file or directory + dest_dir: Destination directory + """ + dest_path = dest_dir / source.name + if source.is_dir(): + # For directories, move the entire tree + if dest_path.exists(): + shutil.rmtree(dest_path) + shutil.move(str(source), str(dest_path)) + else: + # For files, move directly + shutil.move(str(source), str(dest_path)) + + def invalidate(self, store_key: str) -> None: + """Remove an artifact from the store. + + Args: + store_key: Store key for the artifact to remove + """ + store_subdir = self._get_store_subdir(store_key) + if store_subdir.exists(): + shutil.rmtree(store_subdir) + logger.info(f"Invalidated store entry: {store_key}") + + def clear(self) -> None: + """Clear all stored artifacts.""" + if self.store_dir.exists(): + shutil.rmtree(self.store_dir) + self.store_dir.mkdir(parents=True, exist_ok=True) + logger.info("All stored artifacts cleared") + + @classmethod + def create_temp_dir(cls, prefix: str = "temp-") -> Path: + """Create a temporary directory within the artifact store. + + Creates temp directory on same filesystem as artifact store to enable + fast atomic moves and avoid filling up /tmp. Useful for parallel builds + that need isolation. + + This is a class method so it can be called without an instance. + + Args: + prefix: Prefix for the temporary directory name + + Returns: + Path to the created temporary directory + """ + # Get artifact store directory + store_dir = ( + get_artifact_store_dir() + if cls.ARTIFACT_STORE_DIR is None + else cls.ARTIFACT_STORE_DIR + ) + + temp_base = store_dir / ".tmp" + temp_base.mkdir(parents=True, exist_ok=True) + return Path(tempfile.mkdtemp(dir=temp_base, prefix=prefix)) + + @staticmethod + def delete_temp_dir(temp_dir: Path) -> None: + """Delete a temporary directory created by create_temp_dir(). + + This is a static method so it can be called without an instance. + + Args: + temp_dir: Path to the temporary directory to delete + """ + if temp_dir.exists(): + shutil.rmtree(temp_dir, ignore_errors=True) + logger.debug(f"Deleted temporary directory: {temp_dir}") + + +def find_artifact_in_dir( + output_dir: Path, pattern: str, component_name: str = "Component" +) -> Path: + """Find a single artifact matching a glob pattern in a directory. + + Supports both uncompressed (.tar) and zstd-compressed (.tar.zst) variants. + + Args: + output_dir: Directory to search in + pattern: Glob pattern to match (e.g., "kernel-*.rpms.tar") + component_name: Name of component for error messages + + Returns: + Path to the found artifact + + Raises: + ArtifactError: If no artifacts found + + Note: + If multiple artifacts match, returns the most recent one based on modification time. + """ + # Find both uncompressed and compressed versions + artifacts = list(output_dir.glob(pattern)) + list(output_dir.glob(f"{pattern}.zst")) + + if not artifacts: + raise ArtifactError( + f"{component_name} build output not found in: {output_dir} " + f"(patterns: {pattern}, {pattern}.zst)" + ) + + # If multiple artifacts found, use the most recent one based on modification time + if len(artifacts) > 1: + artifacts.sort(key=lambda p: p.stat().st_mtime, reverse=True) + logger.warning( + f"Multiple artifacts found matching '{pattern}' or '{pattern}.zst', " + f"using most recent: {artifacts[0]}" + ) + + logger.info(f"Found {component_name} artifact: {artifacts[0]}") + return artifacts[0] diff --git a/fboss-image/distro_cli/tests/artifact_test.py b/fboss-image/distro_cli/tests/artifact_test.py new file mode 100644 index 0000000000000..b91afd7cf7956 --- /dev/null +++ b/fboss-image/distro_cli/tests/artifact_test.py @@ -0,0 +1,337 @@ +#!/usr/bin/env python3 +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Unit tests for ArtifactStore class.""" + +import os +import shutil +import tempfile +import time +import unittest +from pathlib import Path + +from distro_cli.lib.artifact import ArtifactStore, find_artifact_in_dir +from distro_cli.lib.exceptions import ArtifactError + + +class TestArtifactStore(unittest.TestCase): + """Test ArtifactStore data/metadata separation.""" + + def setUp(self): + """Set up test fixtures.""" + self.temp_dir = Path(tempfile.mkdtemp()) + # Override class attribute for testing + self.original_store_dir = ArtifactStore.ARTIFACT_STORE_DIR + ArtifactStore.ARTIFACT_STORE_DIR = self.temp_dir / "store" + self.store = ArtifactStore() + + def tearDown(self): + """Clean up test directory.""" + # Restore original ARTIFACT_STORE_DIR + ArtifactStore.ARTIFACT_STORE_DIR = self.original_store_dir + + if self.temp_dir.exists(): + shutil.rmtree(self.temp_dir) + + def test_store_data_only(self): + """Test storing data files without metadata.""" + data_file = Path(self.temp_dir) / "data.txt" + data_file.write_text("test data") + + stored_data, stored_meta = self.store.store("test-key", [data_file], []) + + self.assertEqual(len(stored_data), 1) + self.assertEqual(len(stored_meta), 0) + self.assertTrue(stored_data[0].exists()) + self.assertTrue(stored_data[0].read_text() == "test data") + + def test_store_data_and_metadata(self): + """Test storing both data and metadata files.""" + data_file = Path(self.temp_dir) / "data.txt" + data_file.write_text("test data") + meta_file = Path(self.temp_dir) / "meta.json" + meta_file.write_text('{"etag": "abc123"}') + + stored_data, stored_meta = self.store.store( + "test-key", [data_file], [meta_file] + ) + + self.assertEqual(len(stored_data), 1) + self.assertEqual(len(stored_meta), 1) + self.assertTrue(stored_data[0].exists()) + self.assertTrue(stored_meta[0].exists()) + self.assertTrue(stored_meta[0].read_text() == '{"etag": "abc123"}') + + def test_get_store_miss(self): + """Test get() with store miss.""" + data_file = Path(self.temp_dir) / "data.txt" + data_file.write_text("fetched data") + + def fetch_fn(stored_data, stored_meta): + self.assertEqual(len(stored_data), 0) + self.assertEqual(len(stored_meta), 0) + return (False, [data_file], []) + + result_data, result_meta = self.store.get("new-key", fetch_fn) + + self.assertEqual(len(result_data), 1) + self.assertEqual(len(result_meta), 0) + self.assertTrue(result_data[0].read_text() == "fetched data") + + def test_get_store_hit(self): + """Test get() with store hit.""" + data_file = Path(self.temp_dir) / "data.txt" + data_file.write_text("original data") + self.store.store("hit-key", [data_file], []) + + def fetch_fn(stored_data, stored_meta): + self.assertEqual(len(stored_data), 1) + return (True, stored_data, stored_meta) + + result_data, _ = self.store.get("hit-key", fetch_fn) + + self.assertEqual(len(result_data), 1) + self.assertTrue(result_data[0].read_text() == "original data") + + def test_data_metadata_separation(self): + """Test that data and metadata are stored in separate subdirectories.""" + data_file = self.temp_dir / "data.txt" + data_file.write_text("data") + meta_file = self.temp_dir / "meta.json" + meta_file.write_text("metadata") + + stored_data, stored_meta = self.store.store("sep-key", [data_file], [meta_file]) + + self.assertTrue("data" in str(stored_data[0])) + self.assertTrue("metadata" in str(stored_meta[0])) + self.assertNotEqual(stored_data[0].parent, stored_meta[0].parent) + + def test_multiple_data_files(self): + """Test storing multiple data files.""" + data1 = self.temp_dir / "file1.txt" + data1.write_text("data1") + data2 = self.temp_dir / "file2.txt" + data2.write_text("data2") + + stored_data, stored_meta = self.store.store("multi-key", [data1, data2], []) + + self.assertEqual(len(stored_data), 2) + self.assertEqual(len(stored_meta), 0) + self.assertTrue(all(f.exists() for f in stored_data)) + + def test_fetch_fn_receives_stored_files(self): + """Test that fetch function receives stored files on subsequent calls.""" + data_file = self.temp_dir / "data.txt" + data_file.write_text("stored") + meta_file = self.temp_dir / "meta.json" + meta_file.write_text('{"stored": true}') + + self.store.store("fetch-key", [data_file], [meta_file]) + + received_data = [] + received_meta = [] + + def fetch_fn(stored_data, stored_meta): + received_data.extend(stored_data) + received_meta.extend(stored_meta) + return (True, stored_data, stored_meta) + + self.store.get("fetch-key", fetch_fn) + + self.assertEqual(len(received_data), 1) + self.assertEqual(len(received_meta), 1) + self.assertTrue(received_data[0].exists()) + self.assertTrue(received_meta[0].exists()) + + def test_fetch_fn_store_miss_updates_store(self): + """Test that fetch function on store miss updates the store.""" + new_data = self.temp_dir / "new.txt" + new_data.write_text("new content") + + def fetch_fn(_stored_data, _stored_meta): + return (False, [new_data], []) + + _, _ = self.store.get("update-key", fetch_fn) + + # Verify store was updated + def verify_fn(stored_data, stored_meta): + self.assertEqual(len(stored_data), 1) + return (True, stored_data, stored_meta) + + verify_data, _ = self.store.get("update-key", verify_fn) + self.assertEqual(len(verify_data), 1) + + def test_different_keys_for_compressed_and_uncompressed(self): + """Test that compressed and uncompressed artifacts use different store keys.""" + # Store uncompressed artifact + uncompressed_data = self.temp_dir / "artifact.tar" + uncompressed_data.write_text("uncompressed content") + self.store.store("component-build-abc123-uncompressed", [uncompressed_data], []) + + # Store compressed artifact with different key + compressed_data = self.temp_dir / "artifact.tar.zst" + compressed_data.write_text("compressed content") + self.store.store("component-build-abc123-compressed", [compressed_data], []) + + # Verify both are stored separately + def fetch_uncompressed(stored_data, stored_meta): + self.assertEqual(len(stored_data), 1) + self.assertIn("artifact.tar", stored_data[0].name) + return (True, stored_data, stored_meta) + + def fetch_compressed(stored_data, stored_meta): + self.assertEqual(len(stored_data), 1) + self.assertIn("artifact.tar.zst", stored_data[0].name) + return (True, stored_data, stored_meta) + + uncompressed_result, _ = self.store.get( + "component-build-abc123-uncompressed", fetch_uncompressed + ) + compressed_result, _ = self.store.get( + "component-build-abc123-compressed", fetch_compressed + ) + + # Verify they are different files + self.assertNotEqual(uncompressed_result[0], compressed_result[0]) + self.assertIn("artifact.tar", uncompressed_result[0].name) + self.assertIn("artifact.tar.zst", compressed_result[0].name) + + def test_artifact_key_salt_creates_different_cache_entries(self): + """Test that different artifact_key_salt values create separate cache entries.""" + # Simulate building the same component with different salts + artifact1 = self.temp_dir / "build1.tar" + artifact1.write_text("build with salt=uncompressed") + + artifact2 = self.temp_dir / "build2.tar" + artifact2.write_text("build with salt=compressed") + + # Store with different salts (simulating what ComponentBuilder does) + key_base = "kernel-build-12345678" + self.store.store(f"{key_base}-salt=uncompressed", [artifact1], []) + self.store.store(f"{key_base}-salt=compressed", [artifact2], []) + + # Retrieve and verify they're different + def fetch_fn1(stored_data, stored_meta): + return (True, stored_data, stored_meta) + + def fetch_fn2(stored_data, stored_meta): + return (True, stored_data, stored_meta) + + result1, _ = self.store.get(f"{key_base}-salt=uncompressed", fetch_fn1) + result2, _ = self.store.get(f"{key_base}-salt=compressed", fetch_fn2) + + # Verify different content + self.assertEqual(result1[0].read_text(), "build with salt=uncompressed") + self.assertEqual(result2[0].read_text(), "build with salt=compressed") + + +class TestFindArtifactInDir(unittest.TestCase): + """Test find_artifact_in_dir function with compression variant support.""" + + def setUp(self): + """Set up test fixtures.""" + self.temp_dir = Path(tempfile.mkdtemp()) + + def tearDown(self): + """Clean up test directory.""" + if self.temp_dir.exists(): + shutil.rmtree(self.temp_dir) + + def test_finds_uncompressed_tar(self): + """Test finding uncompressed .tar file.""" + artifact = self.temp_dir / "kernel-6.11.1.rpms.tar" + artifact.write_text("uncompressed tar") + + result = find_artifact_in_dir(self.temp_dir, "kernel-*.rpms.tar", "kernel") + + self.assertEqual(result, artifact) + + def test_finds_zstd_compressed(self): + """Test finding zstd-compressed .tar.zst file.""" + artifact = self.temp_dir / "sai-1.0.tar.zst" + artifact.write_text("zstd compressed") + + result = find_artifact_in_dir(self.temp_dir, "sai-*.tar", "sai") + + self.assertEqual(result, artifact) + + def test_uses_most_recent_when_both_exist(self): + """Test that most recent file is used when both .tar and .tar.zst exist.""" + # Test 1: Uncompressed is newer + zstd = self.temp_dir / "kernel-6.11.1.rpms.tar.zst" + zstd.write_text("zstd") + old_time = time.time() - 10 # 10 seconds ago + os.utime(zstd, (old_time, old_time)) + + uncompressed = self.temp_dir / "kernel-6.11.1.rpms.tar" + uncompressed.write_text("uncompressed") + + result = find_artifact_in_dir(self.temp_dir, "kernel-*.rpms.tar", "kernel") + self.assertEqual(result, uncompressed, "Should use newer uncompressed file") + + # Test 2: Compressed is newer + new_time = time.time() + os.utime(zstd, (new_time, new_time)) + + result = find_artifact_in_dir(self.temp_dir, "kernel-*.rpms.tar", "kernel") + self.assertEqual(result, zstd, "Should use newer compressed file") + + def test_falls_back_to_zstd_if_no_uncompressed(self): + """Test falling back to .tar.zst when .tar doesn't exist.""" + zstd = self.temp_dir / "sai-1.0.tar.zst" + zstd.write_text("zstd only") + + result = find_artifact_in_dir(self.temp_dir, "sai-*.tar", "sai") + + self.assertEqual(result, zstd) + + def test_raises_error_when_no_artifacts_found(self): + """Test that ArtifactError is raised when no artifacts found.""" + with self.assertRaises(ArtifactError) as cm: + find_artifact_in_dir(self.temp_dir, "nonexistent-*.tar.zst", "nonexistent") + + error_msg = str(cm.exception) + self.assertIn("nonexistent", error_msg) + self.assertIn("nonexistent-*.tar", error_msg) + self.assertIn("nonexistent-*.tar.zst", error_msg) + + def test_handles_multiple_matching_files(self): + """Test handling of multiple files matching the same pattern.""" + # Create multiple matching files + artifact1 = self.temp_dir / "kernel-6.11.1.rpms.tar" + artifact1.write_text("first") + artifact2 = self.temp_dir / "kernel-6.12.0.rpms.tar" + artifact2.write_text("second") + + # Should return one of them (first in glob order) + result = find_artifact_in_dir(self.temp_dir, "kernel-*.rpms.tar", "kernel") + + self.assertIn(result, [artifact1, artifact2]) + + def test_zst_pattern_input(self): + """Test when input pattern already ends with .tar.zst.""" + artifact = self.temp_dir / "component-1.0.tar.zst" + artifact.write_text("zstd") + + result = find_artifact_in_dir(self.temp_dir, "component-*.tar.zst", "component") + + self.assertEqual(result, artifact) + + def test_pattern_without_extension(self): + """Test pattern that doesn't end with .tar extension.""" + # Create a file matching the pattern + artifact = self.temp_dir / "custom-pattern.tar" + artifact.write_text("custom") + + result = find_artifact_in_dir(self.temp_dir, "custom-pattern.tar", "custom") + + self.assertEqual(result, artifact) + + +if __name__ == "__main__": + unittest.main() diff --git a/fboss-image/distro_cli/tests/test_helpers.py b/fboss-image/distro_cli/tests/test_helpers.py index ab26fb74d05b1..642fd885ee547 100644 --- a/fboss-image/distro_cli/tests/test_helpers.py +++ b/fboss-image/distro_cli/tests/test_helpers.py @@ -15,6 +15,7 @@ from contextlib import contextmanager from pathlib import Path +from distro_cli.lib.artifact import ArtifactStore from distro_cli.lib.docker.image import build_fboss_builder_image logger = logging.getLogger(__name__) @@ -33,10 +34,10 @@ def ensure_test_docker_image(): @contextmanager -def sandbox_tempdir(prefix: str = "test_") -> Generator[Path, None, None]: - """Create a temporary directory that works in sandboxed test environments. +def enter_tempdir(prefix: str = "test_") -> Generator[Path, None, None]: + """Create a temporary directory that works in both Bazel and non-Bazel test environments. - When TEST_TMPDIR environment variable is set (e.g., in Bazel sandbox), creates + When TEST_TMPDIR environment variable is set (e.g., in Bazel sandboxed tests), creates the temporary directory under TEST_TMPDIR which is guaranteed to be writable. Otherwise, uses the system's default temporary directory. @@ -47,7 +48,7 @@ def sandbox_tempdir(prefix: str = "test_") -> Generator[Path, None, None]: Path to the temporary directory Example: - with sandbox_tempdir("my_test_") as tmpdir: + with enter_tempdir("my_test_") as tmpdir: test_file = tmpdir / "test.txt" test_file.write_text("test content") """ @@ -66,3 +67,33 @@ def sandbox_tempdir(prefix: str = "test_") -> Generator[Path, None, None]: # Clean up if tmpdir.exists(): shutil.rmtree(tmpdir) + + +@contextmanager +def override_artifact_store_dir(store_dir: Path) -> Generator[None, None, None]: + """Temporarily override ArtifactStore.ARTIFACT_STORE_DIR for testing. + + This is necessary in sandboxed test environments where the default artifact + store directory may be read-only. The override is automatically restored when + the context exits. + + Args: + store_dir: Path to use as the artifact store directory + + Yields: + None + + Example: + with enter_tempdir("artifacts_") as tmpdir: + with override_artifact_store_dir(tmpdir): + # ArtifactStore will now use tmpdir + store = ArtifactStore() + # ... test code ... + # ArtifactStore.ARTIFACT_STORE_DIR is restored + """ + original = ArtifactStore.ARTIFACT_STORE_DIR + try: + ArtifactStore.ARTIFACT_STORE_DIR = store_dir + yield + finally: + ArtifactStore.ARTIFACT_STORE_DIR = original From a6648ab0e83aad1330371343185d2975710997d0 Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Fri, 9 Jan 2026 14:42:10 +0000 Subject: [PATCH 06/12] [Nexthop] Add download support for FBOSS Image Builder Add file download capabilities for fetching build dependencies. - Implement download module for HTTP/HTTPS file retrieval - Add support for progress tracking during downloads - Include comprehensive unit tests for download operations --- fboss-image/distro_cli/lib/download.py | 211 ++++++++++++++ fboss-image/distro_cli/tests/download_test.py | 268 ++++++++++++++++++ 2 files changed, 479 insertions(+) create mode 100644 fboss-image/distro_cli/lib/download.py create mode 100644 fboss-image/distro_cli/tests/download_test.py diff --git a/fboss-image/distro_cli/lib/download.py b/fboss-image/distro_cli/lib/download.py new file mode 100644 index 0000000000000..14fd684719a51 --- /dev/null +++ b/fboss-image/distro_cli/lib/download.py @@ -0,0 +1,211 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Download utilities for FBOSS image builder.""" + +import json +import logging +import shutil +import urllib.request +from http import HTTPStatus +from pathlib import Path + +from .artifact import ArtifactStore +from .exceptions import ArtifactError + +logger = logging.getLogger(__name__) + +FILE_URL_PREFIX = "file:" +HTTP_METADATA_FILENAME = ".http_metadata.json" # Stored alongside cached artifact + + +def download_artifact( + url: str, + manifest_dir: Path | None = None, + cached_data_files: list[Path] | None = None, + cached_metadata_files: list[Path] | None = None, +) -> tuple[bool, list[Path], list[Path]]: + """Download artifact from URL. + + Supports http://, https://, and file:// URLs. + HTTP(S) downloads use ETag and Last-Modified headers for conditional requests. + + For downloads, we expect exactly one data file and one metadata file. + + Args: + url: URL to download from + manifest_dir: Base directory for resolving relative file:// paths + cached_data_files: Cached data files (expects 0 or 1 file for downloads) + cached_metadata_files: Cached metadata files (expects 0 or 1 file for downloads) + + Returns: + Tuple of (cache_hit, data_files, metadata_files) + + Raises: + ArtifactError: If download fails + """ + logger.info(f"Downloading artifact from: {url}") + + if url.startswith(FILE_URL_PREFIX): + file_path = url.removeprefix(FILE_URL_PREFIX) + if manifest_dir and not Path(file_path).is_absolute(): + source_path = (manifest_dir / file_path).resolve() + else: + source_path = Path(file_path).resolve() + + if not source_path.exists(): + raise ArtifactError(f"Local file not found: {source_path}") + + # Check modification time to detect changes + current_mtime = source_path.stat().st_mtime + + # For downloads, we expect at most one metadata file with a known name + cached_mtime = None + if cached_metadata_files: + # Use the first (and only) metadata file + metadata_path = cached_metadata_files[0] + if metadata_path.name == HTTP_METADATA_FILENAME: + metadata = _load_http_metadata(metadata_path) + cached_mtime = metadata.get("mtime") + + # Check if file has been modified + if cached_mtime is not None and current_mtime == cached_mtime: + logger.info(f"Using cached file:// artifact (unchanged): {source_path}") + return (True, cached_data_files, cached_metadata_files) + + # File is new or modified - create metadata with mtime + temp_download_dir = ArtifactStore.create_temp_dir(prefix="download-") + metadata_path = temp_download_dir / HTTP_METADATA_FILENAME + metadata = {"mtime": current_mtime} + try: + with metadata_path.open("w") as f: + json.dump(metadata, f, indent=2) + except Exception as e: + logger.warning(f"Failed to save file:// metadata to {metadata_path}: {e}") + # Clean up temp dir on error + ArtifactStore.delete_temp_dir(temp_download_dir) + raise + + logger.info(f"Using local file: {source_path}") + return (False, [source_path], [metadata_path]) + + return _download_http_with_cache(url, cached_data_files, cached_metadata_files) + + +def _load_http_metadata(metadata_path: Path) -> dict: + """Load HTTP metadata from file. + + Args: + metadata_path: Path to metadata file + + Returns: + Dictionary with 'etag' and 'last_modified' keys + """ + if metadata_path: + try: + with metadata_path.open() as f: + return json.load(f) + except Exception as e: + logger.warning(f"Failed to load HTTP metadata from {metadata_path}: {e}") + return {} + + +def _save_http_metadata( + artifact_dir: Path, etag: str | None, last_modified: str | None +): + """Save HTTP metadata to file. + + Args: + artifact_dir: Directory for metadata file + etag: ETag header value + last_modified: Last-Modified header value + """ + metadata_path = artifact_dir / HTTP_METADATA_FILENAME + metadata = { + "etag": etag, + "last_modified": last_modified, + } + try: + with metadata_path.open("w") as f: + json.dump(metadata, f, indent=2) + except Exception as e: + logger.warning(f"Failed to save HTTP metadata to {metadata_path}: {e}") + + +def _download_http_with_cache( + url: str, + cached_data_files: list[Path] | None, + cached_metadata_files: list[Path] | None, +) -> tuple[bool, list[Path], list[Path]]: + """Download from HTTP/HTTPS with caching support using ETag and Last-Modified headers. + + For downloads, we expect exactly one metadata file. + + Args: + url: HTTP or HTTPS URL + cached_data_files: Cached data files (expects 0 or 1 file) + cached_metadata_files: Cached metadata files (expects 0 or 1 file) + + Returns: + Tuple of (cache_hit, data_files, metadata_files) + + Raises: + ArtifactError: If download fails + """ + # For downloads, expect at most one metadata file with a known name + http_metadata = {} + if cached_metadata_files: + # Construct static path using known filename instead of searching + metadata_dir = cached_metadata_files[0].parent + metadata_path = metadata_dir / HTTP_METADATA_FILENAME + if metadata_path.exists(): + http_metadata = _load_http_metadata(metadata_path) + + request = urllib.request.Request(url) + if http_metadata: + if http_metadata.get("etag"): + request.add_header("If-None-Match", http_metadata["etag"]) + logger.debug(f"Added If-None-Match: {http_metadata['etag']}") + if http_metadata.get("last_modified"): + request.add_header("If-Modified-Since", http_metadata["last_modified"]) + logger.debug(f"Added If-Modified-Since: {http_metadata['last_modified']}") + + temp_download_dir = None + try: + response = urllib.request.urlopen(request) + + # Use temporary directory on same filesystem as artifact store + temp_download_dir = ArtifactStore.create_temp_dir(prefix="download-") + filename = url.split("/")[-1] + artifact_path = temp_download_dir / filename + + with artifact_path.open("wb") as f: + shutil.copyfileobj(response, f) + + etag = response.headers.get("ETag") + last_modified = response.headers.get("Last-Modified") + metadata_path = temp_download_dir / HTTP_METADATA_FILENAME + _save_http_metadata(temp_download_dir, etag, last_modified) + logger.debug(f"Saved HTTP metadata: ETag={etag}, Last-Modified={last_modified}") + + logger.info(f"Downloaded: {url} -> {artifact_path}") + return (False, [artifact_path], [metadata_path]) + + except urllib.error.HTTPError as e: + if e.code == HTTPStatus.NOT_MODIFIED: + logger.info(f"HTTP 304 Not Modified for {url} - content unchanged") + return (True, cached_data_files, cached_metadata_files) + # Clean up temp dir on error + if temp_download_dir: + ArtifactStore.delete_temp_dir(temp_download_dir) + raise ArtifactError(f"HTTP error {e.code} downloading {url}: {e}") from e + + except Exception as e: + # Clean up temp dir on error + if temp_download_dir: + ArtifactStore.delete_temp_dir(temp_download_dir) + raise ArtifactError(f"Failed to download {url}: {e}") from e diff --git a/fboss-image/distro_cli/tests/download_test.py b/fboss-image/distro_cli/tests/download_test.py new file mode 100644 index 0000000000000..d869c19286498 --- /dev/null +++ b/fboss-image/distro_cli/tests/download_test.py @@ -0,0 +1,268 @@ +#!/usr/bin/env python3 +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Unit tests for download utilities.""" + +import http.server +import json +import shutil +import socketserver +import tempfile +import threading +import time +import unittest +from pathlib import Path + +from distro_cli.lib.download import HTTP_METADATA_FILENAME, download_artifact +from distro_cli.tests.test_helpers import enter_tempdir, override_artifact_store_dir + + +class TestDownloadArtifact(unittest.TestCase): + """Test download_artifact function.""" + + def setUp(self): + """Set up test fixtures.""" + # Use sandbox-safe temporary directory + self._tempdir_ctx = enter_tempdir("download_test_") + self.temp_dir = self._tempdir_ctx.__enter__() + self.manifest_dir = self.temp_dir / "manifest" + self.manifest_dir.mkdir() + + # Override artifact store directory for testing + self.artifact_store_dir = self.temp_dir / ".artifacts" + self._artifact_store_ctx = override_artifact_store_dir(self.artifact_store_dir) + self._artifact_store_ctx.__enter__() + + def tearDown(self): + """Clean up test directory.""" + # Exit context managers in reverse order + self._artifact_store_ctx.__exit__(None, None, None) + self._tempdir_ctx.__exit__(None, None, None) + + def test_download_file_url(self): + """Test downloading from file:// URL.""" + source_file = self.temp_dir / "source.txt" + source_file.write_text("file content") + + url = f"file://{source_file}" + cache_hit, data_files, meta_files = download_artifact( + url, self.manifest_dir, [], [] + ) + + self.assertFalse(cache_hit) + self.assertEqual(len(data_files), 1) + self.assertEqual(len(meta_files), 1) # Now includes mtime metadata + self.assertTrue(data_files[0].exists()) + self.assertEqual(data_files[0].read_text(), "file content") + + # Verify metadata contains mtime + metadata = json.loads(meta_files[0].read_text()) + self.assertIn("mtime", metadata) + + def test_download_file_url_mtime_caching(self): + """Test that file:// URLs use mtime for cache detection.""" + source_file = self.temp_dir / "source.txt" + source_file.write_text("original content") + + url = f"file://{source_file}" + + # First download - should not be a cache hit + cache_hit1, data_files1, meta_files1 = download_artifact( + url, self.manifest_dir, [], [] + ) + self.assertFalse(cache_hit1) + self.assertEqual(len(data_files1), 1) + self.assertEqual(len(meta_files1), 1) + + # Second download without modifying file - should be a cache hit + cache_hit2, data_files2, meta_files2 = download_artifact( + url, self.manifest_dir, data_files1, meta_files1 + ) + self.assertTrue(cache_hit2) + self.assertEqual(data_files2, data_files1) + self.assertEqual(meta_files2, meta_files1) + + # Modify the source file + time.sleep(0.01) # Ensure mtime changes + source_file.write_text("modified content") + + # Third download after modification - should NOT be a cache hit + cache_hit3, data_files3, meta_files3 = download_artifact( + url, self.manifest_dir, data_files2, meta_files2 + ) + self.assertFalse(cache_hit3) + self.assertEqual(len(data_files3), 1) + self.assertEqual(len(meta_files3), 1) + + def test_metadata_file_structure(self): + """Test that metadata files are created with correct structure.""" + # Create a test metadata file + meta_file = self.temp_dir / HTTP_METADATA_FILENAME + meta_file.write_text( + json.dumps( + {"etag": "test-etag", "last_modified": "Mon, 01 Jan 2024 00:00:00 GMT"} + ) + ) + + # Verify structure + metadata = json.loads(meta_file.read_text()) + self.assertIn("etag", metadata) + self.assertIn("last_modified", metadata) + self.assertEqual(metadata["etag"], "test-etag") + self.assertEqual(metadata["last_modified"], "Mon, 01 Jan 2024 00:00:00 GMT") + + +class SimpleHTTPServer: + """Simple HTTP server for testing downloads.""" + + def __init__(self, directory, port=0): + """Initialize HTTP server. + + Args: + directory: Directory to serve files from + port: Port to listen on (default: 0 = let OS assign random port) + """ + self.directory = directory + self.port = port + self.server = None + self.server_thread = None + self._ready = threading.Event() + + def start(self): + """Start the HTTP server in a background thread.""" + # Enable SO_REUSEADDR to avoid "Address already in use" errors + socketserver.TCPServer.allow_reuse_address = True + + self.server = socketserver.TCPServer( + ("127.0.0.1", self.port), + lambda *args, **kwargs: http.server.SimpleHTTPRequestHandler( + *args, directory=str(self.directory), **kwargs + ), + ) + + # Get the actual port assigned by the OS (important when port=0) + self.port = self.server.server_address[1] + + def serve_with_ready_signal(): + """Serve forever and signal when ready.""" + self._ready.set() + self.server.serve_forever() + + self.server_thread = threading.Thread( + target=serve_with_ready_signal, daemon=True + ) + self.server_thread.start() + + # Wait for server to be ready (with timeout) + if not self._ready.wait(timeout=180): # 3 minutes max + raise RuntimeError("HTTP server failed to start within 3 minutes") + + def stop(self): + """Stop the HTTP server.""" + if self.server: + self.server.shutdown() + self.server.server_close() + + def get_url(self, filename): + """Get URL for a file served by this server. + + Args: + filename: Name of file to get URL for + + Returns: + Full HTTP URL to the file + """ + return f"http://127.0.0.1:{self.port}/{filename}" + + +class TestDownloadHTTP(unittest.TestCase): + """Test download_artifact with HTTP server.""" + + @classmethod + def setUpClass(cls): + """Start HTTP server for all tests.""" + cls.test_dir = Path(tempfile.mkdtemp()) + + # Create a test file to serve + cls.test_file = cls.test_dir / "test_artifact.tar.zst" + cls.test_file.write_text("test artifact content from HTTP") + + # Start HTTP server (port=0 lets OS assign random available port) + cls.http_server = SimpleHTTPServer(cls.test_dir) + cls.http_server.start() + + @classmethod + def tearDownClass(cls): + """Stop HTTP server and cleanup.""" + cls.http_server.stop() + if cls.test_dir.exists(): + shutil.rmtree(cls.test_dir) + + def setUp(self): + """Set up test fixtures.""" + # Use sandbox-safe temporary directory + self._tempdir_ctx = enter_tempdir("download_http_test_") + self.temp_dir = self._tempdir_ctx.__enter__() + self.manifest_dir = self.temp_dir / "manifest" + self.manifest_dir.mkdir() + + # Override artifact store directory for testing + self.artifact_store_dir = self.temp_dir / ".artifacts" + self._artifact_store_ctx = override_artifact_store_dir(self.artifact_store_dir) + self._artifact_store_ctx.__enter__() + + def tearDown(self): + """Clean up test directory.""" + # Exit context managers in reverse order + self._artifact_store_ctx.__exit__(None, None, None) + self._tempdir_ctx.__exit__(None, None, None) + + def test_download_http_url(self): + """Test downloading from HTTP URL.""" + url = self.http_server.get_url("test_artifact.tar.zst") + + cache_hit, data_files, meta_files = download_artifact( + url, self.manifest_dir, [], [] + ) + + self.assertFalse(cache_hit) + self.assertEqual(len(data_files), 1) + self.assertEqual(len(meta_files), 1) + self.assertTrue(data_files[0].exists()) + self.assertEqual(data_files[0].read_text(), "test artifact content from HTTP") + + # Verify metadata file exists and has correct structure + self.assertTrue(meta_files[0].exists()) + self.assertEqual(meta_files[0].name, HTTP_METADATA_FILENAME) + + def test_download_http_caching(self): + """Test HTTP caching with ETag/Last-Modified.""" + url = self.http_server.get_url("test_artifact.tar.zst") + + # First download + cache_hit1, data_files1, meta_files1 = download_artifact( + url, self.manifest_dir, [], [] + ) + + self.assertFalse(cache_hit1) + self.assertEqual(len(data_files1), 1) + self.assertEqual(len(meta_files1), 1) + + # Second download with cached files - should get 304 Not Modified + cache_hit2, data_files2, meta_files2 = download_artifact( + url, self.manifest_dir, data_files1, meta_files1 + ) + + # Should be a cache hit (304 Not Modified) + self.assertTrue(cache_hit2) + self.assertEqual(data_files2, data_files1) + self.assertEqual(meta_files2, meta_files1) + + +if __name__ == "__main__": + unittest.main() From d73c166bf6f8409d1cdd56b7be709142e847904e Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Fri, 9 Jan 2026 14:43:51 +0000 Subject: [PATCH 07/12] [Nexthop] Add command execution support for FBOSS Image Builder Add command execution capabilities for running build operations. - Implement execute module for running commands in Docker containers - Add support for command execution with output capture - Enable build operations to run in isolated container environments --- fboss-image/distro_cli/lib/execute.py | 81 +++++++++++++++++++++++++++ fboss-image/distro_cli/ruff.toml | 15 ++++- 2 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 fboss-image/distro_cli/lib/execute.py diff --git a/fboss-image/distro_cli/lib/execute.py b/fboss-image/distro_cli/lib/execute.py new file mode 100644 index 0000000000000..868806a239967 --- /dev/null +++ b/fboss-image/distro_cli/lib/execute.py @@ -0,0 +1,81 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Execute utilities for FBOSS image builder.""" + +import logging +from pathlib import Path + +from distro_cli.lib.docker.container import run_container +from distro_cli.lib.docker.image import build_fboss_builder_image + +from .exceptions import BuildError + +logger = logging.getLogger(__name__) + + +def execute_build_in_container( + image_name: str, + command: list[str], + volumes: dict[Path, Path], + component_name: str, + privileged: bool = False, + working_dir: str | None = None, + dependency_install_paths: dict[str, Path] | None = None, +) -> None: + """Execute build command in Docker container. + + Args: + image_name: Docker image name + command: Command to execute as list + volumes: Host to container path mappings + component_name: Component name + privileged: Run in privileged mode + working_dir: Working directory in container + dependency_install_paths: Dict mapping dependency names to their container mount paths + (e.g., {'kernel': Path('/deps/kernel')}) + RPMs from these paths will be installed before running the build + + Raises: + BuildError: If build fails + """ + logger.info(f"Executing {component_name} build in Docker container: {image_name}") + + # Ensure fboss_builder image is built + build_fboss_builder_image() + + # Use build_entrypoint.py from /tools (mounted from distro_cli/tools) + if dependency_install_paths: + logger.info( + f"Build entry point will process {len(dependency_install_paths)} dependencies" + ) + + # Build the command: python3 /tools/build_entrypoint.py + # The entry point will look for dependencies in /deps and install them. + cmd_list = ["python3", "/tools/build_entrypoint.py", *command] + logger.info("Build will produce uncompressed artifacts (.tar)") + + logger.info(f"Running in container: {' '.join(cmd_list)}") + + try: + exit_code = run_container( + image=image_name, + command=cmd_list, + volumes=volumes, + privileged=privileged, + working_dir=working_dir, + ) + + if exit_code != 0: + raise BuildError( + f"{component_name} build failed with exit code {exit_code}" + ) + + logger.info(f"{component_name} build in container complete") + + except RuntimeError as e: + raise BuildError(f"Failed to run Docker container: {e}") diff --git a/fboss-image/distro_cli/ruff.toml b/fboss-image/distro_cli/ruff.toml index fd81220907b99..e6171bfd83887 100644 --- a/fboss-image/distro_cli/ruff.toml +++ b/fboss-image/distro_cli/ruff.toml @@ -22,8 +22,17 @@ select = [ "RUF", # ruff-specific ] +ignore = [ + "B904", # Allow raising exceptions without 'from' for cleaner error messages + "PLR0913", # Allow more than 8 arguments to be specified +] + [lint.per-file-ignores] -# Tests need to modify sys.path before importing local modules # Tests may have unused mock arguments (ARG002) -"tests/*.py" = ["E402", "ARG002"] -"*/tests/*.py" = ["E402", "ARG002"] +# Tests may use open() instead of Path.open() for simplicity (PTH123) +"tests/*.py" = ["ARG002", "PTH123"] +"*/tests/*.py" = ["ARG002", "PTH123"] + +[lint.pylint] +# Allow more arguments for builder/execute functions +max-args = 8 From 5197a2f59d60286d96bf298966d6112aba87d23f Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Fri, 9 Jan 2026 14:49:09 +0000 Subject: [PATCH 08/12] [Nexthop] Add component system for FBOSS Image Builder Add abstract build component framework for managing build operations. - Implement AbstractComponent base class for build components - Add component lifecycle management (prepare, build, extract) - Integrate with artifact store, download, and execute modules - Enable extensible component-based build architecture Tests utilizing the above infrastructure will be added when component build supports are included. --- fboss-image/distro_cli/builder/__init__.py | 8 + fboss-image/distro_cli/builder/component.py | 343 ++++++++++++++++++ fboss-image/distro_cli/lib/manifest.py | 6 + .../distro_cli/tests/data/kernel-test.tar.zst | Bin 0 -> 99 bytes .../distro_cli/tests/kernel_download_test.py | 341 +++++++++++++++++ 5 files changed, 698 insertions(+) create mode 100644 fboss-image/distro_cli/builder/__init__.py create mode 100644 fboss-image/distro_cli/builder/component.py create mode 100644 fboss-image/distro_cli/tests/data/kernel-test.tar.zst create mode 100644 fboss-image/distro_cli/tests/kernel_download_test.py diff --git a/fboss-image/distro_cli/builder/__init__.py b/fboss-image/distro_cli/builder/__init__.py new file mode 100644 index 0000000000000..162bd57c07b74 --- /dev/null +++ b/fboss-image/distro_cli/builder/__init__.py @@ -0,0 +1,8 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""FBOSS Distribution CLI builder package.""" diff --git a/fboss-image/distro_cli/builder/component.py b/fboss-image/distro_cli/builder/component.py new file mode 100644 index 0000000000000..922d1d61760e1 --- /dev/null +++ b/fboss-image/distro_cli/builder/component.py @@ -0,0 +1,343 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Generic component builder for FBOSS image components.""" + +import hashlib +import logging +from os.path import commonpath +from pathlib import Path + +from distro_cli.lib.artifact import find_artifact_in_dir +from distro_cli.lib.constants import FBOSS_BUILDER_IMAGE +from distro_cli.lib.download import download_artifact +from distro_cli.lib.exceptions import ComponentError +from distro_cli.lib.execute import execute_build_in_container +from distro_cli.lib.paths import get_abs_path + +logger = logging.getLogger(__name__) + + +def _get_component_directory(component_name: str, script_path: str) -> str: + """Determine the component directory for build artifacts. + + For scripts_path that has the component_name, we return the path in script_path + leading to the component_name. Otherwise, the script's parent directory is returned. + + Examples: + kernel component: + component_name="kernel" + script_path="fboss-image/kernel/scripts/build_kernel.sh" + returns: "fboss-image/kernel" (component_name found in path) + + sai component: + component_name="sai" + script_path="broadcom-sai-sdk/build_fboss_sai.sh" + returns: "broadcom-sai-sdk" (fallback to script's parent) + + Args: + component_name: Base component name (without array index) + script_path: Path to the build script from the execute directive + + Returns: + Component directory path (relative to workspace root) + + """ + script_path_obj = Path(script_path) + + # Check if component_name appears in the script path + if component_name in script_path_obj.parts: + # Find the last occurrence of component_name in the path + # Handle cases where component name appears multiple times + # e.g., /src/kernel/fboss/12345/kernel/build.sh -> use the last "kernel" + parts = script_path_obj.parts + # Find last occurrence by reversing and using index + component_index = len(parts) - 1 - parts[::-1].index(component_name) + # Return the path up to and including the component_name + return str(Path(*parts[: component_index + 1])) + + # Fall back to script's parent directory + return str(script_path_obj.parent) + + +class ComponentBuilder: + """Generic builder for FBOSS image components. + + Supports two modes: + - download: Download pre-built artifact from URL + - execute: Build component using a build script in Docker container + + Component-specific logic (argument parsing, paths, etc.) should be + embedded in the component build script, not in this builder. + """ + + def __init__( + self, + component_name: str, + component_data: dict, + manifest_dir: Path, + store, + artifact_pattern: str | None = None, + dependency_artifacts: dict[str, Path] | None = None, + artifact_key_salt: str | None = None, + ): + """Initialize the component builder. + + Args: + component_name: Name of the component + component_data: Component data dict from manifest + manifest_dir: Path to the manifest directory + store: ArtifactStore instance + artifact_pattern: Glob pattern for finding build artifacts (e.g., "kernel-*.rpms.tar.zst") + If None, component cannot use execute mode + dependency_artifacts: Optional dict mapping dependency names to their artifact paths + artifact_key_salt: Salt added to artifact store key to differentiate variants + """ + self.component_name = component_name + self.component_data = component_data + self.manifest_dir = manifest_dir + self.store = store + self.artifact_pattern = artifact_pattern + self.dependency_artifacts = dependency_artifacts or {} + self.artifact_key_salt = artifact_key_salt + + def build(self) -> Path: + """Build or download the component. + + Returns: + Path to the component artifact (or None for empty components) + + Raises: + ComponentError: If component has invalid structure + """ + if self.component_data is None: + raise ComponentError(f"Component '{self.component_name}' has no data") + + # ComponentBuilder handles single component instances only + # Array components should be handled at a higher level (e.g., ImageBuilder) + # by creating one ComponentBuilder instance per array element + if isinstance(self.component_data, list): + raise ComponentError( + f"Component '{self.component_name}' data is an array. " + "ComponentBuilder only handles single component instances. " + "Create one ComponentBuilder per array element instead." + ) + + # Check for both download and execute (invalid) + has_download = "download" in self.component_data + has_execute = "execute" in self.component_data + + if has_download and has_execute: + raise ComponentError( + f"Component '{self.component_name}' has both 'download' and 'execute' fields. " + "Only one is allowed." + ) + + # Allow empty components + if not has_download and not has_execute: + logger.info(f"Component '{self.component_name}' is empty, skipping") + return None + + if has_download: + return self._download_component(self.component_data["download"]) + + # this must be an "execute" + # Use artifact pattern from manifest if specified, otherwise use default + artifact_pattern = self.component_data.get("artifact", self.artifact_pattern) + return self._execute_component(self.component_data["execute"], artifact_pattern) + + def _download_component(self, url: str) -> Path: + """Download component artifact from URL. + + Args: + url: URL to download from + + Returns: + Path to downloaded artifact (cached) + """ + store_key = ( + f"{self.component_name}-download-{hashlib.sha256(url.encode()).hexdigest()}" + ) + data_files, metadata_files = self.store.get( + store_key, + lambda data, meta: download_artifact(url, self.manifest_dir, data, meta), + ) + + if not data_files: + raise ComponentError(f"No artifact files found for {self.component_name}") + + artifact_path = data_files[0] + logger.info(f"{self.component_name} artifact ready: {artifact_path}") + if metadata_files: + logger.debug(f" with {len(metadata_files)} metadata file(s)") + return artifact_path + + def _execute_component( + self, cmd_line: str | list[str], artifact_pattern: str | None = None + ) -> Path: + """Execute component build in Docker container. + + Args: + cmd_line: Command line to execute (string or list of strings) + artifact_pattern: Optional artifact pattern override from manifest + + Returns: + Path to build artifact in cache + """ + # For store key, convert to string (works for both str and list) + # Include artifact_key_salt to differentiate cache variants (e.g., compressed vs uncompressed) + store_key_str = f"{cmd_line}-salt={self.artifact_key_salt}" + + # _execute_build as a fetch_fn always starts a build expecting the underlying + # build system to provide build specific optimizations. The objects are returned + # back to the store with a store-miss indication. + store_key = f"{self.component_name}-build-{hashlib.sha256(store_key_str.encode()).hexdigest()[:8]}" + data_files, _ = self.store.get( + store_key, + lambda _data, _meta: ( + False, + [self._execute_build(cmd_line, artifact_pattern)], + [], + ), + ) + + if not data_files: + raise ComponentError(f"No artifact files found for {self.component_name}") + + artifact_path = data_files[0] + logger.info(f"{self.component_name} build complete: {artifact_path}") + return artifact_path + + def _execute_build( + self, cmd_line: str | list[str], artifact_pattern: str | None = None + ) -> Path: + """Execute build in Docker container. + + Args: + cmd_line: Command line to execute (string or list of strings) + artifact_pattern: Optional artifact pattern override from manifest + + Returns: + Path to build artifact + + Raises: + ComponentError: If build fails or artifact not found + """ + # Get script path from command line + script_path_str = ( + cmd_line[0] if isinstance(cmd_line, list) else cmd_line.split()[0] + ) + + # Resolve script path: if absolute, use as-is; if relative, resolve from manifest_dir + script_path = Path(script_path_str) + resolved_script_path = ( + script_path + if script_path.is_absolute() + else (self.manifest_dir / script_path).resolve() + ) + + # Verify the script exists + if not resolved_script_path.exists(): + raise ComponentError( + f"Build script not found: {resolved_script_path} " + f"(from manifest path: {script_path_str})" + ) + + # We mount the common parent of the script path and manifest dir + src_dir = Path(commonpath([resolved_script_path, self.manifest_dir])) + script_relative_to_src = resolved_script_path.relative_to(src_dir) + container_script_path = Path("/src") / script_relative_to_src + + # For array elements, extract the base name + base_name = ( + self.component_name.split("[")[0] + if "[" in self.component_name + else self.component_name + ) + + # Determine component directory (component root if known, else script's parent) + # Use the resolved script path relative to src_dir + script_relative_to_src = resolved_script_path.relative_to(src_dir) + component_dir = _get_component_directory(base_name, str(script_relative_to_src)) + + # Create build and dist directories under the component directory + artifact_base_dir = src_dir / component_dir + + # Use artifact_pattern from parameter, or fall back to instance pattern, or use generic pattern + # Generic pattern uses .tar (will match both .tar and .tar.zst via compression variant finder) + if artifact_pattern is None: + artifact_pattern = self.artifact_pattern or "*.tar" + if not artifact_pattern: + logger.warning( + f"Component '{self.component_name}' has no artifact_pattern specified. " + f"Using generic pattern: {artifact_pattern}" + ) + + build_dir = artifact_base_dir / ".build" + build_dir.mkdir(parents=True, exist_ok=True) + + dist_dir = artifact_base_dir / "dist" + dist_dir.mkdir(parents=True, exist_ok=True) + + # Mount src_dir as /src in the container + logger.info(f"Mounting {src_dir} as /src") + + # Mount distro_cli/tools as /tools for build utilities + tools_dir = get_abs_path("fboss-image/distro_cli/tools") + + # Mount fboss/oss/scripts for common build utilities (sccache config, etc.) + common_scripts_dir = get_abs_path("fboss/oss/scripts") + + volumes = { + src_dir: Path("/src"), + build_dir: Path("/build"), + dist_dir: Path("/output"), + tools_dir: Path("/tools"), + common_scripts_dir: Path("/fboss/oss/scripts"), + } + + # Mount dependency artifacts into the container + dependency_install_paths = {} + for dep_name, dep_artifact in self.dependency_artifacts.items(): + dep_mount_point = Path(f"/deps/{dep_name}") + volumes[dep_artifact] = dep_mount_point + dependency_install_paths[dep_name] = dep_mount_point + logger.info( + f"Mounting dependency '{dep_name}' at {dep_mount_point}: {dep_artifact}" + ) + + # Working directory is the parent of the script + working_dir = str(container_script_path.parent) + + # Build the container command using the container script path + if isinstance(cmd_line, list): + # Replace first element with container path, keep the rest + container_cmd = [str(container_script_path), *cmd_line[1:]] + else: + # For string commands, replace the script path with the in-container version + container_cmd = [str(container_script_path)] + + logger.info(f"Container command: {container_cmd}") + + # Execute build command + execute_build_in_container( + image_name=FBOSS_BUILDER_IMAGE, + command=container_cmd, + volumes=volumes, + component_name=self.component_name, + privileged=False, + working_dir=working_dir, + dependency_install_paths=( + dependency_install_paths if dependency_install_paths else None + ), + ) + + return find_artifact_in_dir( + output_dir=dist_dir, + pattern=artifact_pattern, + component_name=self.component_name.capitalize(), + ) diff --git a/fboss-image/distro_cli/lib/manifest.py b/fboss-image/distro_cli/lib/manifest.py index ce0d6b61f4fb6..e3e07b121f90a 100644 --- a/fboss-image/distro_cli/lib/manifest.py +++ b/fboss-image/distro_cli/lib/manifest.py @@ -56,6 +56,12 @@ def has_component(self, component: str) -> bool: """Check if component is present in manifest.""" return component in self.data + def get_component(self, component: str) -> dict | None: + """Return component data in manifest.""" + if component not in self.data: + return None + return self.data[component] + def resolve_path(self, path: str) -> Path: """Resolve a path relative to the manifest file.""" if path.startswith("http://") or path.startswith("https://"): diff --git a/fboss-image/distro_cli/tests/data/kernel-test.tar.zst b/fboss-image/distro_cli/tests/data/kernel-test.tar.zst new file mode 100644 index 0000000000000000000000000000000000000000..a50fdf22540854a29477f8e48c6d3e78e3e8714f GIT binary patch literal 99 zcmV-p0G$6QwJ-go0Js4FodN)21P&CSHys8~ZM`aDRUKj5&2H&;|2}ueDTtHoE*)j8 z&$1wPyDXSQtf~Ds__M{nmCS!rhyoIX)({8)w6-9Jp>O~l;As>LuvN@-R*ZTR=mNCE FOZ*jfE3*Iq literal 0 HcmV?d00001 diff --git a/fboss-image/distro_cli/tests/kernel_download_test.py b/fboss-image/distro_cli/tests/kernel_download_test.py new file mode 100644 index 0000000000000..df551d4cc090a --- /dev/null +++ b/fboss-image/distro_cli/tests/kernel_download_test.py @@ -0,0 +1,341 @@ +#!/usr/bin/env python3 +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Test kernel component download functionality with caching.""" + +import hashlib +import http.server +import json +import shutil +import socketserver +import tempfile +import threading +import unittest +from pathlib import Path + +from distro_cli.builder.component import ComponentBuilder +from distro_cli.lib.artifact import ArtifactStore +from distro_cli.lib.manifest import ImageManifest + + +class SimpleHTTPServer: + """Simple HTTP server for testing downloads.""" + + def __init__(self, directory, port=0): + """Initialize HTTP server. + + Args: + directory: Directory to serve files from + port: Port to listen on (default: 0 = let OS assign random port) + """ + self.directory = directory + self.port = port + self.server = None + self.server_thread = None + self._ready = threading.Event() + + def start(self): + """Start the HTTP server in a background thread.""" + # Enable SO_REUSEADDR to avoid "Address already in use" errors + socketserver.TCPServer.allow_reuse_address = True + + self.server = socketserver.TCPServer( + ("127.0.0.1", self.port), + lambda *args, **kwargs: http.server.SimpleHTTPRequestHandler( + *args, directory=str(self.directory), **kwargs + ), + ) + + # Get the actual port assigned by the OS (important when port=0) + self.port = self.server.server_address[1] + + def serve_with_ready_signal(): + """Serve forever and signal when ready.""" + self._ready.set() + self.server.serve_forever() + + self.server_thread = threading.Thread( + target=serve_with_ready_signal, daemon=True + ) + self.server_thread.start() + + # Wait for server to be ready (with timeout) + if not self._ready.wait(timeout=180): # 3 minutes max + raise RuntimeError("HTTP server failed to start within 3 minutes") + + def stop(self): + """Stop the HTTP server.""" + if self.server: + self.server.shutdown() + self.server.server_close() + + def get_url(self, filename): + """Get URL for a file served by this server. + + Args: + filename: Name of file to get URL for + + Returns: + Full HTTP URL to the file + """ + return f"http://127.0.0.1:{self.port}/{filename}" + + +class KernelDownloadTestHelper: + """Helper class with common test logic for kernel download tests. + + This is NOT a TestCase - it's a helper that test classes use via delegation. + """ + + def __init__( + self, + test_case, + manifest_path, + store_dir, + source_file=None, + temp_source_dir=None, + ): + """Initialize helper. + + Args: + test_case: The unittest.TestCase instance (for assertions) + manifest_path: Path to manifest file + store_dir: Path to store directory + source_file: Original source file to copy from (for file:// tests) + temp_source_dir: Temp directory where source file is staged (for file:// tests) + """ + self.test_case = test_case + self.manifest_path = manifest_path + self.store_dir = store_dir + self.source_file = source_file + self.temp_source_dir = temp_source_dir + + def build_kernel(self): + """Build kernel with caching. + + For file:// tests, ensures source file exists by copying from original if needed. + + Returns: + Path to built/cached artifact + """ + # For file:// tests, ensure source file exists + # (it may have been moved to artifact store on previous build) + if self.source_file and self.temp_source_dir: + temp_source_file = self.temp_source_dir / self.source_file.name + if not temp_source_file.exists(): + shutil.copy2(self.source_file, temp_source_file) + + manifest = ImageManifest(self.manifest_path) + # Override class attribute for testing + ArtifactStore.ARTIFACT_STORE_DIR = self.store_dir + store = ArtifactStore() + + # Get kernel component data + kernel_data = manifest.get_component("kernel") + + kernel_builder = ComponentBuilder( + component_name="kernel", + component_data=kernel_data, + manifest_dir=manifest.manifest_dir, + store=store, + artifact_pattern="kernel-*.rpms.tar", + ) + return kernel_builder.build() + + def get_store_key(self): + """Get store key for kernel component in manifest. + + Returns: + Store key string + """ + manifest = ImageManifest(self.manifest_path) + component_data = manifest.get_component("kernel") + url = component_data["download"] + return f"kernel-download-{hashlib.sha256(url.encode()).hexdigest()}" + + def test_download_and_cache(self): + """Test that kernel download works and caching is effective.""" + # First build - should download and cache + artifact_path_1 = self.build_kernel() + + # Verify artifact exists and is in cache + self.test_case.assertTrue( + artifact_path_1.exists(), f"Artifact not found: {artifact_path_1}" + ) + self.test_case.assertTrue( + str(artifact_path_1).startswith(str(self.store_dir)), + f"Artifact not in store dir: {artifact_path_1}", + ) + + # Second build - should hit cache (no download) + artifact_path_2 = self.build_kernel() + + # Verify we got the same cached artifact + self.test_case.assertEqual( + artifact_path_1, + artifact_path_2, + "Second build should return same cached artifact", + ) + self.test_case.assertTrue( + artifact_path_2.exists(), f"Cached artifact not found: {artifact_path_2}" + ) + + def test_cache_invalidation(self): + """Test that cache invalidation works.""" + # Build and cache artifact + artifact_path = self.build_kernel() + self.test_case.assertTrue(artifact_path.exists()) + + # Get store and invalidate + ArtifactStore.ARTIFACT_STORE_DIR = self.store_dir + store = ArtifactStore() + store_key = self.get_store_key() + store.invalidate(store_key) + + # Verify artifact is gone + self.test_case.assertFalse( + artifact_path.exists(), "Artifact should be removed after invalidation" + ) + + def test_cache_clear(self): + """Test that clearing cache removes all artifacts.""" + # Build and cache artifact + artifact_path = self.build_kernel() + self.test_case.assertTrue(artifact_path.exists()) + + # Clear the store + ArtifactStore.ARTIFACT_STORE_DIR = self.store_dir + store = ArtifactStore() + store.clear() + + # Verify store directory is empty + store_contents = list(self.store_dir.glob("*")) + self.test_case.assertEqual( + len(store_contents), + 0, + f"Store should be empty after clear, but contains: {store_contents}", + ) + + +class TestKernelDownloadFile(unittest.TestCase): + """Test kernel component download from file:// URL.""" + + def setUp(self): + """Set up test fixtures.""" + self.test_dir = Path(__file__).parent + self.store_dir = Path(tempfile.mkdtemp(prefix="test-store-")) + + # Create temp directory for source files (on same filesystem as artifact store) + # This ensures the file can be moved efficiently during download + self.temp_source_dir = Path(tempfile.mkdtemp(prefix="test-source-")) + + # Copy test data file to temp source directory + # This way the original test data file is preserved when download moves it + # Downloads should use compressed files to save bandwidth + self.source_file = self.test_dir / "data" / "kernel-test.tar.zst" + temp_source_file = self.temp_source_dir / "kernel-test.tar.zst" + shutil.copy2(self.source_file, temp_source_file) + + # Create test manifest that points to temp source file + self.manifest_path = self.temp_source_dir / "test-manifest.json" + manifest_data = { + "kernel": {"download": f"file:{temp_source_file}"}, + "distribution_formats": {"usb": "output/test-download.iso"}, + } + with open(self.manifest_path, "w") as f: + json.dump(manifest_data, f, indent=2) + + # Create helper with delegation, passing source file info + self.helper = KernelDownloadTestHelper( + self, + self.manifest_path, + self.store_dir, + source_file=self.source_file, + temp_source_dir=self.temp_source_dir, + ) + + def tearDown(self): + """Clean up test fixtures.""" + if self.store_dir.exists(): + shutil.rmtree(self.store_dir) + if self.temp_source_dir.exists(): + shutil.rmtree(self.temp_source_dir) + + def test_download_and_cache(self): + """Test that kernel download works and caching is effective.""" + self.helper.test_download_and_cache() + + def test_cache_invalidation(self): + """Test that cache invalidation works.""" + self.helper.test_cache_invalidation() + + def test_cache_clear(self): + """Test that clearing cache removes all artifacts.""" + self.helper.test_cache_clear() + + +class TestKernelDownloadHTTP(unittest.TestCase): + """Test kernel component download from HTTP server.""" + + @classmethod + def setUpClass(cls): + """Start HTTP server for all tests.""" + cls.test_dir = Path(__file__).parent + cls.data_dir = cls.test_dir / "data" + + # Start HTTP server using helper (port=0 lets OS assign random available port) + cls.http_server = SimpleHTTPServer(cls.data_dir) + cls.http_server.start() + + @classmethod + def tearDownClass(cls): + """Stop HTTP server.""" + cls.http_server.stop() + + def setUp(self): + """Set up test fixtures.""" + self.temp_dir = Path(tempfile.mkdtemp(prefix="test-http-")) + self.store_dir = Path(tempfile.mkdtemp(prefix="test-store-")) + + # Create test manifest with HTTP URL + self.manifest_path = self.temp_dir / "test-http-manifest.json" + manifest_data = { + "name": "test-http-download", + "version": "1.0.0", + "kernel": {"download": self.http_server.get_url("kernel-test.tar.zst")}, + "distribution_formats": {"usb": "output/test.iso"}, + } + + with open(self.manifest_path, "w") as f: + json.dump(manifest_data, f, indent=2) + + # Create helper with delegation (HTTP tests don't need source file restoration) + self.helper = KernelDownloadTestHelper(self, self.manifest_path, self.store_dir) + + def tearDown(self): + """Clean up test fixtures.""" + if self.temp_dir.exists(): + shutil.rmtree(self.temp_dir) + if self.store_dir.exists(): + shutil.rmtree(self.store_dir) + + def test_download_and_cache(self): + """Test that kernel download works and caching is effective.""" + self.helper.test_download_and_cache() + + def test_cache_invalidation(self): + """Test that cache invalidation works.""" + self.helper.test_cache_invalidation() + + def test_cache_clear(self): + """Test that clearing cache removes all artifacts.""" + self.helper.test_cache_clear() + + +if __name__ == "__main__": + unittest.main() From 853ec8833c819c5090ad06e10dd4344629b157b0 Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Fri, 9 Jan 2026 14:55:30 +0000 Subject: [PATCH 09/12] [Nexthop] Add build entrypoint for FBOSS Image Builder Add build entrypoint orchestration for component-based builds. - Implement build entrypoint for coordinating component build workflows - Add support for build configuration and execution management - Include comprehensive unit tests for entrypoint functionality --- .../distro_cli/tests/build_entrypoint_test.py | 191 +++++++++++++++++ .../tests/data/kernel/stub_build.sh | 22 ++ .../tests/data/test-stub-component.json | 9 + fboss-image/distro_cli/tools/__init__.py | 8 + .../distro_cli/tools/build_entrypoint.py | 201 ++++++++++++++++++ 5 files changed, 431 insertions(+) create mode 100644 fboss-image/distro_cli/tests/build_entrypoint_test.py create mode 100755 fboss-image/distro_cli/tests/data/kernel/stub_build.sh create mode 100644 fboss-image/distro_cli/tests/data/test-stub-component.json create mode 100644 fboss-image/distro_cli/tools/__init__.py create mode 100644 fboss-image/distro_cli/tools/build_entrypoint.py diff --git a/fboss-image/distro_cli/tests/build_entrypoint_test.py b/fboss-image/distro_cli/tests/build_entrypoint_test.py new file mode 100644 index 0000000000000..a78077221eac3 --- /dev/null +++ b/fboss-image/distro_cli/tests/build_entrypoint_test.py @@ -0,0 +1,191 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Test build_entrypoint.py behavior.""" + +import subprocess +import tarfile +import unittest +from pathlib import Path + +from distro_cli.lib.constants import FBOSS_BUILDER_IMAGE +from distro_cli.lib.docker.container import run_container +from distro_cli.lib.paths import get_abs_path +from distro_cli.tests.test_helpers import ensure_test_docker_image, enter_tempdir + + +class TestBuildEntrypoint(unittest.TestCase): + """Test build_entrypoint.py as universal build entry point.""" + + @classmethod + def setUpClass(cls): + """Ensure fboss_builder image exists before running tests.""" + ensure_test_docker_image() + + def test_entrypoint_without_dependencies(self): + """Test build_entrypoint.py executes build command when no dependencies exist.""" + with enter_tempdir("entrypoint_no_deps_") as tmpdir_path: + output_file = tmpdir_path / "build_output.txt" + + # Mount tools directory (contains build_entrypoint.py) + # No /deps mount - simulates build without dependencies + tools_dir = get_abs_path("fboss-image/distro_cli/tools") + exit_code = run_container( + image=FBOSS_BUILDER_IMAGE, + command=[ + "python3", + "/tools/build_entrypoint.py", + "sh", + "-c", + "echo 'build completed' > /output/build_output.txt", + ], + volumes={tools_dir: Path("/tools"), tmpdir_path: Path("/output")}, + ephemeral=True, + ) + + self.assertEqual(exit_code, 0, "Build should succeed without dependencies") + self.assertTrue(output_file.exists(), "Build output should be created") + self.assertEqual(output_file.read_text().strip(), "build completed") + + def test_entrypoint_with_empty_dependencies(self): + """Test build_entrypoint.py handles empty /deps directory gracefully.""" + with enter_tempdir("entrypoint_empty_deps_") as tmpdir_path: + output_file = tmpdir_path / "build_output.txt" + deps_dir = tmpdir_path / "deps" + deps_dir.mkdir(exist_ok=True) + + # Mount empty /deps directory + tools_dir = get_abs_path("fboss-image/distro_cli/tools") + exit_code = run_container( + image=FBOSS_BUILDER_IMAGE, + command=[ + "python3", + "/tools/build_entrypoint.py", + "sh", + "-c", + "echo 'build with empty deps' > /output/build_output.txt", + ], + volumes={ + tools_dir: Path("/tools"), + tmpdir_path: Path("/output"), + deps_dir: Path("/deps"), + }, + ephemeral=True, + ) + + self.assertEqual( + exit_code, 0, "Build should succeed with empty dependencies" + ) + self.assertTrue(output_file.exists()) + self.assertEqual(output_file.read_text().strip(), "build with empty deps") + + def test_entrypoint_with_compressed_dependency(self): + """Test build_entrypoint.py can extract zstd-compressed dependency tarballs.""" + with enter_tempdir("entrypoint_compressed_deps_") as tmpdir_path: + # Create a dummy file to include in the tarball + deps_content_dir = tmpdir_path / "deps_content" + deps_content_dir.mkdir() + dummy_file = deps_content_dir / "test.txt" + dummy_file.write_text("compressed dependency content") + + # Create uncompressed tarball first + uncompressed_tar = tmpdir_path / "dependency.tar" + with tarfile.open(uncompressed_tar, "w") as tar: + tar.add(dummy_file, arcname="test.txt") + + # Compress with zstd + compressed_tar = tmpdir_path / "dependency.tar.zst" + subprocess.run( + ["zstd", str(uncompressed_tar), "-o", str(compressed_tar)], + check=True, + capture_output=True, + ) + uncompressed_tar.unlink() # Remove uncompressed version + + # Create deps directory and move compressed tarball there + deps_dir = tmpdir_path / "deps" + deps_dir.mkdir() + final_dep = deps_dir / "dependency.tar.zst" + compressed_tar.rename(final_dep) + + output_file = tmpdir_path / "build_output.txt" + + # Run build_entrypoint.py with compressed dependency + tools_dir = get_abs_path("fboss-image/distro_cli/tools") + exit_code = run_container( + image=FBOSS_BUILDER_IMAGE, + command=[ + "python3", + "/tools/build_entrypoint.py", + "sh", + "-c", + "echo 'build with compressed deps' > /output/build_output.txt", + ], + volumes={ + tools_dir: Path("/tools"), + tmpdir_path: Path("/output"), + deps_dir: Path("/deps"), + }, + ephemeral=True, + ) + + self.assertEqual( + exit_code, 0, "Build should succeed with compressed dependencies" + ) + self.assertTrue(output_file.exists()) + self.assertEqual( + output_file.read_text().strip(), "build with compressed deps" + ) + + def test_entrypoint_with_uncompressed_dependency(self): + """Test build_entrypoint.py can extract uncompressed dependency tarballs.""" + with enter_tempdir("entrypoint_uncompressed_deps_") as tmpdir_path: + # Create a dummy file to include in the tarball + deps_content_dir = tmpdir_path / "deps_content" + deps_content_dir.mkdir() + dummy_file = deps_content_dir / "test.txt" + dummy_file.write_text("uncompressed dependency content") + + # Create uncompressed tarball + deps_dir = tmpdir_path / "deps" + deps_dir.mkdir() + uncompressed_tar = deps_dir / "dependency.tar" + with tarfile.open(uncompressed_tar, "w") as tar: + tar.add(dummy_file, arcname="test.txt") + + output_file = tmpdir_path / "build_output.txt" + + # Run build_entrypoint.py with uncompressed dependency + tools_dir = get_abs_path("fboss-image/distro_cli/tools") + exit_code = run_container( + image=FBOSS_BUILDER_IMAGE, + command=[ + "python3", + "/tools/build_entrypoint.py", + "sh", + "-c", + "echo 'build with uncompressed deps' > /output/build_output.txt", + ], + volumes={ + tools_dir: Path("/tools"), + tmpdir_path: Path("/output"), + deps_dir: Path("/deps"), + }, + ephemeral=True, + ) + + self.assertEqual( + exit_code, 0, "Build should succeed with uncompressed dependencies" + ) + self.assertTrue(output_file.exists()) + self.assertEqual( + output_file.read_text().strip(), "build with uncompressed deps" + ) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/fboss-image/distro_cli/tests/data/kernel/stub_build.sh b/fboss-image/distro_cli/tests/data/kernel/stub_build.sh new file mode 100755 index 0000000000000..0377354cff47e --- /dev/null +++ b/fboss-image/distro_cli/tests/data/kernel/stub_build.sh @@ -0,0 +1,22 @@ +#!/bin/bash +# Stub kernel build script for testing +# Creates a minimal kernel artifact tarball for testing purposes +set -e + +# Parse arguments +OUTPUT_DIR="/output" + +echo "Stub kernel build - creating test artifact" + +# Create a minimal kernel RPM structure +mkdir -p "$OUTPUT_DIR" +cd "$OUTPUT_DIR" + +# Create a dummy kernel RPM file +echo "dummy kernel rpm" >kernel-test.rpm + +tar -cf kernel-test.rpms.tar kernel-test.rpm +echo "Stub kernel artifact created: $OUTPUT_DIR/kernel-test.rpms.tar" + +# Clean up +rm kernel-test.rpm diff --git a/fboss-image/distro_cli/tests/data/test-stub-component.json b/fboss-image/distro_cli/tests/data/test-stub-component.json new file mode 100644 index 0000000000000..566dc601a6c7b --- /dev/null +++ b/fboss-image/distro_cli/tests/data/test-stub-component.json @@ -0,0 +1,9 @@ +{ + "kernel": { + "execute": ["kernel/stub_build.sh"], + "artifact": "kernel-test.rpms.tar" + }, + "distribution_formats": { + "usb": "output/test-stub-component.iso" + } +} diff --git a/fboss-image/distro_cli/tools/__init__.py b/fboss-image/distro_cli/tools/__init__.py new file mode 100644 index 0000000000000..c9f48b8ac1518 --- /dev/null +++ b/fboss-image/distro_cli/tools/__init__.py @@ -0,0 +1,8 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""FBOSS Distribution CLI tools package.""" diff --git a/fboss-image/distro_cli/tools/build_entrypoint.py b/fboss-image/distro_cli/tools/build_entrypoint.py new file mode 100644 index 0000000000000..966c79aab2201 --- /dev/null +++ b/fboss-image/distro_cli/tools/build_entrypoint.py @@ -0,0 +1,201 @@ +#!/usr/bin/env python3 +""" +Build entry point for component builds inside the container. + +This is the standard entry point for all component builds. It: +1. Discovers dependencies mounted at /deps/ +2. Extracts tarballs if needed +3. Installs RPMs from dependencies +4. Executes the component build script + +Usage: + python3 build_entrypoint.py [args...] + +Example: + python3 /src/fboss-image/distro_cli/lib/build_entrypoint.py /src/fboss-image/kernel/scripts/build.sh +""" + +import logging +import shutil +import subprocess +import sys +from pathlib import Path +from typing import Optional + +logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") +logger = logging.getLogger(__name__) + +# Standard location where dependencies are mounted +DEPENDENCIES_DIR = Path("/deps") + + +def extract_tarball(tarball_path: Path, extract_dir: Path) -> None: + """Extract a tarball to the specified directory using tar command. + + Args: + tarball_path: Path to the tarball file + extract_dir: Directory to extract to + """ + logger.info(f"Extracting {tarball_path} to {extract_dir}") + try: + # Use tar command for better compression support (zstd) and multithreading + cmd = ["tar", "-xf", str(tarball_path), "-C", str(extract_dir)] + logger.info(f"Running: {' '.join(cmd)}") + subprocess.run(cmd, check=True, capture_output=True, text=True) + logger.info(f"Successfully extracted {tarball_path}") + except subprocess.CalledProcessError as e: + logger.error(f"Failed to extract {tarball_path}: {e.stderr}") + raise + except Exception as e: + logger.error(f"Failed to extract {tarball_path}: {e}") + raise + + +def find_rpms(directory: Path) -> list[Path]: + """Find all RPM files in a directory (recursively). + + Args: + directory: Directory to search + + Returns: + List of paths to RPM files + """ + rpms = list(directory.rglob("*.rpm")) + logger.info(f"Found {len(rpms)} RPM(s) in {directory}") + return rpms + + +def install_rpms(rpm_paths: list[Path]) -> None: + """Install RPMs using dnf. + + Args: + rpm_paths: List of paths to RPM files + """ + if not rpm_paths: + logger.info("No RPMs to install") + return + + rpm_str_paths = [str(rpm) for rpm in rpm_paths] + logger.info( + f"Installing {len(rpm_paths)} RPM(s): {', '.join([rpm.name for rpm in rpm_paths])}" + ) + + cmd = ["dnf", "install", "-y", *rpm_str_paths] + logger.info(f"Running: {' '.join(cmd)}") + subprocess.run(cmd, check=True, capture_output=False) + logger.info("Successfully installed RPMs") + + +def discover_dependencies() -> list[Path]: + """Discover all dependencies in the standard /deps directory. + + Returns: + List of paths to dependency artifacts (files or directories) + """ + if not DEPENDENCIES_DIR.exists(): + logger.info(f"No dependencies directory found at {DEPENDENCIES_DIR}") + return [] + + dependencies = [] + for dep_path in DEPENDENCIES_DIR.iterdir(): + if dep_path.name.startswith("."): + # Skip hidden files/directories + continue + dependencies.append(dep_path) + logger.info(f"Discovered dependency: {dep_path.name}") + + return dependencies + + +def process_dependency(dep_path: Path) -> Optional[Path]: + """Process a dependency: extract if tarball, then install RPMs. + + Args: + dep_path: Path to the dependency (file or directory) + + Returns: + The directory where the dependency was extracted, or None if nothing + was extracted (for directory deps or missing paths). + """ + dep_name = dep_path.name + logger.info(f"Processing dependency '{dep_name}' at {dep_path}") + + extract_dir: Optional[Path] = None + + if not dep_path.exists(): + logger.warning(f"Dependency path does not exist: {dep_path}") + return None + + # Determine the directory to search for RPMs + if dep_path.is_file(): + # It's a tarball - extract it to /deps/-extracted + extract_dir = DEPENDENCIES_DIR / f"{dep_name}-extracted" + extract_dir.mkdir(parents=True, exist_ok=True) + extract_tarball(dep_path, extract_dir) + search_dir = extract_dir + else: + # It's already a directory + search_dir = dep_path + + # Find and install RPMs + rpms = find_rpms(search_dir) + if rpms: + install_rpms(rpms) + else: + logger.info(f"No RPMs found in dependency '{dep_name}'") + return extract_dir + + +def main(): + # Minimum required arguments: script name + build command + min_args = 2 + if len(sys.argv) < min_args: + logger.error("Usage: build_entrypoint.py [args...]") + logger.error( + "Example: build_entrypoint.py /src/fboss-image/kernel/scripts/build.sh" + ) + sys.exit(1) + + # Build command is everything after the script name + build_command = sys.argv[1:] + + logger.info("=" * 60) + logger.info("Dependency Installation and Build Entry Point") + logger.info("=" * 60) + + # Discover and process all dependencies + dependencies = discover_dependencies() + extract_dirs: list[Path] = [] + if dependencies: + logger.info(f"Found {len(dependencies)} dependency/dependencies") + for dep_path in dependencies: + try: + extract_dir = process_dependency(dep_path) + if extract_dir is not None: + extract_dirs.append(extract_dir) + except Exception as e: + logger.error(f"Failed to process dependency '{dep_path.name}': {e}") + sys.exit(1) + else: + logger.info("No dependencies found - proceeding with build") + + # Execute the build command + logger.info("=" * 60) + logger.info(f"Executing build: {' '.join(build_command)}") + logger.info("=" * 60) + + try: + result = subprocess.run(build_command, check=False) + returncode = result.returncode + except Exception as e: + logger.error(f"Failed to execute build command: {e}") + returncode = 1 + finally: + for extract_dir in extract_dirs: + shutil.rmtree(extract_dir) + + sys.exit(returncode) + + +if __name__ == "__main__": + main() From 5210f7b7656c9d9693c90786faab4da57181be80 Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Mon, 26 Jan 2026 02:06:26 +0000 Subject: [PATCH 10/12] [Nexthop] Add ImageBuilder orchestration for FBOSS Image Builder Add ImageBuilder class for orchestrating component builds and image assembly. - ImageBuilder: Main orchestration class for building FBOSS images - Compression support for build artifacts (zstd) - Component build coordination - Test coverage for compression functionality - Updated build_test.py to use ImageBuilder --- .../distro_cli/builder/image_builder.py | 417 ++++++++++++++++++ fboss-image/distro_cli/cmds/build.py | 6 +- fboss-image/distro_cli/lib/docker/image.py | 111 +---- fboss-image/distro_cli/tests/build_test.py | 68 +-- .../distro_cli/tests/test_compression.py | 68 +++ 5 files changed, 524 insertions(+), 146 deletions(-) create mode 100644 fboss-image/distro_cli/builder/image_builder.py create mode 100644 fboss-image/distro_cli/tests/test_compression.py diff --git a/fboss-image/distro_cli/builder/image_builder.py b/fboss-image/distro_cli/builder/image_builder.py new file mode 100644 index 0000000000000..3f71d63f6153c --- /dev/null +++ b/fboss-image/distro_cli/builder/image_builder.py @@ -0,0 +1,417 @@ +# Copyright (c) 2004-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +"""Image Builder - handles building FBOSS images from manifests.""" + +import logging +import shutil +import subprocess +from pathlib import Path +from typing import ClassVar + +from distro_cli.builder.component import ComponentBuilder +from distro_cli.lib.artifact import ArtifactStore, find_artifact_in_dir +from distro_cli.lib.constants import FBOSS_BUILDER_IMAGE +from distro_cli.lib.docker.container import run_container +from distro_cli.lib.docker.image import build_fboss_builder_image +from distro_cli.lib.exceptions import BuildError, ComponentError, ManifestError +from distro_cli.lib.paths import get_abs_path + +logger = logging.getLogger(__name__) + +# Component-specific artifact patterns +# These are used when the manifest doesn't specify an "artifact" field +# Patterns use base .tar extension - the artifact finder will automatically +# try both uncompressed (.tar) and zstd-compressed (.tar.zst) variants +COMPONENT_ARTIFACT_PATTERNS = { + "kernel": "kernel-*.rpms.tar", + "sai": "sai-*.tar", + "fboss-platform-stack": "fboss-platform-stack-*.tar", + "fboss-forwarding-stack": "fboss-forwarding-stack-*.tar", + "bsps": "bsp-*.tar", +} + + +class ImageBuilder: + """Handles building FBOSS images from manifests.""" + + # Component dependencies - defines build order and dependencies + # Format: {component: [list of dependencies]} + # The order of keys defines the build order (dependencies first) + COMPONENTS: ClassVar[dict[str, list[str]]] = { + "kernel": [], + "other_dependencies": [], + "fboss-platform-stack": [], + "bsps": ["kernel"], # Each BSP needs kernel headers/modules + "sai": ["kernel"], # SAI needs kernel headers/RPMs + "fboss-forwarding-stack": ["sai"], # Forwarding stack needs SAI library + "image_build_hooks": [], + } + + def __init__(self, manifest, kiwi_ng_debug=False): + self.manifest = manifest + self.workspace_root = manifest.manifest_dir + self.store = ArtifactStore() + self.component_artifacts = {} + self.kiwi_ng_debug = kiwi_ng_debug + # Setup the image builder directory + self.image_builder_dir = get_abs_path("fboss-image/image_builder") + self.centos_template_dir = self.image_builder_dir / "templates" / "centos-09.0" + self.after_pkgs_install_file = ( + self.centos_template_dir / "after_pkgs_install_file.json" + ) + self.after_pkgs_execute_file = ( + self.centos_template_dir / "after_pkgs_execute_file.json" + ) + self.compress_artifacts = False + + def _compress_artifact(self, artifact_path: Path, component_name: str) -> Path: + """Compress artifact using zstd.""" + if not self.compress_artifacts: + return artifact_path + + if artifact_path.name.endswith(".tar.zst"): + return artifact_path + + compressed_path = artifact_path.with_suffix(".tar.zst") + logger.info(f"{component_name}: Compressing {artifact_path.name}") + + try: + subprocess.run( + ["zstd", "-T0", str(artifact_path), "-o", str(compressed_path)], + check=True, + capture_output=True, + text=True, + ) + artifact_path.unlink() + logger.info(f"{component_name}: Compressed to {compressed_path.name}") + return compressed_path + + except subprocess.CalledProcessError as e: + raise BuildError(f"{component_name}: Compression failed: {e.stderr}") from e + + def _create_component_builder( + self, + component_name: str, + component_data: dict, + dependency_artifacts: dict[str, Path] | None = None, + ) -> ComponentBuilder: + """Create a ComponentBuilder for the given component using conventions. + + Args: + component_name: Name of the component (from JSON key) + component_data: Component data dict from manifest + dependency_artifacts: Dictionary mapping dependency names to their artifact paths + + Returns: + ComponentBuilder instance configured for the component + """ + # For array elements, extract the base name + base_name = component_name.split("[")[0] + + # Get artifact pattern from the predefined patterns + artifact_pattern = COMPONENT_ARTIFACT_PATTERNS.get(base_name) + + # Determine artifact key salt based on whether we'll compress the artifact + # This ensures compressed and uncompressed builds use different artifact store keys + artifact_key_salt = "compressed" if self.compress_artifacts else "uncompressed" + + return ComponentBuilder( + component_name=component_name, + component_data=component_data, + manifest_dir=self.manifest.manifest_dir, + store=self.store, + artifact_pattern=artifact_pattern, + dependency_artifacts=dependency_artifacts or {}, + artifact_key_salt=artifact_key_salt, + ) + + def build_all(self): + """Build all components and distribution artifacts.""" + logger.info("Building FBOSS Image") + + # Full build: components will be immediately extracted and used + # Skip compression to save several minutes of overhead + self.compress_artifacts = False + + for component in self.COMPONENTS: + if self.manifest.has_component(component): + self._build_component(component) + + self._build_base_image() + + def build_components(self, component_names: list[str]): + """Build specific components.""" + logger.info(f"Building components: {', '.join(component_names)}") + + # Component-only build: artifacts may be stored/reused later + # Enable compression for component builds (not the default) + self.compress_artifacts = True + + for component in component_names: + if not self.manifest.has_component(component): + raise ComponentError(f"Component '{component}' not found in manifest") + + for component in self.COMPONENTS: + if component in component_names: + self._build_component(component) + + def _move_distro_file(self, format_name: str, file_extension: str): + dist_formats = self.manifest.data.get("distribution_formats") + if not dist_formats or format_name not in dist_formats: + return + + output = find_artifact_in_dir( + output_dir=self.image_builder_dir / "output", + pattern=f"FBOSS-Distro-Image.x86_64-1.0.install.{file_extension}", + component_name="Base image", + ) + image = Path(dist_formats[format_name]) + shutil.move(str(output), str(image)) + + def _stage_component_artifacts(self) -> Path: + """Stage component artifacts for container mounting. + + Returns the *container* path where artifacts will be visible. + Artifacts are staged under image_builder_dir/deps_staging on the host, + which is exposed inside the container via the /image_builder bind mount. + This allows for hardlink operations (cp -la) within a single filesystem. + """ + deps_tmpdir = self.image_builder_dir / "deps_staging" + deps_tmpdir.mkdir(parents=True, exist_ok=True) + + if not self.component_artifacts: + raise BuildError("No component artifacts found; cannot build image.") + + logger.info(f"Staging component artifacts in {deps_tmpdir}") + + for component_name, artifact in self.component_artifacts.items(): + if artifact is None: + continue + + component_dir = deps_tmpdir / component_name + component_dir.mkdir(parents=True, exist_ok=True) + + artifacts_to_copy = ( + [artifact] if not isinstance(artifact, list) else artifact + ) + + for artifact_path in artifacts_to_copy: + dest_path = component_dir / artifact_path.name + shutil.copy2(artifact_path, dest_path) + logger.info(f"Staged {component_name}: {artifact_path.name}") + + return Path("/image_builder/deps_staging") + + def _build_base_image(self): + """Build the base OS image and create distribution artifacts.""" + logger.info("Starting base OS image build") + + dist_formats = self.manifest.data.get("distribution_formats") + if not dist_formats or not any( + k in dist_formats for k in ["usb", "pxe", "onie"] + ): + raise ManifestError("No distribution formats specified in manifest") + + logger.info(f"Using image builder: {self.image_builder_dir}") + + build_fboss_builder_image() + + volumes = { + self.image_builder_dir: Path("/image_builder"), + Path("/dev"): Path("/dev"), + } + + self._stage_component_artifacts() + + command = [ + "/image_builder/bin/build_image_in_container.sh", + ] + + # Add flags for PXE/USB if either is requested + if "usb" in dist_formats or "pxe" in dist_formats: + command.append("--build-pxe-usb") + + # Add flag for ONIE if requested + if "onie" in dist_formats: + command.append("--build-onie") + + if self.kiwi_ng_debug: + command.append("--debug") + + image_build_hooks = self.manifest.data.get("image_build_hooks") + try: + Path.unlink(self.after_pkgs_install_file) # Best effort + Path.unlink(self.after_pkgs_execute_file) + except FileNotFoundError: + pass + + # Helper function to process hook files + def process_hook_file(hook_key: str, target_file: Path, cmd_flag: str): + if hook_key in image_build_hooks: + user_input_file = self.manifest.manifest_dir / Path( + image_build_hooks[hook_key] + ) + + if user_input_file.is_file(): + # Copy to fixed name in template/centos-09.0 directory + shutil.copy(str(user_input_file), target_file) + logger.info( + f"Copied {user_input_file.name} to {self.centos_template_dir}" + ) + + # Pass the filename as an argument to the build_image_in_container.sh script + command.append(cmd_flag) + command.append(target_file.name) + else: + raise BuildError( + f"User specified {hook_key}, but file not found: {user_input_file.name}" + ) + + # Process both hook files + process_hook_file( + "after_pkgs_install", + self.after_pkgs_install_file, + "--after-pkgs-input-file", + ) + process_hook_file( + "after_pkgs_execute", + self.after_pkgs_execute_file, + "--after-pkgs-execute-file", + ) + + # Run the build script inside fboss_builder container + try: + exit_code = run_container( + image=FBOSS_BUILDER_IMAGE, + command=command, + volumes=volumes, + privileged=True, + ) + + if exit_code != 0: + raise BuildError(f"Base image build failed with exit code {exit_code}") + + self._move_distro_file("usb", "iso") + self._move_distro_file("pxe", "tar") + self._move_distro_file("onie", "bin") + + logger.info("Finished base OS image build") + finally: + deps_tmpdir = self.image_builder_dir / "deps_staging" + if deps_tmpdir.exists(): + shutil.rmtree(deps_tmpdir) + logger.debug(f"Cleaned up temporary deps directory: {deps_tmpdir}") + + def _get_dependency_artifacts(self, component: str) -> dict[str, Path]: + """Get artifacts for all dependencies of a component. + + For array elements like 'bsps[0]', extracts base name 'bsps' to look up dependencies. + + Args: + component: Component name (e.g., 'kernel' or 'bsps[0]') + + Returns: + Dictionary mapping dependency names to their artifact paths + """ + # Extract base component name for array elements (e.g., 'bsps[0]' -> 'bsps') + base_component = component.split("[")[0] + dependencies = self.COMPONENTS.get(base_component, []) + return { + dep: self.component_artifacts[dep] + for dep in dependencies + if dep in self.component_artifacts + } + + def _ensure_dependencies_built(self, component: str): + """Ensure all dependencies for a component are built. + + For array elements like 'bsps[0]', extracts base name 'bsps' to look up dependencies. + + Args: + component: Component name (e.g., 'kernel' or 'bsps[0]') + + Raises: + ComponentError: If a dependency cannot be built + """ + # Extract base component name for array elements (e.g., 'bsps[0]' -> 'bsps') + base_component = component.split("[")[0] + dependencies = self.COMPONENTS.get(base_component, []) + + for dep in dependencies: + # Skip if dependency is already built + if dep in self.component_artifacts: + logger.debug(f"Dependency '{dep}' already built for '{component}'") + continue + + # Check if dependency exists in manifest + if not self.manifest.has_component(dep): + raise ComponentError( + f"Component '{component}' depends on '{dep}', but '{dep}' is not defined in the manifest" + ) + + # Build the dependency + logger.info(f"Building dependency '{dep}' for '{component}'") + self._build_component(dep) + + def _build_component(self, component: str): + """Build a specific component by delegating to component builder.""" + logger.info(f"Building: {component}") + + component_data = self.manifest.get_component(component) + + # Check if component is an array - if so, build each element + if isinstance(component_data, list): + artifact_paths = [] + for idx, element_data in enumerate(component_data): + element_name = f"{component}[{idx}]" + logger.info(f"Building: {element_name}") + + # Ensure dependencies are built before building this array element + self._ensure_dependencies_built(element_name) + + # Get dependency artifacts to pass to the builder + dependency_artifacts = self._get_dependency_artifacts(element_name) + + # Create a ComponentBuilder for this array element + component_builder = self._create_component_builder( + element_name, element_data, dependency_artifacts + ) + artifact_path = component_builder.build() + + # Compress artifact if needed (happens on host, outside container) + # Skip compression for downloaded artifacts - use them as-is + if artifact_path: + if "download" not in element_data: + artifact_path = self._compress_artifact( + artifact_path, element_name + ) + artifact_paths.append(artifact_path) + + self.component_artifacts[component] = ( + artifact_paths if artifact_paths else None + ) + return + + # Ensure dependencies are built before building this component + self._ensure_dependencies_built(component) + + # Get dependency artifacts to pass to the builder + dependency_artifacts = self._get_dependency_artifacts(component) + + # Create the component builder and build it + component_builder = self._create_component_builder( + component, component_data, dependency_artifacts + ) + artifact_path = component_builder.build() + + # Compress artifact if needed (happens on host, outside container) + # Skip compression for downloaded artifacts - use them as-is + if artifact_path and "download" not in component_data: + artifact_path = self._compress_artifact(artifact_path, component) + + self.component_artifacts[component] = artifact_path diff --git a/fboss-image/distro_cli/cmds/build.py b/fboss-image/distro_cli/cmds/build.py index b3fd8db0994d0..ae86ad72bde57 100644 --- a/fboss-image/distro_cli/cmds/build.py +++ b/fboss-image/distro_cli/cmds/build.py @@ -9,9 +9,9 @@ from pathlib import Path -from lib.builder import ImageBuilder -from lib.cli import validate_path -from lib.manifest import ImageManifest +from distro_cli.builder.image_builder import ImageBuilder +from distro_cli.lib.cli import validate_path +from distro_cli.lib.manifest import ImageManifest def build_command(args): diff --git a/fboss-image/distro_cli/lib/docker/image.py b/fboss-image/distro_cli/lib/docker/image.py index cb746a3878a8b..bfd913a195f9e 100644 --- a/fboss-image/distro_cli/lib/docker/image.py +++ b/fboss-image/distro_cli/lib/docker/image.py @@ -23,9 +23,6 @@ # Default cache expiration time in seconds (24 hours) DEFAULT_CACHE_EXPIRATION_SECONDS = 24 * 60 * 60 -# Container registry for caching builder images across ephemeral VMs -CONTAINER_REGISTRY = "container-registry.sw.internal.nexthop.ai" - def _hash_directory_tree( directory: Path, exclude_patterns: list[str] | None = None @@ -141,95 +138,6 @@ def _get_image_build_timestamp(image_tag: str) -> int | None: return None -def _try_pull_from_registry(checksum: str) -> bool: - """Try to pull the builder image from the container registry. - - Args: - checksum: The checksum tag to pull - - Returns: - True if pull succeeded and image is now available locally, False otherwise - """ - registry_tag = f"{CONTAINER_REGISTRY}/{FBOSS_BUILDER_IMAGE}:{checksum}" - local_tag = f"{FBOSS_BUILDER_IMAGE}:{checksum}" - - logger.info( - f"Trying to pull {FBOSS_BUILDER_IMAGE}:{checksum[:12]} from registry..." - ) - - try: - result = subprocess.run( - ["docker", "pull", registry_tag], - capture_output=True, - text=True, - check=False, - ) - if result.returncode != 0: - logger.debug(f"Pull failed: {result.stderr}") - return False - - # Tag as local image - subprocess.run( - ["docker", "tag", registry_tag, local_tag], - check=True, - ) - subprocess.run( - ["docker", "tag", registry_tag, f"{FBOSS_BUILDER_IMAGE}:latest"], - check=True, - ) - - logger.info( - f"Successfully pulled {FBOSS_BUILDER_IMAGE}:{checksum[:12]} from registry" - ) - return True - - except (subprocess.CalledProcessError, FileNotFoundError) as e: - logger.debug(f"Failed to pull from registry: {e}") - return False - - -def _push_to_registry(checksum: str) -> None: - """Push the builder image to the container registry. - - Pushes fboss_builder:latest (the actual built image) with the checksum tag. - We don't push the timestamp-labeled image because that has an extra layer - and we want to cache the exact image that was built. - - Args: - checksum: The checksum tag to use in the registry - """ - # Push the :latest image (the actual build output) with the checksum tag - local_tag = f"{FBOSS_BUILDER_IMAGE}:latest" - registry_tag = f"{CONTAINER_REGISTRY}/{FBOSS_BUILDER_IMAGE}:{checksum}" - - logger.info(f"Pushing {FBOSS_BUILDER_IMAGE}:{checksum[:12]} to registry...") - - try: - # Tag for registry - subprocess.run( - ["docker", "tag", local_tag, registry_tag], - check=True, - ) - - # Push to registry - result = subprocess.run( - ["docker", "push", registry_tag], - capture_output=True, - text=True, - check=False, - ) - if result.returncode != 0: - logger.warning(f"Failed to push to registry: {result.stderr}") - return - - logger.info( - f"Successfully pushed {FBOSS_BUILDER_IMAGE}:{checksum[:12]} to registry" - ) - - except (subprocess.CalledProcessError, FileNotFoundError) as e: - logger.warning(f"Failed to push to registry: {e}") - - def _should_build_image(root_dir: Path) -> tuple[bool, str, str]: """Determine if the fboss_builder image should be rebuilt. @@ -271,8 +179,7 @@ def build_fboss_builder_image() -> None: Uses a three-tier caching strategy: 1. Local cache: Check if image with checksum tag exists locally - 2. Registry cache: Try to pull from container registry if not local - 3. Build: Build the image if not found in either cache, then push to registry + 2. Build: Build the image if not found in either cache Time-based expiration: Even if checksum matches, rebuilds if image is older than the expiration time (default: 24 hours, configurable via @@ -303,19 +210,6 @@ def build_fboss_builder_image() -> None: ) return - # Image not in local cache - try pulling from registry - if _try_pull_from_registry(checksum): - logger.info( - f"{FBOSS_BUILDER_IMAGE} image with checksum {checksum[:12]} " - f"pulled from registry, skipping build" - ) - return - - logger.info( - f"{FBOSS_BUILDER_IMAGE} image with checksum {checksum[:12]} " - f"{reason} (not in registry either), building" - ) - # Build the image logger.info(f"Building {FBOSS_BUILDER_IMAGE} image...") checksum_tag = f"{FBOSS_BUILDER_IMAGE}:{checksum}" @@ -360,8 +254,5 @@ def build_fboss_builder_image() -> None: logger.info(f"Tagged image with checksum: {checksum[:12]}") - # Push to registry for future builds on other VMs - _push_to_registry(checksum) - except subprocess.CalledProcessError as e: raise RuntimeError(f"Failed to build {FBOSS_BUILDER_IMAGE} image: {e}") from e diff --git a/fboss-image/distro_cli/tests/build_test.py b/fboss-image/distro_cli/tests/build_test.py index b090164e22307..9d633d9c74e76 100644 --- a/fboss-image/distro_cli/tests/build_test.py +++ b/fboss-image/distro_cli/tests/build_test.py @@ -15,17 +15,15 @@ """ import argparse -import sys import unittest from pathlib import Path -from unittest.mock import MagicMock, patch -# Add parent directory to path to import the modules -test_dir = Path(__file__).parent -cli_dir = test_dir.parent -sys.path.insert(0, str(cli_dir)) - -from cmds.build import build_command, setup_build_command +from distro_cli.cmds.build import build_command, setup_build_command +from distro_cli.tests.test_helpers import ( + ensure_test_docker_image, + enter_tempdir, + override_artifact_store_dir, +) class TestBuildCommand(unittest.TestCase): @@ -33,38 +31,42 @@ class TestBuildCommand(unittest.TestCase): def setUp(self): """Set up test fixtures""" - self.test_dir = Path(__file__).parent + self.test_dir = Path(__file__).parent / "data" self.manifest_path = self.test_dir / "dev_image.json" + ensure_test_docker_image() def test_build_command_exists(self): """Test that build command exists""" self.assertTrue(callable(build_command)) self.assertTrue(callable(setup_build_command)) - @patch("lib.builder.subprocess.run") - @patch("lib.builder.Path.exists") - @patch("lib.builder.shutil.move") - def test_build_all_stub(self, _mock_move, mock_exists, mock_run): - """Test build command with no components (build all)""" - # Mock the build script and output ISO existence - mock_exists.return_value = True - mock_run.return_value = MagicMock(returncode=0) - - # Create mock args object - args = argparse.Namespace(manifest=str(self.manifest_path), components=[]) - # Call build command - just verify it doesn't crash - # When implemented, should verify full image build - build_command(args) - - def test_build_specific_components_stub(self): - """Test build command with specific components""" - # Create mock args object - args = argparse.Namespace( - manifest=str(self.manifest_path), components=["kernel", "sai"] - ) - # Call build command with components - just verify it doesn't crash - # When implemented, should verify component-specific builds - build_command(args) + def test_build_components(self): + """Test build command with specific components using stub manifest.""" + with enter_tempdir( + "build_test_artifacts_" + ) as temp_artifacts, override_artifact_store_dir(temp_artifacts): + manifest_path = self.test_dir / "test-stub-component.json" + + args = argparse.Namespace( + manifest=str(manifest_path), + components=["kernel"], + kiwi_ng_debug=False, + ) + + build_command(args) + + # Verify the artifact was created in the artifact store + self.assertTrue( + temp_artifacts.exists(), + f"Artifacts directory not found: {temp_artifacts}", + ) + + # Find the artifact in the store (supports both .tar and .tar.zst) + matching_files = list(temp_artifacts.glob("*/data/kernel-test.rpms.tar*")) + self.assertTrue( + len(matching_files) > 0, + f"Expected artifact file not found in artifact store: {temp_artifacts}", + ) if __name__ == "__main__": diff --git a/fboss-image/distro_cli/tests/test_compression.py b/fboss-image/distro_cli/tests/test_compression.py new file mode 100644 index 0000000000000..2513cfb429956 --- /dev/null +++ b/fboss-image/distro_cli/tests/test_compression.py @@ -0,0 +1,68 @@ +#!/usr/bin/env python3 +"""Tests for compression functionality in ImageBuilder.""" + +import tempfile +import unittest +from pathlib import Path +from unittest.mock import Mock + +from distro_cli.builder.image_builder import ImageBuilder + + +class TestCompression(unittest.TestCase): + """Test compression functionality.""" + + def test_compress_artifact_skip_when_disabled(self): + """Test that compression is skipped when compress_artifacts=False.""" + with tempfile.TemporaryDirectory() as tmpdir: + tmpdir_path = Path(tmpdir) + + # Create a dummy .tar file + test_tar = tmpdir_path / "test-artifact.tar" + test_tar.write_text("dummy content for testing") + + # Create ImageBuilder with compression disabled + mock_manifest = Mock() + mock_manifest.manifest_dir = tmpdir_path + builder = ImageBuilder(mock_manifest) + builder.compress_artifacts = False + + # Try to compress (should be no-op) + result_path = builder._compress_artifact(test_tar, "test-component") + + # Verify original file is returned unchanged + self.assertEqual(result_path, test_tar) + self.assertTrue(test_tar.exists()) + + # Verify no .zst file was created + compressed_path = test_tar.with_suffix(".tar.zst") + self.assertFalse(compressed_path.exists()) + + def test_compress_artifact_when_enabled(self): + """Test that compression works when compress_artifacts=True.""" + with tempfile.TemporaryDirectory() as tmpdir: + tmpdir_path = Path(tmpdir) + + # Create a dummy .tar file + test_tar = tmpdir_path / "test-artifact.tar" + test_tar.write_text("dummy content for testing") + + # Create ImageBuilder with compression enabled + mock_manifest = Mock() + mock_manifest.manifest_dir = tmpdir_path + builder = ImageBuilder(mock_manifest) + builder.compress_artifacts = True + + # Compress the artifact + result_path = builder._compress_artifact(test_tar, "test-component") + + # Verify compressed file was created and returned + self.assertTrue(result_path.exists()) + self.assertTrue(result_path.name.endswith(".tar.zst")) + + # Verify original was removed + self.assertFalse(test_tar.exists()) + + +if __name__ == "__main__": + unittest.main() From 94e29dbd390997e7858bca9414a4d15a3e516501 Mon Sep 17 00:00:00 2001 From: Raghav Rao Date: Mon, 26 Jan 2026 05:59:55 +0000 Subject: [PATCH 11/12] [Nexthop] Add build forwarding and platform stack support Add scripts and build configuration for building FBOSS forwarding and platform stacks. - CMakeLists.txt: Defines build targets for forwarding stack - package.py: Script for packaging forwarding and platform stacks - build_fboss_stack.sh: Script for building forwarding and platform stacks --- CMakeLists.txt | 24 +++ fboss/oss/scripts/build_fboss_stack.sh | 230 +++++++++++++++++++++++++ 2 files changed, 254 insertions(+) create mode 100755 fboss/oss/scripts/build_fboss_stack.sh diff --git a/CMakeLists.txt b/CMakeLists.txt index b377bb2d1b896..d25fa7820174f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1083,6 +1083,30 @@ add_dependencies(fboss2_targets fboss2-dev ) +if (SAI_IMPL OR BUILD_SAI_FAKE) + add_custom_target(fboss_forwarding_stack) + # Common forwarding services + add_dependencies(fboss_forwarding_stack + diag_shell_client + fboss2 + fsdb + qsfp_service + wedge_qsfp_util + ) + + if (BUILD_SAI_FAKE) + add_dependencies(fboss_forwarding_stack + fboss_fake_agent_targets + ) + else() + add_dependencies(fboss_forwarding_stack + fboss_hw_agent-sai_impl + fboss_sw_agent + wedge_agent-sai_impl + ) + endif() +endif() + add_custom_target(fboss_other_services) add_dependencies(fboss_other_services qsfp_targets diff --git a/fboss/oss/scripts/build_fboss_stack.sh b/fboss/oss/scripts/build_fboss_stack.sh new file mode 100755 index 0000000000000..353408a847bbd --- /dev/null +++ b/fboss/oss/scripts/build_fboss_stack.sh @@ -0,0 +1,230 @@ +#!/usr/bin/env bash +# +# Build script for FBOSS forwarding and platform stacks +# This script builds the specified stack binaries inside a container. +# +# Prerequisites (handled by build_entrypoint.py): +# - SAI SDK extracted to /opt/sdk (from /deps/sai tarball) +# - Kernel RPMs installed (from /deps/kernel tarball, optional) +# +# This script: +# - Parses the requested stack type (forwarding or platform) +# - For forwarding, enables SAI/SDK handling via need_sai=1 +# - Detects SAI location at /opt/sdk (when need_sai=1) +# - Configures SAI environment variables (when need_sai=1) +# - Builds FBOSS dependencies +# - Builds the appropriate FBOSS CMake target +# - Packages artifacts into tarballs +# +set -euxo pipefail + +if [ "$#" -lt 1 ]; then + echo "Usage: $0 forwarding|platform" >&2 + exit 1 +fi + +stack_type="$1" + +need_sai=0 +stack_label="" +cmake_target="" +package_target="" + +case "$stack_type" in +forwarding) + need_sai=1 + stack_label="forwarding" + cmake_target="fboss_forwarding_stack" + package_target="forwarding-stack" + ;; +platform) + stack_label="platform" + cmake_target="fboss_platform_services" + package_target="platform-stack" + ;; +*) + echo "Unsupported stack type: $stack_type (only 'forwarding' and 'platform' are supported)" >&2 + exit 1 + ;; +esac + +OUT_DIR=/output + +BUILD_TYPE="${BUILD_TYPE:-MinSizeRel}" + +# Setup FBOSS build environment compatibility: +# build_entrypoint provides: /src (repo), /deps/*-extracted (dependencies) +# FBOSS build expects: /var/FBOSS/fboss (repo), /opt/sdk (SAI) +if [ -d "/src" ]; then + echo "Setting up symlinks for FBOSS build environment" + + # Link /var/FBOSS/fboss -> /src (the worktree root) + mkdir -p /var/FBOSS + rm -rf /var/FBOSS/fboss # Remove any stale symlink or directory + ln -sf /src /var/FBOSS/fboss + echo " Created: /var/FBOSS/fboss -> /src" + + if [ "$need_sai" -eq 1 ]; then + # Link /opt/sdk -> extracted SAI dependency + # build_entrypoint extracts /deps/sai to /deps/sai-extracted + if [ ! -e "/opt/sdk" ]; then + ln -sf /deps/sai-extracted /opt/sdk + echo " Created: /opt/sdk -> /deps/sai-extracted" + fi + fi +fi + +if [ "$need_sai" -eq 1 ]; then + # Look for SAI installation at /opt/sdk + if [ -d "/opt/sdk" ]; then + SAI_DIR="/opt/sdk" + else + echo "ERROR: No SAI found at /opt/sdk" + exit 1 + fi + echo "Found SAI at $SAI_DIR (installed by build_entrypoint.py)" + + # Source SAI build environment + source "$SAI_DIR/sai_build.env" + echo "SAI environment loaded from $SAI_DIR/sai_build.env" + + echo "Using SAI_SDK_VERSION=${SAI_SDK_VERSION:-N/A} for SAI_VERSION=${SAI_VERSION:-Unknown}" + + if [ -n "${BUILD_SAI_FAKE:-}" ]; then + sai_name="sai-fake" + elif [ -n "${SAI_BRCM_IMPL:-}" ]; then + sai_name="sai-bcm-${SAI_SDK_VERSION}" + else + sai_name="sai-unknown-${SAI_SDK_VERSION}" + fi + echo "Using SAI implementation: $sai_name" +fi + +# Use a scratch path for the CMake build tree. +# For forwarding we use /build (host-backed via distro_cli) so caches persist +# across container runs +scratch_root="/build" +if [ "$need_sai" -eq 1 ]; then + common_root="${scratch_root}/forwarding-stack/common" + build_dir="${scratch_root}/forwarding-stack/${sai_name}" +else + common_root="${scratch_root}/platform-stack/common" + build_dir="${scratch_root}/platform-stack" +fi +mkdir -p "$build_dir" + +common_options='--allow-system-packages' +common_options+=' --scratch-path '$build_dir +common_options+=' --src-dir .' +common_options+=' fboss' + +# Share download / repo / extracted caches across different types of builds +mkdir -p "${common_root}/downloads" +if [ ! -L "${build_dir}/downloads" ]; then + ln -s "${common_root}/downloads" "${build_dir}/downloads" +fi +mkdir -p "${common_root}/repos" +if [ ! -L "${build_dir}/repos" ]; then + ln -s "${common_root}/repos" "${build_dir}/repos" +fi +mkdir -p "${common_root}/extracted" +if [ ! -L "${build_dir}/extracted" ]; then + ln -s "${common_root}/extracted" "${build_dir}/extracted" +fi + +echo "Building FBOSS ${stack_label} stack" +echo "Output directory: $OUT_DIR" + +# Navigate to FBOSS source root +cd /var/FBOSS/fboss + +# Save the manifests because we must modify them, at a minimum to use the stable dependency hashes. +tar -cf manifests_snapshot.tar build +tar -xf fboss/oss/stable_commits/latest_stable_hashes.tar.gz + +if [ "$stack_type" = "forwarding" ]; then + # Setup SAI implementation + SAI_INCLUDE_PATH="$SAI_DIR/include" + echo "Using SAI include path for build-helper: $SAI_INCLUDE_PATH" + + SAI_IMPL_OUTPUT_DIR="$build_dir/sai_impl_output" + echo "Preparing SAI manifests and HTTP server via build-helper.py into $SAI_IMPL_OUTPUT_DIR" + + # This will: + # * Copy libsai_impl.a and headers into $SAI_IMPL_OUTPUT_DIR + # * Create libsai_impl.tar.gz + # * Rewrite libsai and sai_impl manifests + # * Ensure fboss manifest depends on sai_impl + # * Start a local http.server on port 8000 serving libsai_impl.tar.gz + # + # getdeps.py will then be able to download sai_impl from + # http://localhost:8000/libsai_impl.tar.gz using these manifests + if [ -z "${BUILD_SAI_FAKE:-}" ]; then + ./fboss/oss/scripts/build-helper.py \ + "$SAI_DIR/lib/libsai_impl.a" \ + "$SAI_INCLUDE_PATH" \ + "$SAI_IMPL_OUTPUT_DIR" \ + "$SAI_VERSION" + else + echo "BUILD_SAI_FAKE is set; skipping build-helper.py (no vendor SAI manifests)" + fi +elif [ "$stack_type" = "platform" ]; then + # For a platform-only build we do not need the vendor SAI implementation. + # Temporarily drop sai_impl from the fboss manifest so getdeps will not try + # to fetch it. The open-source SAI headers (libsai) remain in the manifest. + if grep -q '^[[:space:]]*sai_impl[[:space:]]*$' build/fbcode_builder/manifests/fboss; then + tmp_manifest="$(mktemp)" + sed '/^[[:space:]]*sai_impl[[:space:]]*$/d' \ + build/fbcode_builder/manifests/fboss >"$tmp_manifest" + mv "$tmp_manifest" build/fbcode_builder/manifests/fboss + echo "Temporarily removed sai_impl from fboss manifest for platform-only build" + fi +fi + +# Install system dependencies +echo "Installing system dependencies..." +time nice -n 10 ./fboss/oss/scripts/run-getdeps.py install-system-deps \ + --recursive \ + ${common_options} + +# Build dependencies +echo "Building FBOSS dependencies..." +time nice -n 10 ./fboss/oss/scripts/run-getdeps.py build \ + --only-deps \ + ${common_options} + +echo "Get deps SUCCESS" + +# Build FBOSS stack +echo "Building FBOSS ${stack_label} stack..." + +time nice -n 10 ./fboss/oss/scripts/run-getdeps.py build \ + --no-deps \ + --build-type "${BUILD_TYPE:-MinSizeRel}" \ + --cmake-target "$cmake_target" \ + ${common_options} + +echo "${cmake_target} Build SUCCESS" + +# Package the stack +# Note: package.py creates both .tar (production binaries) +# and -tests.tar (test binaries). We only ship the production tar. +echo "Packaging ${stack_label} stack..." +python3 /var/FBOSS/fboss/fboss/oss/scripts/package.py \ + --build-dir "$build_dir" \ + "$package_target" + +# Copy production artifact to output directory +# Tests are NOT included in production image +mkdir -p "$OUT_DIR" +mv "${package_target}.tar" "$OUT_DIR/" + +echo "FBOSS ${stack_label} stack build complete!" +echo "Production artifact:" +ls -lh "$OUT_DIR"/"${package_target}.tar" + +# Restore modified manifests if we took a snapshot earlier. +if [ -f manifests_snapshot.tar ]; then + tar -xf manifests_snapshot.tar + rm manifests_snapshot.tar +fi From 0fe575d060bdff3525304e0dae396e304e39091e Mon Sep 17 00:00:00 2001 From: Travis Brown Date: Thu, 5 Feb 2026 02:17:46 +0000 Subject: [PATCH 12/12] Add fboss2-dev CLI to Distro Image --- CMakeLists.txt | 1 + fboss/oss/scripts/package.py | 1 + 2 files changed, 2 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index d25fa7820174f..81f5397337f19 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1089,6 +1089,7 @@ if (SAI_IMPL OR BUILD_SAI_FAKE) add_dependencies(fboss_forwarding_stack diag_shell_client fboss2 + fboss2-dev fsdb qsfp_service wedge_qsfp_util diff --git a/fboss/oss/scripts/package.py b/fboss/oss/scripts/package.py index d1dcfc18897a4..730c4b676f2bb 100755 --- a/fboss/oss/scripts/package.py +++ b/fboss/oss/scripts/package.py @@ -23,6 +23,7 @@ FORWARDING_BINARIES = [ "diag_shell_client", "fboss2", + "fboss2-dev", "fboss_hw_agent-sai_impl", "fboss_sw_agent", "fsdb",