Skip to content

[ci]: Add buildenv_setup and cut the Build stage over to it (CI unification, Phase 1) - #1222

Merged
qiluo-msft merged 16 commits into
sonic-net:masterfrom
theasianpianist:ci-unification/phase1-buildenv-setup
Aug 2, 2026
Merged

[ci]: Add buildenv_setup and cut the Build stage over to it (CI unification, Phase 1)#1222
qiluo-msft merged 16 commits into
sonic-net:masterfrom
theasianpianist:ci-unification/phase1-buildenv-setup

Conversation

@theasianpianist

@theasianpianist theasianpianist commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Overview

First PR of the dataplane CI-unification effort (HLD: sonic-net/SONiC#2419):

  1. Introduces buildenv_setup — a shared, declarative build-environment tool
  2. Cuts sonic-swss-common's Build stage over to it, so the build environment is described once in build-env/ instead of being hand-maintained as inline pipeline steps. As part of the cutover, sonic-swss-common now builds and links
    against the SONiC-patched libnl everywhere (never apt stock), which is propagated to every stage/host
    that installs the resulting .debs.

What's added

ci/buildenv_setup/ — a self-contained Python package (schema / predicates / topo / apt_sources /
cascade / azp_client / installer / post_install / planner / cli). Given a repo's build-env/ config it
resolves the (cascaded) apt/pip/upstream-DEB dependency set, installs it (DEBs before pip), and runs
post_install hooks. Hosted here because sonic-swss-common is the root of the dependency cascade and is
already cloned by every consumer's CI. 109 unit tests run in a standalone BuildenvSetupUnitTests
pipeline stage.

build-env/ — sonic-swss-common's declarative config:

  • packages/base.yaml (cascading build+link deps + the canonical shared configure-redis-for-tests hook),
    packages/tooling.yaml (non-cascading Build-only tooling), upstream-artifacts.yaml (common-lib
    SONiC libnl + libyang3, sonic-yang wheels).
  • build.sh, Dockerfile, compose.yaml, README.md — let a developer reproduce the CI build
    environment locally (portable-local-dev goal).

What's cut over (Build stage)

Both jobs of the Build stage now set up their environment with a single buildenv_setup --scope build
call, replacing ~200 lines of inline apt-get install + DownloadPipelineArtifact@2 + dpkg/pip steps:

  • amd64/ubuntu-22.04 (vmImage) → new build-ubuntu-template.yml.
  • container build-template.yml — shared by Build (amd64), BuildArm (armhf/arm64) and
    BuildTrixie (amd64/armhf/arm64), so all six container build jobs are cut over. Compile is now
    ./build-env/build.sh; the shared configure-redis-for-tests.sh runs inline after cargo (redis must
    be down for the Rust logger_init_without_redis test). Container image tag ($(BUILD_BRANCH)),
    coverage flags, artifact names and the unit-test/gcov steps are unchanged, so stage outputs and
    downstream contracts are identical
    .

Scope / safety

  • The ci/ tool is dormant except where explicitly invoked; the cutover keeps every stage's artifacts,
    names and downstream contracts identical
    , so consumers are unaffected.
  • No cross-repo @resource reuse or dependency-cascade cutover yet — those are later migration PRs, and
    there are no downstream template changes here (building against stock libnl keeps the
    libswsscommon deb's dependencies satisfied everywhere).

Validation

  • 109 buildenv_setup unit tests pass locally and in the BuildenvSetupUnitTests stage; diff-coverage 96%.
  • CI confirmed green end-to-end (build 1164874): the cut-over amd64/ubuntu-22.04 + container
    Build/BuildArm/BuildTrixie jobs build against stock apt libnl and pass compile + unit tests (no
    continueOnError); BuildSairedis/BuildSwss/BuildDocker/Test all green.
  • The cutover mechanism (dependency cascade, current-run staging, template reuse) was proven end-to-end on
    the PoC branches across amd64/arm/trixie + VS tests.

Downstream validation (proof)

To prove this migration does not break downstream consumer CI, two draft PRs pin their swss-common
download to this PR's build (1164874) and build/test against it:

Both pin every swss-common download to this PR's fully-green build 1164874 and are pins-only (no libnl companion, since swss-common builds against stock libnl). Every build stage and the VS Test stage passed on both drafts (all tasks succeeded); the overall partiallySucceeded is warning-level noise in the ASAN-only stages, unrelated to the swss-common change. DO NOT MERGE — the
pins revert before those repos merge anything.

How to review

ci/README.mdci/buildenv_setup/cli.py (arg surface) → schema.py (config contract) → cascade.py
(dependency walk) → planner.py/installer.py (install plan); then build-env/ (the declarative config)
and the .azure-pipelines/*.yml cutover diffs.

First PR of the dataplane CI-unification effort (HLD: sonic-net/SONiC#2419). Adds the
buildenv_setup Python package under ci/ -- the shared tool that sets up the build/test
environment for sonic-swss-common, sonic-sairedis, and sonic-swss by reading each repo's
declarative build-env/ config, resolving the (cascaded) apt/pip/upstream-DEB dependency
set, installing it, and running post_install hooks.

Purely additive and dormant: nothing in any existing pipeline references ci/ yet
(consumers are cut over stage-by-stage in later PRs), so CI behavior is unchanged.

- ci/buildenv_setup/ -- multi-module package (schema/predicates/cascade/azp_client/
  installer/post_install/planner/cli/topo/apt_sources/model).
- ci/tests/ -- 39 unit tests (fail-loud schema, cascade, predicates, topo, planner).
- ci/README.md -- overview + layout.

Hosted here because sonic-swss-common is the root of the dependency cascade and is
already checked out by every consumer's CI; versioned with its branches.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
Copilot AI review requested due to automatic review settings July 9, 2026 16:13
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a new, shared Python tool (ci/buildenv_setup) intended to unify build/test environment setup across SONiC dataplane repositories by resolving and executing install plans from declarative build-env/ YAML configuration. It is currently additive-only and not wired into existing pipelines yet.

Changes:

  • Added ci/buildenv_setup/ package implementing schema validation, predicate evaluation, upstream artifact cascade resolution, install planning, and a CLI entrypoint.
  • Added a pytest suite under ci/tests/ covering schema validation, predicates, topo ordering, cascade resolution, and planner behaviors.
  • Added ci/README.md documenting purpose, layout, schema evolution policy, and test invocation.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
ci/README.md Documents the new CI tooling package, its scope, and how to run tests.
ci/.gitignore Ignores Python/pytest cache artifacts under ci/.
ci/buildenv_setup/init.py Declares package metadata and version.
ci/buildenv_setup/main.py Enables python3 -m buildenv_setup execution.
ci/buildenv_setup/apt_sources.py Generates commands to register third-party APT sources.
ci/buildenv_setup/azp_client.py Implements Azure DevOps artifact resolution/download/extraction.
ci/buildenv_setup/cascade.py Resolves upstream-artifacts config and recursively collects bundle artifacts.
ci/buildenv_setup/cli.py Provides CLI surface for configuring and running the tool.
ci/buildenv_setup/installer.py Centralizes apt/pip/dpkg/script execution primitives.
ci/buildenv_setup/model.py Defines dataclasses representing parsed config and runtime context.
ci/buildenv_setup/planner.py Orchestrates plan resolution and execution ordering (apt → deb → pip → post_install).
ci/buildenv_setup/post_install.py Selects/dedups/resolves post_install hooks and their scripts.
ci/buildenv_setup/predicates.py Evaluates when: predicates and performs {var} substitutions.
ci/buildenv_setup/schema.py Loads YAML and enforces additive schema with fail-loud unknown-field rejection.
ci/buildenv_setup/topo.py Provides stable topological sorting for requires: ordering.
ci/tests/conftest.py Ensures buildenv_setup is importable when running pytest.
ci/tests/test_cascade.py Validates upstream resolution, staged bundle collection, and nested cascade behavior.
ci/tests/test_planner.py Validates package selection, batching, and DEB grouping behaviors.
ci/tests/test_predicates.py Validates predicate matching and substitution behavior.
ci/tests/test_schema.py Validates schema parsing and fail-loud behavior for invalid inputs.
ci/tests/test_topo.py Validates stable topo ordering and cycle detection.

Comment thread ci/buildenv_setup/installer.py Outdated
Comment thread ci/buildenv_setup/installer.py
Comment thread ci/buildenv_setup/azp_client.py Outdated
Comment thread ci/buildenv_setup/schema.py
Comment thread ci/buildenv_setup/schema.py
Comment thread ci/buildenv_setup/planner.py Outdated
Comment thread ci/buildenv_setup/cascade.py
Comment thread ci/buildenv_setup/apt_sources.py
Add a standalone pipeline stage (dependsOn: []) that installs pytest/pyyaml/requests
and runs the ci/buildenv_setup unit tests, publishing JUnit results. Gives the tool
CI coverage from day one. The stage runs in parallel and does not gate the existing
Build/BuildDocker/Test stages (they don't depend on it), while its own pass/fail still
contributes to the overall pipeline result.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

theasianpianist and others added 2 commits July 9, 2026 18:56
…les; log fix-broken dpkg

Two buildenv_setup refinements needed by the cross-repo build-env cascade:

1. post_install.resolve_script falls back to cascaded upstream build-env/ dirs
   (by basename) when a source: script is not present locally. This lets a
   consumer reuse a hook the upstream bundle already provides, so a shared script
   (e.g. configure-redis-for-tests.sh) lives in exactly one repo -- the cascade
   root -- instead of being duplicated. planner passes the cascaded build-env dirs.

2. Executor.dpkg_install logs the '+ <cmd>' line on the apt_fix_broken path too
   (it previously ran subprocess.run directly, hiding the command + any per-upstream
   install_env such as vpp's VPP_INSTALL_SKIP_SYSCTL=1).

Adds tests/test_post_install.py (local-wins / upstream-fallback / not-found /
scope-filter dedup) and tests/test_installer.py (env prefix present in dpkg argv
on both the plain and apt_fix_broken paths).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
…env_setup

First real cutover of the CI-unification effort. Introduces sonic-swss-common's
declarative build-env/ configuration and switches the Build stage's amd64/ubuntu-22.04
job to set up its environment via buildenv_setup instead of ~90 lines of inline
apt-install + DownloadPipelineArtifact steps.

build-env/:
- packages/base.yaml   -- cascading build+link deps (libhiredis-dev, swig, redis-server,
  pip libyang) + the canonical, shared configure-redis-for-tests post_install hook.
- packages/tooling.yaml -- non-cascading, Build-only tooling (the jammy vmImage toolchain,
  gtest/gmock, pytest, Pympler) + mkdir-yang-models.
- upstream-artifacts.yaml -- common-libs (SONiC-patched libnl + libyang3 DEBs) and
  sonic-buildimage-vs (sonic-yang wheels), resolved on $(BUILD_BRANCH).
- configure-redis-for-tests.sh, build.sh, Dockerfile, compose.yaml, README.md
  (build.sh/Dockerfile/compose enable local reproduction of the CI build env -- Goal 3).

Cutover (.azure-pipelines/build-ubuntu-template.yml, azure-pipelines.yml):
- The inline amd64/ubuntu-22.04 job becomes a build-ubuntu-template.yml reference whose
  setup step runs buildenv_setup --scope build --host-os ubuntu-22.04. Compile / Bazel /
  archive are unchanged; it still publishes sonic-swss-common.amd64.ubuntu22_04 (same
  artifact the Test stage consumes), so no downstream contract changes.
- Only this job changes. The shared build-template.yml (container amd64 + BuildArm +
  BuildTrixie) and all downstream stages are untouched.

Behavior delta vs the old inline job: libnl now comes from common-libs (SONiC-patched,
consistent with the container builds) rather than apt stock; the bogus 'python' (py2)
and redundant 'sudo' installs are dropped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@theasianpianist theasianpianist changed the title [ci]: Add buildenv_setup shared build-environment tool (CI unification, Phase 1) [ci]: Add buildenv_setup + cut the ubuntu-22.04 Build job over to it (CI unification, Phase 1) Jul 9, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment thread ci/tests/test_installer.py Fixed
The ubuntu-22.04 Build job now builds libswsscommon against the SONiC-patched libnl
from common-libs (3.7.0-0.2+b1sonicN), so its dpkg Depends is >= +b1sonicN. The VS
Test stage installs that .deb on the jammy sonictest host, whose apt only has stock
libnl (+b1) -- which does NOT satisfy +b1sonicN, so 'dpkg -i libswsscommon || apt-get
install -f' removed libswsscommon and python3-swsscommon then failed.

Fix (consistent with the repo policy of always using SONiC libnl, never apt stock):
download the SONiC libnl runtime debs (libnl-3-200/genl/route/nf) from the common-lib
artifact -- already fetched here for libyang -- and apt-install them before the
swss-common deb. Mirrors how the build stages obtain libnl.

Companion to the ubuntu-22.04 Build cutover; the Test stage is otherwise unchanged
(full --scope test cutover to buildenv_setup is a later PR).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

theasianpianist and others added 2 commits July 9, 2026 23:39
Resolves the automated review findings on PR sonic-net#1222:

- installer: run_script now uses 'bash -e -o pipefail' so a failure anywhere in a
  pipeline (curl | gpg | tee) fails the command instead of being masked. dpkg_install's
  apt_fix_broken path routes through a shared _exec() so the command is logged
  consistently (no direct subprocess.run).
- azp_client: extract downloaded artifact zips via _safe_extractall(), rejecting
  members whose path escapes the target dir (zip-slip / path traversal).
- schema: fail loudly when list fields (pip_args/requires/dpkg_args/scopes/result_filter)
  are not lists and install_env is not a mapping -- previously list('string') silently
  produced a per-character list. Adds _as_str_list/_as_str_map validators.
- planner: _deb_install_groups keeps empty-install_env group first but otherwise
  preserves artifact insertion order (was sorted by env-var count, which could reorder
  inter-dependent special-env groups). register_commands is passed use_sudo so --no-sudo
  is honoured for apt-source registration.
- cascade: thread the used-staged accumulator explicitly through the recursion (drop the
  collect_bundles._used module-attr hack) so a staged upstream referenced only via a
  nested bundle isn't wrongly reported as unused.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
The diff-coverage gate (threshold 80%) reported 74% because the new buildenv_setup
modules were under-tested (azp_client 29%, planner 38%, installer 58%, schema 79%).
Add unit tests to bring every module well above threshold (total 60% -> 93%, 109 tests):

- test_azp_client.py (new): fake requests.Session -- auth modes, _get retry/backoff,
  definition/build/artifact resolution + not-found errors, _content_root variants,
  zip-slip guard (accept nested / reject ../), fetch_artifact end-to-end.
- test_planner_run.py (new): planner.run() with a fake Executor + staged upstream --
  dry-run rendering, real apt/dpkg/pip/post_install sequence, no-build-env error,
  apt-source selection.
- test_cli.py (new): arg parser defaults/required, main() dry-run success + error exit code.
- test_apt_sources.py (new): register_commands with/without sudo.
- test_installer.py: rewritten (single import style -- fixes the CodeQL import finding) +
  full Executor primitive coverage incl. the apt_fix_broken fallback.
- test_schema.py / test_cascade.py / test_planner.py: added cases for the new list/map
  validation, the used-staged recursion fix, and deb-group insertion ordering.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment thread ci/tests/test_azp_client.py Fixed
Drop the mixed 'import buildenv_setup.azp_client as azp' + 'from ... import' in
test_azp_client.py (same CodeQL finding as test_installer.py). Keep only the
from-import and use the string-target form for the time.sleep monkeypatch.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

…ONiC libnl

Cut the shared container build job (build-template.yml) over to buildenv_setup, the
same mechanism the amd64/ubuntu-22.04 job already uses. This template is instantiated
by the Build (amd64), BuildArm (armhf/arm64) and BuildTrixie (amd64/armhf/arm64)
stages, so all six container build jobs now set up their environment via the tool.

- Replaces the inline 'apt-get install ... libnl-3-dev libnl-genl-3-dev libnl-route-3-dev
  libnl-nf-3-dev ...' (apt-stock libnl) + the DownloadPipelineArtifact@2 libyang/yang-wheel
  steps + inline dpkg/pip with a single 'buildenv_setup --scope build' invocation driven by
  build-env/. libnl now comes from common-lib (SONiC-patched, +b1sonicN) for every arch,
  matching the ubuntu-22.04 job and the VS test host -- the repo policy of always using
  SONiC libnl, never apt stock.
- Compile is now ./build-env/build.sh (single source of truth). The unit-test step runs the
  shared configure-redis-for-tests.sh inline after cargo (redis must be down for the Rust
  logger_init_without_redis test), and yang-models mkdir moves to the tool's post_install.
- Keeps the sonic-slave container, $(BUILD_BRANCH) image tag, coverage flags, artifact
  names and the unit-test/gcov steps unchanged, so stage outputs and downstream contracts
  are identical.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

The container Build cutover makes libswsscommon Depend on the SONiC-patched libnl
(>= 3.7.0-0.2+b1sonicN). BuildSairedis installs that libswsscommon .deb but its
sonic-slave container had only apt-stock libnl (+b1), so 'dpkg -i libswsscommon'
failed with unmet libnl-3-200/libnl-nf-3-200/libnl-route-3-200 (>= +b1sonicN) deps.

Fix (consistent with the always-SONiC-libnl policy and build-swss-template.yml):
drop the apt-stock libnl-*-dev install and download the full SONiC libnl family
(runtime + -dev, used to build sonic-sairedis too) from the common-lib artifact,
installed before the swss-common .deb.

BuildSwss already downloads+installs SONiC libnl with the swss-common deb (no
change). BuildDocker installs into the SONiC docker-sonic-vs base image, which
already carries SONiC libnl.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
Comment thread build-env/packages/tooling.yaml Outdated
@theasianpianist theasianpianist changed the title [ci]: Add buildenv_setup and cut the Build stage over to it + SONiC libnl (CI unification, Phase 1) [ci]: Add buildenv_setup and cut the Build stage over to it (CI unification, Phase 1) Jul 16, 2026
The sonic-slave build containers already provide Pympler and pytest
(python3-pympler / python3-pytest), so installing them via pip in
build-env/packages/tooling.yaml is redundant. The Ubuntu 22.04 job runs
the Bazel test suite and does not invoke pytest, so it does not need them
either. Addresses review feedback from @saiarcot895 on PR sonic-net#1222.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 19e0b615-271d-48f0-98cc-7ad6905c7f9e
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Comment thread ci/buildenv_setup/planner.py
Comment thread ci/buildenv_setup/planner.py Outdated
Two review comments from @qiluo-msft on buildenv_setup/planner.py:

1. _collect_cascaded_config now fails loud (PlannerError) if a cascaded
   base.yaml declares apt_sources or a cascaded package references an
   apt_source. The cascade only propagates packages + post_install, and
   apt sources are registered from local files only, so a cascaded
   apt_source would leave a package's source unregistered and break the
   subsequent apt-get install. Dormant today (no base.yaml uses
   apt_sources) but this keeps the base.yaml-cascades contract honest.

2. _pip_batches now preserves the requires:-toposort order across
   batches: consecutive no-args specs are grouped, but a pip_args spec is
   emitted in place (flushing the pending plain batch first), so a plain
   pip that requires an args pip (or vice versa) installs in dependency
   order. The previous "all plain first, args after" grouping discarded
   that order.

Adds tests for both (fail-loud on cascaded apt_source / apt_sources,
happy path, and pip batch ordering) and documents the cascade limitation
in build-env/README.md.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 19e0b615-271d-48f0-98cc-7ad6905c7f9e
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
Copilot AI review requested due to automatic review settings July 29, 2026 18:22
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Comment thread ci/tests/test_planner.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

.azure-pipelines/build-template.yml:69

  • The inline comment says buildenv_setup downloads/installs SONiC-patched libnl and runs the redis post_install hooks, but the repo’s build-env config explicitly keeps libnl as stock apt packages and the redis hook is test-scoped (Build runs it inline later). This comment is misleading for anyone debugging build dependencies.
      # It installs apt/pip deps, downloads + installs the SONiC-patched libnl +
      # libyang3 DEBs and the sonic-yang wheels (from upstream-artifacts.yaml), and
      # runs the redis / yang-models post-install hooks. libnl now comes from
      # common-lib (SONiC-patched), replacing the apt-stock libnl this job used to
      # install -- consistent with the ubuntu-22.04 job and the VS test host.

.azure-pipelines/build-ubuntu-template.yml:25

  • This comment states buildenv_setup installs SONiC libnl and runs the redis post_install hooks. In this repo’s build-env config, libnl stays as stock apt packages and the redis configure hook is test-scoped (Build runs it inline later). Updating the comment will avoid confusion when reproducing the jammy job locally.
      # installed on jammy, as before). It apt/pip-installs deps, downloads +
      # installs the SONiC libnl + libyang3 DEBs and the sonic-yang wheels, and runs
      # the redis / yang-models post-install hooks.

build-env/packages/tooling.yaml:45

  • The post_install hook uses sudo, but build-env/Dockerfile runs buildenv_setup with --no-sudo (intended for root/minimal environments). As written, local-dev builds can fail if sudo isn’t installed. Consider making the hook work with or without sudo by detecting it at runtime.
  - name: mkdir-yang-models
    script: |
      sudo mkdir -p /usr/local/yang-models
    scopes: [build]

CodeQL flagged the unused `import os` added in the previous commit's new
cascaded-config tests (they use the pytest tmp_path fixture, not os).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 19e0b615-271d-48f0-98cc-7ad6905c7f9e
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
Copilot AI review requested due to automatic review settings July 29, 2026 18:36
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

.azure-pipelines/build-template.yml:69

  • The comment claims buildenv_setup downloads/installs SONiC-patched libnl from common-libs, but this repo’s declarative config explicitly does the opposite (build-env/upstream-artifacts.yaml notes libnl is NOT downloaded; build-env/packages/base.yaml installs stock apt libnl). This mismatch also contradicts the PR description and will confuse future maintainers when debugging libnl linkage.
      # It installs apt/pip deps, downloads + installs the SONiC-patched libnl +
      # libyang3 DEBs and the sonic-yang wheels (from upstream-artifacts.yaml), and
      # runs the redis / yang-models post-install hooks. libnl now comes from
      # common-lib (SONiC-patched), replacing the apt-stock libnl this job used to
      # install -- consistent with the ubuntu-22.04 job and the VS test host.

.azure-pipelines/build-ubuntu-template.yml:25

  • This comment says the environment setup “downloads + installs the SONiC libnl + libyang3 DEBs”, but build-env/upstream-artifacts.yaml explicitly does not download libnl (it’s installed from stock apt via build-env/packages/base.yaml). Please adjust the comment to match the actual configuration to avoid misleading future updates.
      # installed on jammy, as before). It apt/pip-installs deps, downloads +
      # installs the SONiC libnl + libyang3 DEBs and the sonic-yang wheels, and runs
      # the redis / yang-models post-install hooks.

Comment thread ci/buildenv_setup/azp_client.py Outdated
…l hook

Addresses the remaining review feedback on PR sonic-net#1222:

- azp_client._safe_extractall: extract members individually and reject
  symlink members (S_IFLNK) in addition to absolute/../ paths, instead of
  validating then calling ZipFile.extractall(). CPython's zipfile does not
  materialise symlinks (a symlink-based zip-slip is not actually
  exploitable there — verified empirically), but per-member extraction with
  an explicit symlink reject makes the guarantee self-evident and robust.
  Adds test_safe_extractall_rejects_symlink_member.

- build-template.yml / build-ubuntu-template.yml: correct stale comments
  that still described SONiC-patched libnl. libnl is stock apt (base.yaml);
  only libyang3 + sonic-yang wheels come from common-lib; the redis hook is
  test-scoped (Build runs it inline / the jammy job runs Bazel).

- tooling.yaml mkdir-yang-models hook: make sudo optional at runtime.
  --no-sudo only strips the prefix from the tool's own apt/pip/dpkg
  commands, not inline hook scripts, so the local-dev Dockerfile (root,
  --no-sudo, possibly no sudo binary) could fail on `sudo mkdir`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 19e0b615-271d-48f0-98cc-7ad6905c7f9e
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
Copilot AI review requested due to automatic review settings July 29, 2026 21:25
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

build-env/packages/base.yaml:17

  • PR description claims the Build stage now uses SONiC-patched libnl “everywhere (never apt stock)”, but build-env/packages/base.yaml explicitly installs stock distro libnl-*-dev packages and documents that choice as intentional (“STOCK, not SONiC-patched”). This is a direct mismatch that will confuse reviewers and downstream consumers.

Please clarify the intended behavior and either (a) update the PR description to match the current implementation, or (b) change build-env/ to source SONiC-patched libnl instead of apt stock.

  # libnl: apt-installed from the distro (STOCK, not SONiC-patched). This keeps the
  # libswsscommon .deb's libnl Depends on the stock version, which is satisfied in
  # every build/test/docker environment without a SONiC-libnl companion step. These
  # are the same four -dev packages master's build-template.yml apt-installs; the
  # sonic-slave-* containers and the ubuntu-22.04 vmImage both provide them.

build-env/configure-redis-for-tests.sh:18

  • configure-redis-for-tests.sh hardcodes sudo for all operations. This breaks the documented --no-sudo/root-container use case (and mirrors the concern already handled in packages/tooling.yaml post_install hooks): if the hook runs as root in a minimal environment without sudo, it will fail even though no privilege escalation is needed.

Make the script sudo-optional by detecting whether we’re already root and whether sudo exists, then prefix commands conditionally.

sudo sed -i  's/notify-keyspace-events ""/notify-keyspace-events AKE/' /etc/redis/redis.conf
sudo sed -ri 's/^# unixsocket/unixsocket/'                             /etc/redis/redis.conf
sudo sed -ri 's/^unixsocketperm .../unixsocketperm 777/'              /etc/redis/redis.conf
sudo sed -ri 's/redis-server.sock/redis.sock/'                        /etc/redis/redis.conf
sudo service redis-server restart

@qiluo-msft
qiluo-msft merged commit 6acbb00 into sonic-net:master Aug 2, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants