feat(tensorflow): add TF Serving 2.20 inference DLC on AL2023 - #6243
Open
bhanutejagk wants to merge 71 commits into
Open
feat(tensorflow): add TF Serving 2.20 inference DLC on AL2023#6243bhanutejagk wants to merge 71 commits into
bhanutejagk wants to merge 71 commits into
Conversation
First TF inference DLC on the v2 (main) branch. Mirrors master's TF 2.19 inference image but ports to AL2023 + CUDA 12.9.1 + Python 3.12, switches to uv-driven dependency management, and rebuilds nginx-mod-njs from source (no AL2023 RPM exists). SageMaker only, x86 only, single-model + MME support preserved. - Dockerfile.cuda + Dockerfile.cpu with builder-njs source-build stage - Ports SageMaker handler scripts (Falcon + nginx + njs + multi_model_utils) from master TF 2.19 build_artifacts/sagemaker/ byte-for-byte - GitHub Actions workflows mirror PR #6107 (TF 2.21 training) shape - SageMaker integration tests for single-model and MME endpoints - ECR scan allowlist overlay at tensorflow/tensorflow-2.20.json TFS 2.20.0 binary copied from tensorflow/serving:2.20.0-devel-gpu image. tensorflow-serving-api installed with --no-deps to avoid pulling 600 MB of TF framework. framework: "tensorflow" + job_type: "inference" matches the cross-team release-logic convention (training and inference share the framework slug, distinguished by job_type). Known follow-ups (not blocking this PR): - PR #6107 (TF 2.21 training) will add tensorflow/framework_allowlist.json (shared base) once it merges. CI security-test may fail until then. - Separate change against the release-logic config needed to add tensorflow entry to frameworks.yml before image can release to prod.
added 10 commits
June 15, 2026 11:21
Run pre-commit hooks (ruff-format, ruff-check, requirements-txt-fixer, trailing-whitespace) on Phase 1-6 files. Mechanical reformatting plus noqa annotations for intentional E402 imports after gevent monkey-patch in python_service.py and one F841 unused variable in serve.py (preserved as-is from master TF 2.19 vendored handler).
Re-indent RUN/ENV continuation lines from 4-space to 2-space per dockerfmt's normalization. Mechanical reformatting only — no logic changes. Closes the dockerfmt CI failure on PR #6243.
The builder-njs stage's `tar xzf` invocation needs gzip, which AL2023's base image does not include by default. CI build failed at the nginx source extraction step on PR #6243. Add `gzip xz` to the existing dnf install line in both Dockerfile.cuda and Dockerfile.cpu builder-njs stages.
nginx ./configure --with-compat with the njs dynamic module auto-enables the HTTP XSLT module, which requires libxml2 and libxslt development headers. AL2023 base lacks these. Adding to the existing dnf install line in both Dockerfile.cpu and Dockerfile.cuda builder-njs stages.
nginx + njs `make modules` with the http-only --add-dynamic-module produces ngx_http_js_module.so but not the stream variant. SageMaker HTTP-based model serving uses only the http njs module (tensorflowServing.js routes /invocations and /ping). Removing the unused cp in builder-njs, the orphan COPY in runtime-base, and the comment reference in both Dockerfile.cuda and Dockerfile.cpu.
Training PR's sagemaker-test job avoids the test/conftest.py fabric import not by installing fabric, but by having test/tensorflow/pytest.ini cap pytest rootdir at test/tensorflow/. Mirror that mechanism here. - Add test/tensorflow/pytest.ini ([pytest] stanza) - Match training's requirements.txt content exactly (boto3, pytest, sagemaker>=3.0.0) at test/tensorflow/integration/inference/ - Both inference workflows now install via -r requirements.txt
CVE-2025-23339 and CVE-2025-23308 in cuda-toolkit-config-common 12.9.79. Fix requires CUDA 13 major version bump which is tracked as a separate currency update; CUDA 12.9 is pinned to match the AL2023 base image and parallel TF 2.21 training PR.
ecr_scan.py constructs the overlay path as <framework>/<framework>-<framework_version>.json. With framework_version=2.20.0 (full semver) the lookup is tensorflow-2.20.0.json, not the short tensorflow-2.20.json we previously committed. Rename to match so the two CUDA toolkit CVEs are actually picked up.
The PyPI sagemaker package v3.x removed the top-level sagemaker.Session class along with sagemaker.tensorflow.serving.TensorFlowModel and sagemaker.multidatamodel.MultiDataModel. Our conftest used the legacy v2 API. Rewrite to use boto3 directly (sagemaker, sagemaker-runtime, s3 clients) so tests work with the >=3.0.0 pin and are SDK-version independent. boto3 maps 1:1 to the create_model / create_endpoint_config / create_endpoint / invoke_endpoint flow these tests already needed, which is simpler than adopting v3's heavier ModelBuilder abstraction for integration tests. The sagemaker dep is dropped from requirements.txt since it is no longer imported.
Replaces commit 4b99d0f which used boto3 directly. The SageMaker Python SDK v3 is the supported entrypoint for these tests; v3 removed the v2 sagemaker.Session, sagemaker.tensorflow.serving.TensorFlowModel, and sagemaker.multidatamodel.MultiDataModel classes the old fixtures relied on. For DLC integration tests we already supply image_uri and a pre-built model.tar.gz, so ModelBuilder's auto-detection adds no value. We use the v3 sagemaker-core resource layer directly (the same surface ModelBuilder calls underneath): - conftest sagemaker_session fixture uses sagemaker.core.helper.session_helper.Session (default_bucket / upload_data); cleanup_endpoint uses Endpoint.get(...).delete(), EndpointConfig.get(...).delete(), Model.get(...).delete() - single-model test uses Model.create(primary_container=ContainerDefinition(...)) + EndpointConfig.create([ProductionVariant(...)]) + Endpoint.create() with endpoint.wait_for_status("InService") and endpoint.invoke(...) - MME test expresses the multi-model contract directly: ContainerDefinition(mode="MultiModel", model_data_url=<S3 prefix>) and endpoint.invoke(target_model="modelN.tar.gz") - sagemaker>=3.0.0 retained in requirements.txt
added 2 commits
July 24, 2026 12:40
Main refactored _reusable.{sanity,security,telemetry}-tests.yml to
accept only config-file and image-uri, deriving all other metadata
from the config-file's contents. Rewrite our 6 caller sites to pass
the new contract, mirroring main's own TF PR workflows.
bhanutejagk
marked this pull request as ready for review
July 24, 2026 23:33
.github/scripts/buildkitd.sh no longer exists on main after the CI refactor. Mirror .github/actions/build-image/action.yml's buildkitd setup step in both TF 2.20 inference PR workflows.
added 4 commits
July 24, 2026 17:09
Three coordinated fixes to the sagemaker-test job:
1. Delete test/tensorflow/pytest.ini. Its premise ("mirror training's
pytest rootdir cap") was wrong — training never had this file.
The file blocks test/conftest.py from loading, hiding the
--image-uri pytest addoption and breaking training's tests when
they piggyback on our checkout.
2. Install test/requirements.txt (which pulls in fabric) alongside
the inference-specific requirements. test/conftest.py transitively
imports fabric via test_utils.aws; without it pytest ImportErrors.
Mirrors training's tensorflow.tests-sagemaker.yml install pattern.
3. Standardize on TEST_IMAGE_URI env var (matches what the workflow
already sets, and matches PyTorch/openfold3/EFA SM conventions).
Rename INFERENCE_IMAGE_URI reads in the conftest. The bespoke
INFERENCE_IMAGE_URI name had no other users.
…ocation Main's PR #6250 refactor moved: - scripts/common/ -> scripts/docker/common/ - scripts/telemetry/ -> scripts/docker/telemetry/ Our Dockerfiles referenced the old paths — same class of merge fallout as the earlier .github/scripts/buildkitd.sh -> .github/actions/build-image/buildkitd.sh relocation. Update all 8 affected COPY lines (4 per Dockerfile).
…factor main
Three coordinated fixes to unblock sagemaker-test from silent-skip
behavior after the merge from main:
1. conftest reads SM_ROLE_ARN (matches training's tensorflow.tests-
sagemaker.yml and what our workflow already exports).
2. Workflow path triggers and paths-filter reference scripts/docker/
{common,telemetry}/** (post-PR-6250 relocation).
3. Image configs use post-refactor schema (image/metadata/build/
release blocks). Workflow yq lookups updated to .metadata.* and
.build.* accordingly. Old `common:` schema caused resolve-image-
uri to derive URI ending in /null and sanity/security/telemetry
to skip silently.
Migrate PR #6243 to match origin/main's post-refactor conventions: - Move image configs into .github/config/image/tensorflow/ subfolder - Move handler scripts to scripts/docker/tensorflow/inference/ - Adopt .github/actions/build-image composite (drops inlined buildx, labels, build-args, and downstream URI reconstruction) - Delete versions-*.env (pins now live in image config build: block) - Rename workflows to tensorflow.pr-2.20-inference-{cpu,cuda}.yml - Add tensorflow.pipeline-inference.yml as parallel reusable pipeline (inference-only sagemaker-test; no exit-5 tolerance) Aligns file layout with all other frameworks (pytorch, ray, sglang, etc.) and unlocks the shared build-arg / pre-build-hook mechanism.
added 7 commits
July 27, 2026 12:41
Our conftest fixture reads env var TEST_IMAGE_URI, but the pipeline
was passing --image-uri as a CLI flag. Neither was set → fixture
would pytest.skip("TEST_IMAGE_URI not set") → both tests silently
skipped and sagemaker-test reported green without asserting
anything.
Align workflow with conftest: set TEST_IMAGE_URI in step env,
drop --image-uri CLI flag. Matches pytorch.tests-sagemaker.yml
and openfold3.tests-sagemaker.yml peer convention.
Move the sagemaker-test job body from tensorflow.pipeline-inference.yml into a new tensorflow.tests-sagemaker-inference.yml reusable, matching the pattern used by every other framework (pytorch.tests-sagemaker.yml, openfold3.tests-sagemaker.yml, tensorflow.tests-sagemaker.yml). Pipeline becomes a thin caller. Cognitive load stays where it belongs: job orchestration in pipeline files, job bodies in test files.
- Add tensorflow.autorelease-2.20-inference-sagemaker.yml (workflow_dispatch only, no schedule). Mirrors training's autorelease shape. Set release.environment=gamma and release.public_registry=false in both inference configs so the release-gate/release jobs in pipeline-inference request the gamma GitHub environment (belt-and-suspenders: no public ECR push even if approved). - Drop DLC_MINOR_VERSION ARG and dlc_minor_version LABEL from both Dockerfiles. Matches training's TF 2.21 Dockerfile shape; the minor version LABEL always resolved to hardcoded 0 (config never provided it), so it was config-that-isn't-really-config. - Delete orphan .gitkeep in test/tensorflow/integration/inference/ (directory now has 7 tracked files).
The sagemaker-test on PR #6243 ran for the first time (after 631c772 fixed the silent-skip) and failed at test collection with: AttributeError: module 'tensorflow' has no attribute 'constant' Root cause: build_sample_model.py uses full TF APIs (tf.constant, tf.function, tf.saved_model.save) to construct the SavedModel tarball uploaded to S3 for the container to serve. That code runs on the CI runner, not inside the container. The runner installs test/tensorflow/integration/inference/requirements.txt only — which had no tensorflow entry. sagemaker>=3.0.0 pulls a tensorflow namespace stub (no ops), hence the AttributeError. The container itself deliberately does NOT ship full tensorflow (--no-deps on tensorflow-serving-api, matches master TF 2.19 inference convention). Runtime serving path uses the TFS C++ binary only. Only the test scaffolding needs full TF, and only on the runner. Add tensorflow>=2.20,<3.0 to test requirements. Runner gets full TF for sample-model construction; container is unchanged.
_reusable.sanity-tests.yml fires test_sanity_training.py for any framework=='tensorflow', which fails on tensorflow inference images that legitimately don't ship the training-cluster surface (SSH, MPI, EFA, /opt/ml, DLC_CONTAINER_TYPE=training). Add the job_type=='training' gate, mirroring the existing Ray precedent three lines below. TF 2.21 training (job_type=training) still runs the script; the new TF 2.20 inference (job_type=inference) correctly skips it, matching peer inference frameworks (openfold3, vllm_omni, ray inference, huggingface-vllm) that all rely on universal sanity checks plus their own tests-sagemaker reusables for real functional validation.
Two fixes that turn CI green: 1. njs dynamic module path (Dockerfile.cpu + Dockerfile.cuda) AL2023 nginx compiled with --prefix=/usr/share/nginx resolves relative `load_module modules/...` paths against that prefix. The njs .so was being copied to /usr/lib64/nginx/modules/ (the --modules-path default), which nginx never checks for a relative load_module directive. Result: nginx failed at parse, supervisor infinite-restart-looped, /ping health check never came up, SM endpoint deployment timed out. Fix: copy the module to /usr/share/nginx/modules/ where the relative directive resolves. nginx.conf.template stays byte-for-byte identical to the ported upstream toolkit. Discovered via CloudWatch logs from the failed endpoint on 92ba4bc CI run. 2. pyasn1 CVE bump (cpu/pyproject.toml + cuda/pyproject.toml) CVE-2026-59885, CVE-2026-59886 (HIGH, DoS) in pyasn1 0.6.3. Fixed in 0.6.4. Add direct pin, mirroring TF 2.21 training's solution in docker/tensorflow/2.21/{cpu,cuda}/pyproject.toml.
Fix 1 in commit 2abebaf moved ngx_http_js_module.so to the path nginx actually reads (/usr/share/nginx/modules/). Now nginx finds the module, but dlopen() cascades into a missing transitive dep: nginx: [emerg] dlopen() ".../ngx_http_js_module.so" failed (libxslt.so.1: cannot open shared object file) The njs module was compiled against libxslt-devel in builder-njs (added in 21de4f2), so it links against libxslt.so.1. The runtime image installs nginx but doesn't pull libxslt. Add libxslt to the runtime dnf install in both Dockerfiles. Discovered via CloudWatch logs from the ping-health-check failure on 2abebaf.
added 30 commits
July 29, 2026 23:11
Override the pipeline default (false) at the autorelease caller so the ECR CVE scan gates the gamma release path. PR-time pipeline keeps the platform-standard default.
Adds a small Conv2D SavedModel builder and a GPU-only integration test that deploys the model and issues a real prediction. Guards against silent cuDNN ABI drift that ldd + ldconfig -p checks miss. Model: Conv2D(4,k=3) -> GlobalAveragePooling2D -> Dense(1), 117 params. Skip mechanism: module-level pytestmark on SM_DEVICE_TYPE != "gpu".
Add Falcon TestClient unit tests for python_service handlers that
aren't reachable from outside a SageMaker MME container:
- on_delete: unload + reload cycle
- on_get: loaded model status + 404 for missing
- traversal-guard: 8 rejection cases (parent path, absolute, NUL,
dot, dot-dot, hidden prefix, etc.)
Also add a parametrized SageMaker integration test asserting the
traversal guard rejects malicious target_model values end-to-end.
Ruff v0.14.3 (repo-pinned in .pre-commit-config.yaml) auto-formatted two inference test files added on this branch: - test_conv_gpu.py: wrap two long-line assert messages onto multiple lines; drop the extra blank line between imports and the pytestmark block. - test_mme_dynamic.py: collapse a chained .get() call back onto a single line under the 100-char limit. No behavioral change. Verified locally with ruff 0.14.3 (format --check and check --select I both pass).
The unit tests in test_python_service_unit.py depended on the container-only /sagemaker/ lock file path and transitively triggered gevent monkey-patching of the pytest process, causing an SSL recursion cascade in every downstream boto3-using test. Coverage of handler-level MME code paths (on_delete, on_get, traversal guard) will be re-added as a container-side unit test suite in a follow-up. The SageMaker integration traversal test in test_mme_dynamic still covers the end-to-end path.
Wrap the Keras Conv2D model in a tf.Module subclass so the serving signature and the model variables share a trackable root. The prior bare tf.function closed over the Keras model via Python scope, which left dense/bias unbound at invoke time and returned: FAILED_PRECONDITION: Could not find variable dense/bias Mirrors the MultiplierModel pattern used by build_sample_model.
Ruff-format flagged the @tf.function decorator on line 142 of build_sample_model.py as too long after the ConvSmokeModel rewrite in 15ff812. Split the input_signature argument onto its own line to match ruff's formatting.
Customers export Keras models to SavedModel via either model.export() (Keras 3, recommended) or the legacy tf.keras.models.save_model(). Parametrize the Conv2D GPU test over both paths so a regression in TFS 2.20 compat with either path surfaces in CI rather than in a customer endpoint. Prior tf.Module + Keras layer wrapper hit auto-uniqueing during @tf.function trace and produced FAILED_PRECONDITION on serve. The Keras writers own both signature and variables end-to-end and match what customers actually run.
Keras 3 (bundled with TF 2.20) removed the save_format='tf' argument from both tf.keras.models.save_model() and model.save(). Any attempt to produce a "legacy" SavedModel from within a Keras 3 environment raises ValueError immediately, so the parametrization tested nothing that could ever pass. The remaining model_export parametrization already exercises the same cuDNN Conv2D forward path end-to-end on GPU, which is what the test exists to prove. Drop the parametrize/dict indirection and call the single builder directly.
…opagation
Five defects surfaced by a deep code review of the inference handler:
* GET /models/{name} returned 500 for every loaded model.
json.dumps(Response) raised TypeError, which the surrounding
except ValueError did not catch. Parse the body first and widen
the except.
* The MME model-name traversal guard existed only on the POST
(load) path. GET and DELETE routes accepted the same path
component unvalidated. Hoist the guard into a helper and apply
it uniformly.
* dockerd_entrypoint.sh ran the CMD via 'eval "\$@"', which leaves
the payload as a child of PID 1 and swallows SIGTERM. Every
endpoint scale-in was a SIGKILL after Docker's 10s grace, with
in-flight requests dropped instead of drained. Switch to exec.
* _delete_model raised ProcessLookupError (an OSError subclass) if
a recorded TFS pid was already gone, aborting on_delete before
the dict/pickle bookkeeping. The model name was then permanently
wedged at 409 with no self-healing path. Catch the specific
subclass and log.
* tensorflowServing.js csv_request used String.replace with string
arguments, which replace only the first match. Any CSV with a
leading non-numeric column returned a 200 with wrong tensor
shape. Use /g regex form.
Extend the CSV content-type test to reach the previously-uncovered
branch.
Collapse the multi-line json.dumps call in _reject_bad_model_name to match ruff-format's line-length rules; ruff-check clean afterward.
Two Dockerfile constructs silently masked what they claimed to do: * uv sync did not include --extra sagemaker, so the pandas / scikit-learn / cloudpickle packages declared in the pyproject optional-dependencies (and documented as available for customer inference handlers) never landed in the runtime venv. A customer handler with `import pandas` failed at first invocation. Mirror the pattern already used by the TF 2.21 training Dockerfile. * /opt/venv/bin/uv did not exist — uv was staged only into builder-base, and only /opt/venv is copied out to the runtime stage. Every build silently took the `|| pip install` fallback. Stage uv into the sagemaker stage that uses it (same :latest reference as builder-base and training), drop the fallback, and let a real failure surface at build time.
Four test-layer defects surfaced by a deep code review: * deploy_endpoint returned the tuple only after wait_for_status succeeded, so a Failed endpoint never registered for cleanup and leaked continuously. Move registration into the fixture before any AWS create call so a mid-flight deploy failure still tears down. * test_nginx_env_vars supplied no code_files, so _use_gunicorn stayed False and three of the four SAGEMAKER_GUNICORN_* env vars were inert despite the docstring claiming coverage. Add a minimal passthrough inference.py so gunicorn actually starts and assert a gunicorn-served marker key round-trips. * test_conv_gpu fed an all-zeros payload; Keras zero-init biases make the output deterministically 0.0 regardless of kernel weights, so a stubbed-zero response would pass. Feed 1.0 and assert non-zero. * MME target_model traversal is already exercised end-to-end via the SageMaker InvokeEndpoint API in test_mme_dynamic.py; add a README-coverage-gaps.md documenting the residual GET/DELETE coverage gap (routes reachable only from inside the container) and what a container-level harness would need to look like to close it.
* dockerfmt rewrote the two-line RUN continuation indent from 4 spaces to 2 spaces in both inference Dockerfiles. * mdformat rewrote the numbered list in README-coverage-gaps.md to all start with "1." (renumbered on render).
* H-1 — test_conv_gpu had a ~6% flake rate on all-ones + Glorot init: P(all 4 Conv2D filters have non-positive weight sum) = 1/16. Pin bias_initializer to a positive constant on both Conv2D and Dense layers so pre-activation is deterministically positive. Removes a flake whose failure message is indistinguishable from the real cuDNN regression it guards. * H-3 — add /usr/bin compat symlinks for tensorflow_model_server and tf_serving_entrypoint.sh in both Dockerfile.cpu and Dockerfile.cuda. Master's canary/ECS/EKS harnesses hardcode /usr/bin/; v2 image installs to /usr/local/bin/. Symlinks eliminate a latent hard-break at GA promotion, zero risk today. * H-5 — extend test_content_types.py with a quoted, comma-containing CSV case that reaches the needs_quotes=true branch. Previously the fix to tensorflowServing.js:214 (non-global replace -> /g regex) shipped with no test coverage. * H-6 — narrow test_mme_traversal_rejected's pytest.raises to (ClientError, ParamValidationError) and assert unconditionally. Was pytest.raises(Exception) with the assertion inside a ClientError isinstance guard, so a ParamValidationError satisfied the test without asserting anything. * M-2 — remove stale comment in Dockerfile.cuda that still describes the pre-fix eval-based entrypoint behavior; the current entrypoint uses exec so signals reach TFS as PID 1. * M-11 — tighten test_mme_dynamic.py unknown-model status band from 400..600 to 400..500. Docstring says 4xx, band accepted 5xx.
* H-2 — enumerate cuDNN 9 sub-libraries (_ops, _cnn, _adv, _graph, _engines_precompiled, _engines_runtime_compiled, _heuristics) and assert each is present on disk and resolvable via ldconfig. The existing libcudnn*-glob and libcudnn.so substring checks would pass for a build shipping only the dispatcher stub, which is the exact failure mode that let a prior training-canary cuDNN regression ship past ldd + libcudnn glob checks. * H-4 — pin the TFS-2.20-serves-TF-2.21 forward-compat boundary. DecodeJxl is the one net-new op in TF 2.21 that TFS 2.20 does not know; failure signature is uniquely nasty (status reports AVAILABLE, first predict returns 4xx Op type not registered), so a health-check rollout gate would go green with the customer taking the error. Document the boundary in code, not just prose — a future TFS upgrade that gains DecodeJxl support flips this test green automatically.
dockerfmt wants 2-space indent on `&&`-continuation lines. The H-3 symlink block used 1 space, tripping pre-commit. Match the file's established 2-space style.
…l test Two issues surfaced on the c176746 CI run: * Sanity test asserted `libcudnn_heuristics.so.9` (plural), but upstream cuDNN 9 ships `libcudnn_heuristic.so.9` (singular). Typo in the sub-library list added by 7df7dff. One-char fix. * The DecodeJxl boundary test could not reliably distinguish "TFS rejected the op" from "SM returned an empty response body for another reason": when the test host has TF 2.21, the exported SavedModel reaches the endpoint in a form TFS 2.20 accepts (op inlined or optimized away during export). The test asserted a 4xx with an op-not-registered marker in the body and got a 4xx with empty body — which reveals the test is not measuring what its docstring claims. Delete the test and document the DecodeJxl boundary in README-coverage-gaps.md instead. A proper boundary test needs a direct-to-TFS harness (bypassing SM's response reshaping) or a pre-built SavedModel artifact that guarantees the op survives export — both out of scope for this PR.
mdformat re-wrapped the "First predict" bullet in the DecodeJxl section to one line. Apply the auto-fix so pre-commit is green.
* B-1 (test/tensorflow/integration/inference/test_conv_gpu.py + resources/
build_sample_model.py): the conv test had a 100% false-negative rate
on cuDNN kernel bypass. With Dense bias pinned to 1.0, a fully dead
conv (feature map = 0) still produced Dense output = 0 + 1.0 = 1.0,
which passed `scalar != 0.0`. Replace random-init with closed-form
constant init: Conv2D kernel=1.0/bias=0.0, Dense kernel=1.0/bias=0.0,
all-ones input. Deterministic output is 108.0 (27+27+27+27 * 1.0).
A dead conv now yields 0.0 and fails loudly. No flake because
everything is a constant.
* B-2 (.github/workflows/tensorflow.pr-2.20-inference-{cpu,cuda}.yml):
test-only PR edits skipped every SageMaker integration test and
reported the lane green. Chain: test files not in build-change
filter -> build job skipped -> image-uri empty -> check-image-exists
returns false (gracefully) -> sagemaker-test skipped -> parent job
succeeds. Add test/tensorflow/integration/inference/** to
build-change so an integration test edit forces a rebuild.
* B-3 (test/sanity/scripts/test_sanity_tf_inference.py): my cuDNN
sub-library list omitted two libraries the 9.24 wheel actually
ships: libcudnn_ext.so.9 (dlopen'd by name by the libcudnn.so.9
dispatcher, new in the 9.24 line) and libcudnn_engines_tensor_ir.so.9
(transitive via DT_NEEDED on libcudnn_graph). The cp glob copies
them so the image is fine today, but the guard cannot detect a
regression that dropped either. Add both.
* B-4 (scripts/docker/tensorflow/inference/sagemaker/python_service.py):
MME + batching (SAGEMAKER_MULTI_MODEL=true and
SAGEMAKER_TFS_ENABLE_BATCHING=true) failed every POST /models with
a 500. Root cause: tfs_config_file's parent dir is created via
os.makedirs, but batching_config_file's parent dir is not.
create_batching_config does a bare open() which raises
FileNotFoundError (OSError errno=2), which the outer handler only
maps to 507 for errno=12 (ENOMEM), so it fell through to 500.
Inherited from master. Fix: mirror the tfs-config makedirs.
* PR trigger paths: add tensorflow.tests-sagemaker-inference.yml
to both cpu and cuda pr workflows. Without it, a PR editing only
the test workflow triggers zero CI checks and is mergeable with
no signal. Training already lists its test workflows both.
* SM_DEVICE_TYPE fail-closed: conftest.py and test_conv_gpu.py both
defaulted to "cpu" on missing env var. If yq ever prints "null"
for a missing config key, the CUDA lane would silently test on a
CPU instance and skip the GPU-only conv test — green. Fail loudly
on missing / invalid values.
* Drop /opt/ml/code from Dockerfile mkdir: serve.py gates the MME
universal-script S3 download on `not os.path.exists("/opt/ml/code")`.
Pre-creating the dir made exists() true, silently disabling the
download for customers using SAGEMAKER_MULTI_MODEL_UNIVERSAL_BUCKET.
Master creates none of /opt/ml/*.
* Strip internal-artifact provenance from public-repo comments:
handoff-section refs and audit-finding IDs in pyproject/sanity/
conftest replaced with the technical constraint stated directly.
Public PR numbers (#6418, #6243, #6107) retained.
* Sanity: add ctypes.CDLL per cuDNN sub-lib (mirrors training file
pattern). Catches truncated / zero-byte / wrong-arch / ABI-drifted
.so files that pass glob + ldconfig -p.
* Sanity: assert EXPECTED_TFS_VERSION in tensorflow_model_server
--version. Previously accepted any "TensorFlow ModelServer"
string — a build that silently shipped 2.19 or 2.21 passed.
* Delete test_mme_traversal_rejected — botocore does not enforce
pattern-validation on TargetModel, so ParamValidationError is
dead code, and the ClientError assertion is byte-identical to
test_mme_target_model_not_found. Reverting the container guard
leaves this test green. Update README-coverage-gaps.md.
* Delete test_csv_content_type_quoted_string_with_embedded_commas
— payload starts with " so needs_quotes evaluates false and the
/g branch this test claimed to cover never executes.
HIGH — out-of-bounds heap read in aiohttp's C response parser pre-3.14.3. Bump the direct pin from >=3.14.0 to >=3.14.3 in both cpu and cuda pyproject.toml, re-lock. Only aiohttp version changed in both lockfiles (3.14.1 -> 3.14.3, 45 CPU / 48 CUDA packages resolved unchanged otherwise). aiohttp is pulled transitively via aiobotocore. The pin exists to force the resolver to a patched line; keeping the pin below the CVE fix version defeats the purpose.
serve.py:_enable_per_process_gpu_memory_fraction hardcodes /usr/bin/nvidia-smi (inherited from master). On a GPU host the NVIDIA container runtime injects nvidia-smi into the container at that path. A base-image bump (e.g. nvidia/cuda 12.9.1 -> 12.10 -> 13.0) that moves the injection path would silently disable the memory-fraction gate, and MME customers running SAGEMAKER_TFS_INSTANCE_COUNT>1 would hit OOM at first inference with no log signal. Assert /usr/bin/nvidia-smi exists under @gpu_only so CI catches this class of regression on every push. Cost is a single os.path.exists call at sanity time.
Comment-only sweep. Trims long design-decision explanations, historical narration, and multi-paragraph docstrings down to crisp one-liners. Keeps load-bearing facts (why a fix exists, what a subtle constraint means) but cuts narration of prior review rounds, restated code behavior, and rejected alternatives. Zero code changes.
The sanity container is launched via `docker run -d -it --entrypoint /bin/bash` in _reusable.sanity-tests.yml — no `--gpus all` flag. The NVIDIA container runtime only injects /usr/bin/nvidia-smi when the container is launched with GPU passthrough, so the assertion fails on every CUDA image regardless of image content. @gpu_only skips based on EXPECTED_DEVICE env var, not on actual GPU availability in the sanity container. The check needs to run against a real GPU-passthrough container (devbox with `docker run --gpus all <image> ls -l /usr/bin/nvidia-smi` or an integration test that deploys to a real SM GPU endpoint). Removing the sanity assertion; the actual verification is a separate follow-up.
* tfs_utils split("=", 1): base64-padded custom-attribute values raised
ValueError uncaught, surfacing as opaque 500.
* python_service pickle write: made atomic (tmp + fsync + rename) and
wrapped load in try/except (EOFError, UnpicklingError) resetting to
empty. Previously an OOM SIGKILL mid-write bricked all MME routes
with permanent 500 while /ping stayed 200.
* tfs_utils.make_tfs_uri: validate tfs-model-version against ^\d+$ and
tfs-method against {predict, classify, regress}. Prevents unvalidated
interpolation into the TFS REST URI.
* multi_model_utils.lock: replaced blocking fcntl.lockf with non-blocking
retry + gevent-yielding sleep. Blocking lockf parked the entire hub
including /ping under contention with SAGEMAKER_GUNICORN_WORKERS>=2.
Dropped the inherited-master 1s sleep before LOCK_UN.
* python_service load failure: pop the ghost dict entry and re-upload
status. Without this, setdefault().append() on the failure branch
left a permanent 409 "already loaded" for that model name.
* tests-sagemaker-inference.yml: device-type now required, no default.
Fail closed on config wiring bugs instead of silently deploying the
CUDA lane to ml.c5.xlarge.
* pr-2.20-inference-{cpu,cuda}.yml: added test/sanity/** to build-change.
Sanity edits now trigger a rebuild instead of silently skipping.
…exec, cuDNN RNN * test_mme_concurrency: fires ThreadPoolExecutor(8) invokes across 3 distinct-multiplier MME models under SAGEMAKER_GUNICORN_WORKERS=2. Asserts each returns its own model's arithmetic. Guards against pickle-write atomicity regressions, fcntl.lockf stalls under gevent, and ghost-entry-on-failed-load bugs. * test_sanity_tf_inference: asserts entrypoint scripts use exec (not eval) form and contain at least one `exec <cmd>` line — covers /usr/local/bin/dockerd_entrypoint.sh, /usr/local/bin/tf_serving_entrypoint.sh, and /sagemaker/serve. A regression to eval or a subshell silently breaks SIGTERM propagation and turns every SM scale-in into a SIGKILL after 10s with in-flight requests dropped. * test_lstm_gpu: LSTM(units=1) with constant-init weights + all-ones input, invoked twice. Asserts finite + non-zero + deterministic across invokes. Exercises libcudnn_adv.so.9 (RNN library asserted present at sanity but never actually loaded by any request until now); complements test_conv_gpu.py which covers libcudnn_cnn / libcudnn_ops. Fixture pattern: mirrors existing test_conv_gpu.py — SM_DEVICE_TYPE gate at module scope, deploy_endpoint / cleanup_endpoint reuse, no hardcoded instance types.
The file cataloged residual test-coverage gaps as review-round narrative. It duplicated information now captured in commit messages and the PR body, and its content included branch-local SHAs that won't survive a squash merge and framing (e.g., "past 24 green CI checks") that reads as internal review artifacts. Drop it.
test_lstm_gpu_predict fails deterministically with: UNKNOWN: JIT compilation failed node: sequential/lstm/while/lstm_cell/Sigmoid Same class of forward-compat boundary as tf.raw_ops.DecodeJxl: TF 2.21's Keras 3 LSTM export produces a functional graph with a while_loop that TFS 2.20's XLA/JIT autotuner cannot compile. The test exposed a real customer-facing gap but the fix belongs to TFS 2.21+ or a downstream XLA config change — out of scope for this PR. Also removes _build_lstm_sequential + build_lstm_sample_model helpers from build_sample_model.py (no remaining callers). Customer guidance until TFS 2.21+ ships: LSTM / GRU models require either CPU inference or a future TFS version with the JIT path fixed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The first TensorFlow inference DLC on the
main(v2) branch. TensorFlow Serving 2.20.0 serving TF 2.21-trained SavedModels on AL2023 + CUDA 12.9.1 + Python 3.12, SageMaker-only, x86-only, with single-model and Multi-Model Endpoint (MME) support.docker/tensorflow/inference/2.20/tree —Dockerfile.cudaandDockerfile.cpuon AL2023, uv-driven dependency management, cuDNN pinned vianvidia-cudnn-cu12==9.24.0.43.nginx-mod-njsinstalled from AL2023 core (--releasever latest) — no source-build stage needed.build_artifacts/sagemaker/with 3 inherited-master bug fixes surfaced by review:GET /models/{name}returning 500, missing traversal guard,ProcessLookupErrorleak on DELETE.inference.py, error paths, model versioning, nginx/gunicorn env tuning, TFS batching, and GPU-only Conv2D exercising cuDNN kernel dispatch.Why this PR
This is the first TensorFlow inference image on
main(v2 architecture). The master branch's TF 2.19 inference image is on Ubuntu 22.04 + Python 3.10 + CUDA 12.2. Porting tomain's AL2023 + Python 3.12 + CUDA 12.9.1 substrate is the migration this PR delivers, aligned with the TF 2.21 training PR (#6107) already merged onmain.Test plan
tensorflow_model_server --version, cuDNN sub-library presence vialdconfig -p, handler artifacts, entrypoint executable,exec-hands-off-PID1 assertion for SIGTERM-critical scripts)After merge
Follow-up PR + tasks required before customer-facing release:
environment : productionin.github/config/image/tensorflow/2.20-inference-sagemaker-{cpu,cuda}.ymlschedule:trigger (Tue/Thu 17:30 UTC, matching training) toautorelease-tensorflow-inference-sagemaker-{cpu,cuda}.ymldocs/src/data/tensorflow-inference/*.yml)docs/tensorflow/tree)Pre-review notes
while_loopgraphs under its XLA/JIT autotuner. Same forward-compat gap class as DecodeJxl. Not a deferral — LSTM/GRU inference is simply out-of-scope for this image; customers should use TFS 2.21+ or CPU inference for those workloads.Related
test/security/data/ecr_scan_allowlist/tensorflow/framework_allowlist.jsonfile ships from there.