Skip to content

Basic AL4 support in CI#7972

Open
maxtropets wants to merge 7 commits into
microsoft:mainfrom
maxtropets:f/al4-wip
Open

Basic AL4 support in CI#7972
maxtropets wants to merge 7 commits into
microsoft:mainfrom
maxtropets:f/al4-wip

Conversation

@maxtropets

@maxtropets maxtropets commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Minimal AL4 support

  • run on demand with azure-linux-4 label
  • will also run nightly on main
  • currently builds with few deprecation warnings that are explicitly excluded from compiler werror-checks
    • fix is incompatible with the current compiler version, so I left warning as is until we switch to AL4 and then implement the straightforward fix
  • runs unit + e2e tests (currently except partitions)
    • instead of fixing on common code with macros/etc will build as is until fully switching to AL4

@maxtropets maxtropets self-assigned this Jun 25, 2026
@maxtropets maxtropets force-pushed the f/al4-wip branch 11 times, most recently from b606ef9 to e990329 Compare July 1, 2026 16:45
@maxtropets maxtropets changed the title [DRAFT] AL4 experiments Basic AL4 support in CI Jul 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 minimal Azure Linux 4 (AL4) coverage in CI (nightly on main and on-demand via the azure-linux-4 label) and makes a handful of small compatibility adjustments across C++, tests, and build tooling to get the codebase building and exercising unit/e2e tests on AL4.

Changes:

  • Add a new AL4 GitHub Actions workflow plus an AL4-specific dependency installation script.
  • Update a few code paths for AL4 toolchain/library differences (Arrow Parquet reader API, OpenSSL header deprecations, QUIC session member cleanup).
  • Minor test harness robustness tweaks (curl -m argument formatting, tolerate additional transient connection errors).

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md

Reviewed changes

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

Show a summary per file
File Description
.github/workflows/ci-al4.yml Adds AL4 CI workflow with VMSS jobs to build and run unit/e2e buckets, with artifact upload of logs.
scripts/setup-ci-al4.sh New AL4 bootstrap script installing build/test deps, Node, h2spec, and Python tooling (uv).
tests/perf-system/submitter/submit.cpp Adds Arrow version gating for Parquet reader creation (Arrow ≥ 19 API change).
tests/infra/clients.py Fixes curl timeout argument formatting by passing -m and its value as separate argv entries.
tests/connections.py Expands transient exception handling to include BrokenPipeError during polling.
src/quic/quic_session.h Removes redundant/shadowing session_id member in derived QUIC session type.
src/enclave/enclave.h Drops OpenSSL engine.h include to align with OpenSSL 3 deprecations.
include/ccf/crypto/openssl/openssl_wrappers.h Drops OpenSSL engine.h include from wrapper header.
src/crypto/openssl/ec_public_key.cpp Drops OpenSSL engine.h include.
src/crypto/openssl/ec_key_pair.cpp Drops OpenSSL engine.h include.
src/ds/test/openapi.cpp Adds std::hash<Baz> specialization to enable schema generation for std::unordered_set<Baz>.
src/crypto/openssl/hash.cpp Adjusts HKDF salt pointer handling for empty salts (review comment raised on semantics risk).
samples/apps/logging/logging.cpp Simplifies unauthenticated caller detection by removing an unused dynamic_cast result variable.

Comment thread src/crypto/openssl/hash.cpp
@maxtropets maxtropets marked this pull request as ready for review July 2, 2026 08:06
@maxtropets maxtropets requested a review from a team as a code owner July 2, 2026 08:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread scripts/setup-ci-al4.sh

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread src/ds/test/openapi.cpp
Comment thread src/ds/test/openapi.cpp
DECLARE_JSON_OPTIONAL_FIELDS(Baz, x, y);

namespace std
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am surprised we suddenly need this, do you know why?

// and 3.5 both pass the pointer and salt_size through to HKDF, so no salt
// bytes are read and the derived key is unchanged.
const uint8_t empty_salt = 0;
const auto* salt_data = salt.empty() ? &empty_salt : salt.data();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it possible for the span to have a null ptr despite being non empty?

Comment thread scripts/setup-ci-al4.sh

install_source_control() {
# Source control and tools used by this script.
tdnf -y install \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought azl4 used dnf rather than tdnf? If we are going to split the script anyway, we might as well avoid aliases?

Comment thread scripts/setup-ci-al4.sh
libstdc++-devel
# Azure Linux 4 beta does not publish libbacktrace-static yet; the Azure
# Linux 3.0 RPM contains only backtrace.h and libbacktrace.a and works here.
tdnf install -y https://packages.microsoft.com/azurelinux/3.0/prod/base/x86_64/Packages/l/libbacktrace-static-13.2.0-7.azl3.x86_64.rpm

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we request this asap, we've just added this as a dependency, and we do not want to have this hack for an indefinite period of time.

git config --global --add safe.directory /__w/CCF/CCF
mkdir build
cd build
cmake -GNinja -DCMAKE_BUILD_TYPE=Debug "-DCMAKE_CXX_FLAGS=-Wno-error=deprecated-declarations -Wno-error=#warnings" ..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This feels like it should be in cmake, maybe gated on compiler version detection, but definitely not inline in the CI, this will make it too easy for local runs to diverge from CI.


jobs:
vmss-virtual-a:
if: &check_trigger_conditions ${{ (github.event.action == 'labeled' && github.event.label.name == 'azure-linux-4') || (github.event.action != 'labeled' && contains(github.event.pull_request.labels.*.name, 'azure-linux-4')) || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems to me that if the CI passes, we ought to run it every time and not use labels. Happy to discuss.

@achamayou achamayou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few questions, but LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants