fix(batch): sync Batch application settings across code and deploy paths

.github/scripts/check_env_drift.py

@@ -0,0 +1,237 @@
+#!/usr/bin/env python3
+"""Fail when an application setting the code requires is not deployed.
+
+`hastegeo` reads its configuration from environment variables, and the Function
+Apps receive those as application settings from two deploy paths:
+
+    .github/scripts/deploy_apps.sh   (used by .github/workflows/deploy-apps.yml)
+    infra/modules/functions.bicep    (used by the Bicep/IaC path)
+
+Renaming a variable in code without renaming it in *both* deploy paths leaves
+the setting silently unset, and the code falls back to a `<placeholder>` default
+that only fails once Azure rejects it -- far from the cause. That is exactly how
+`AZURE_BATCH_REGISTRY_SERVER` broke image preprocessing.
+
+A variable is treated as REQUIRED when the code either reads it with no default
+at all, or defaults it to a value still containing a `<placeholder>`. Anything
+with a real, working default is optional and ignored.
+
+Usage:  python .github/scripts/check_env_drift.py
+Exit 0 when in sync, 1 otherwise.
+"""
+
+from __future__ import annotations
+
+import ast
+import re
+import sys
+from pathlib import Path
+
+REPO = Path(__file__).resolve().parents[2]
+
+CODE_ROOTS = (
+    REPO / "hastelib" / "src",
+    REPO / "api" / "hastefuncapi",
+    REPO / "api" / "hastefuncqueues",
+    REPO / "api" / "titilerfuncapi",
+)
+
+DEPLOY_SH = REPO / ".github" / "scripts" / "deploy_apps.sh"
+FUNCTIONS_BICEP = REPO / "infra" / "modules" / "functions.bicep"
+
+PLACEHOLDER = re.compile(r"<[^<>]+>")
+
+# Variables that are genuinely optional for an Azure deployment, with the reason
+# each one is exempt. Anything not listed here that the code marks required must
+# be emitted by both deploy paths.
+ALLOWLIST = {
+    # Alternative metadata/storage backends. Azure deployments use blob storage
+    # (METADATA_STORAGE_TYPE=blob), so these are never read there.
+    "COSMOS_ENDPOINT": "cosmos backend only",
+    "COSMOS_DATABASE": "cosmos backend only",
+    "COSMOS_CONTAINER": "cosmos backend only",
+    "DATALAKE_ACCOUNT_URL": "datalake backend only",
+    "DATALAKE_FILESYSTEM": "datalake backend only",
+    "POSTGRES_HOST": "postgres backend only",
+    "POSTGRES_PORT": "postgres backend only",
+    "POSTGRES_USER": "postgres backend only",
+    # pragma: allowlist nextline secret
+    "POSTGRES_PASSWORD": "postgres backend only",  # pragma: allowlist secret
+    "POSTGRES_DATABASE": "postgres backend only",
+    "POSTGRES_TABLE": "postgres backend only",
+    # Local docker-compose runner (RUNNER_TYPE=local); unused on Azure Batch.
+    "AZURE_STORAGE_CONNECTION_STRING": "local runner only",
+    "AZURE_STORAGE_ACCOUNT": "local runner only",
+    "HASTE_DOCKER_NETWORK": "local runner only",
+    "HASTE_DOCKER_MEM_LIMIT": "local runner only",
+    "HASTE_DOCKER_SHM_SIZE": "local runner only",
+    "HASTE_DOCKER_AZURITE_VOLUME": "local runner only",
+    "HASTE_ENABLE_GPU": "local runner only",
+    "HASTE_GPU_DEVICES": "local runner only",
+    "HASTE_DATALOADER_WORKERS": "local runner only",
+    "HASTE_DEBUG_VERBOSE": "local runner only",
+    "CLEANUP_CONTAINERS": "local runner only",
+    "PRESERVE_LOCAL_TASK_DIRS": "local runner only",
+    "FAIL_ON_EMPTY_OUTPUT_LOG": "local runner only",
+    # Set by the Batch node agent / supplied inside the task container.
+    "WORKDIR": "set inside the task container",
+    "INPUT_DIR": "set inside the task container",
+    "OUTPUT_TRAINING_ZIP_NAME": "set inside the task container",
+    "OUTPUT_INFERENCE_ZIP_NAME": "set inside the task container",
+    # Azure Functions / platform-provided.
+    "AzureWebJobsStorage": "provided by the Functions runtime",
+    "WEBSITE_HOSTNAME": "provided by the Functions runtime",
+    "FUNCTIONS_WORKER_RUNTIME": "provided by the Functions runtime",
+    # Deprecated legacy name, still read as a fallback so environments
+    # provisioned before the rename keep working. Deliberately not emitted.
+    "AZURE_BATCH_REGISTRY_SERVER_URL": "deprecated legacy fallback",
+    # GDAL tuning read inside the imageryprep/training container, where the
+    # workflow supplies them; not Function App settings.
+    "GDAL_SKIP": "read inside the task container",
+    "GDAL_WARP_PARAMS": "read inside the task container",
+    "GDAL_TRANSLATE_PARAMS": "read inside the task container",
+}
+
+
+def _literal(node: ast.AST):
+    try:
+        return ast.literal_eval(node)
+    except (ValueError, SyntaxError):
+        return None
+
+
+def scan_code() -> tuple[dict[str, set[Path]], set[str]]:
+    """Return ({required var: files}, every var the code reads)."""
+    required: dict[str, set[Path]] = {}
+    seen: set[str] = set()
+
+    for root in CODE_ROOTS:
+        if not root.exists():
+            continue
+        for path in root.rglob("*.py"):
+            if "__pycache__" in path.parts or "tests" in path.parts:
+                continue
+            try:
+                tree = ast.parse(path.read_text(encoding="utf-8"))
+            except SyntaxError:
+                continue
+
+            for node in ast.walk(tree):
+                name, default, has_default = None, None, False
+
+                if isinstance(node, ast.Call):
+                    func = node.func
+                    is_getenv = (
+                        isinstance(func, ast.Attribute)
+                        and func.attr in ("getenv", "get")
+                        and node.args
+                    )
+                    if is_getenv:
+                        target = ast.unparse(func)
+                        if target.endswith(
+                            ("os.getenv", "os.environ.get", "environ.get")
+                        ):
+                            name = _literal(node.args[0])
+                            has_default = len(node.args) > 1
+                            if has_default:
+                                default = _literal(node.args[1])
+
+                elif isinstance(node, ast.Subscript):
+                    value = node.value
+                    if isinstance(value, ast.Attribute) and ast.unparse(
+                        value
+                    ).endswith("os.environ"):
+                        name = _literal(node.slice)
+
+                if not isinstance(name, str):
+                    continue
+                seen.add(name)
+
+                # Required = no default, or a default that is still a
+                # <placeholder> the operator was meant to replace.
+                unresolved = isinstance(default, str) and PLACEHOLDER.search(
+                    default
+                )
+                if has_default and not unresolved:
+                    continue
+                required.setdefault(name, set()).add(path.relative_to(REPO))
+
+    return required, seen
+
+
+def settings_in_deploy_sh() -> set[str]:
+    """Extract only the keys inside the `appsettings set --settings` block.
+
+    Scanning the whole file would also pick up resource tags such as
+    `project=haste`, which are not application settings.
+    """
+    names: set[str] = set()
+    in_block = False
+    for line in DEPLOY_SH.read_text(encoding="utf-8").splitlines():
+        if "appsettings set" in line:
+            in_block = True
+            continue
+        if in_block:
+            names.update(re.findall(r'"([A-Za-z_][A-Za-z0-9_]*)=', line))
+            if not line.rstrip().endswith("\\"):
+                in_block = False
+    return names
+
+
+def settings_in_bicep() -> set[str]:
+    text = FUNCTIONS_BICEP.read_text(encoding="utf-8")
+    return set(re.findall(r"\{\s*name:\s*'([A-Za-z_][A-Za-z0-9_]*)'", text))
+
+
+def main() -> int:
+    required, all_read = scan_code()
+    surfaces = {
+        str(DEPLOY_SH.relative_to(REPO)): settings_in_deploy_sh(),
+        str(FUNCTIONS_BICEP.relative_to(REPO)): settings_in_bicep(),
+    }
+
+    failures: list[str] = []
+    for var in sorted(required):
+        if var in ALLOWLIST:
+            continue
+        missing = [name for name, s in surfaces.items() if var not in s]
+        if missing:
+            readers = ", ".join(sorted(str(p) for p in required[var]))
+            failures.append(
+                f"  {var}\n"
+                f"      read by:     {readers}\n"
+                f"      NOT set by:  {', '.join(missing)}"
+            )
+
+    # Settings a deploy path emits that no code reads -- dead config, and a
+    # strong signal that a rename was applied on only one side.
+    dead: list[str] = []
+    for surface, names in surfaces.items():
+        for var in sorted(names):
+            if var not in all_read and var not in ALLOWLIST:
+                dead.append(f"  {var}  (set by {surface}, read by no code)")
+
+    if failures:
+        print(
+            "Required application settings are not emitted by every "
+            "deploy path:\n"
+        )
+        print("\n".join(failures))
+    if dead:
+        print("\nDead application settings (set but never read):\n")
+        print("\n".join(dead))
+
+    if failures or dead:
+        print(
+            "\nFix by updating .github/scripts/deploy_apps.sh and "
+            "infra/modules/functions.bicep together, or add a documented "
+            "entry to ALLOWLIST in this script."
+        )
+        return 1
+
+    print("Application settings are in sync across code and deploy paths.")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

.github/scripts/deploy_apps.sh

@@ -42,6 +42,20 @@ USER_MANAGED_IDENTITY="${RESOURCE_PREFIX}-haste-${RANDOM_SUFFIX}-umi"
 TRAINING_DOCKER_IMAGE="hastetraining:${TRAINING_IMAGE_TAG}"
 IMAGEPREP_DOCKER_IMAGE="hasteimageryprep:${IMAGEPREP_IMAGE_TAG}"
 BATCH_POOL_ID="${RESOURCE_PREFIX}-haste-${RANDOM_SUFFIX}-pool"
+# Batch pool wiring. Defaults reproduce the legacy single-pool behavior; set the
+# corresponding GitHub Environment variables to point an environment at
+# pre-created shared pools (see docs/configuration.md).
+#   *_POOL_ID   - the pool used when no candidate list is supplied. It is also
+#                 the default Batch *job* id, so it must be identical across the
+#                 api and queues apps or status lookups miss the job.
+#   *_POOL_IDS  - ordered candidate lists for capacity-aware routing.
+BATCH_TRAINING_POOL_ID="${BATCH_TRAINING_POOL_ID:-$BATCH_POOL_ID}"
+BATCH_IMAGERYPREP_POOL_ID="${BATCH_IMAGERYPREP_POOL_ID:-$BATCH_POOL_ID}"
+BATCH_TRAINING_POOL_IDS="${BATCH_TRAINING_POOL_IDS:-}"
+BATCH_INFERENCE_POOL_IDS="${BATCH_INFERENCE_POOL_IDS:-}"
+BATCH_IMAGERYPREP_POOL_IDS="${BATCH_IMAGERYPREP_POOL_IDS:-}"
+BATCH_USE_SAS="${BATCH_USE_SAS:-false}"
+BATCH_MANAGE_POOLS="${BATCH_MANAGE_POOLS:-true}"
 MAPS_ACCOUNT="${RESOURCE_PREFIX}haste${RANDOM_SUFFIX}maps"
 API_MANAGEMENT="${RESOURCE_PREFIX}-haste-${RANDOM_SUFFIX}-apim"
 FIXED_TAGS="project=haste created_by=deploy_apps"
@@ -87,6 +101,7 @@ deploy_function() {
             "STATS_QUEUE_NAME=stats-queue" \
             "TRAIN_QUEUE_NAME=train-queue" \
             "ZIP_QUEUE_NAME=zip-queue" \
+            "EMBEDDING_QUEUE_NAME=embedding-queue" \
             "IMAGERY_STORAGE_TYPE=blob" \
             "METADATA_STORAGE_TYPE=blob" \
             "ARTIFACT_STORAGE_TYPE=blob" \
@@ -104,9 +119,14 @@ deploy_function() {
             "AZURE_BATCH_IMAGERYPREP_DOCKER_IMAGE=${ACR_NAME}.azurecr.io/${IMAGEPREP_DOCKER_IMAGE}" \
             "AZURE_BATCH_DOCKER_IMAGE=${ACR_NAME}.azurecr.io/${TRAINING_DOCKER_IMAGE}" \
             "AZURE_BATCH_OUTPUT_CONTAINER_URL=https://${STORAGE_ACCOUNT}.blob.core.windows.net/data" \
-            "AZURE_BATCH_TRAINING_POOL_ID=${BATCH_POOL_ID}" \
-            "AZURE_BATCH_IMAGERYPREP_POOL_ID=${BATCH_POOL_ID}" \
-            "AZURE_BATCH_REGISTRY_SERVER_URL=https://${ACR_NAME}.azurecr.io" \
+            "AZURE_BATCH_TRAINING_POOL_ID=${BATCH_TRAINING_POOL_ID}" \
+            "AZURE_BATCH_IMAGERYPREP_POOL_ID=${BATCH_IMAGERYPREP_POOL_ID}" \
+            "AZURE_BATCH_TRAINING_POOL_IDS=${BATCH_TRAINING_POOL_IDS}" \
+            "AZURE_BATCH_INFERENCE_POOL_IDS=${BATCH_INFERENCE_POOL_IDS}" \
+            "AZURE_BATCH_IMAGERYPREP_POOL_IDS=${BATCH_IMAGERYPREP_POOL_IDS}" \
+            "AZURE_BATCH_USE_SAS=${BATCH_USE_SAS}" \
+            "AZURE_BATCH_MANAGE_POOLS=${BATCH_MANAGE_POOLS}" \
+            "AZURE_BATCH_REGISTRY_SERVER=${ACR_NAME}.azurecr.io" \
             "AZURE_BATCH_REGISTRY_IMAGE=${ACR_NAME}.azurecr.io/${TRAINING_DOCKER_IMAGE}" \
             "AZURE_BATCH_REGISTRY_IDENTITY_RESOURCE_ID=/subscriptions/$SUBSCRIPTION_ID/resourceGroups/$RESOURCE_GROUP/providers/Microsoft.ManagedIdentity/userAssignedIdentities/$USER_MANAGED_IDENTITY" \
             "STATIC_APP_SUBSCRIPTION_ID=$SUBSCRIPTION_ID" \

.github/workflows/config-drift.yml

@@ -0,0 +1,43 @@
+name: Config drift
+
+# A variable renamed in code but not in every deploy path leaves the setting
+# silently unset in Azure, and the app falls back to a <placeholder> default
+# that only fails once Azure rejects it. Catch that here instead of in an
+# environment.
+on:
+  pull_request:
+    branches:
+      - main
+    paths:
+      - "hastelib/src/**"
+      - "api/**"
+      - "infra/modules/functions.bicep"
+      - ".github/scripts/deploy_apps.sh"
+      - ".github/scripts/check_env_drift.py"
+      - ".github/workflows/config-drift.yml"
+  push:
+    branches:
+      - main
+    paths:
+      - "hastelib/src/**"
+      - "api/**"
+      - "infra/modules/functions.bicep"
+      - ".github/scripts/deploy_apps.sh"
+
+permissions:
+  contents: read
+
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+        with:
+          persist-credentials: false
+
+      - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5
+        with:
+          python-version: "3.11"
+
+      - name: Check application-setting drift
+        run: python .github/scripts/check_env_drift.py

.github/workflows/deploy-apps.yml

@@ -115,6 +115,16 @@ jobs:
           # `func publish` (deploy_apps.sh); the editable default can't resolve
           # on Azure's remote build.
           HASTEGEO_WHEEL_URL: ${{ steps.hastegeo.outputs.url }}
+          # Batch pool wiring. All optional: unset reproduces the legacy
+          # single-pool behavior. Set these as GitHub Environment variables to
+          # point an environment at pre-created shared pools.
+          BATCH_TRAINING_POOL_ID: ${{ vars.BATCH_TRAINING_POOL_ID }}
+          BATCH_IMAGERYPREP_POOL_ID: ${{ vars.BATCH_IMAGERYPREP_POOL_ID }}
+          BATCH_TRAINING_POOL_IDS: ${{ vars.BATCH_TRAINING_POOL_IDS }}
+          BATCH_INFERENCE_POOL_IDS: ${{ vars.BATCH_INFERENCE_POOL_IDS }}
+          BATCH_IMAGERYPREP_POOL_IDS: ${{ vars.BATCH_IMAGERYPREP_POOL_IDS }}
+          BATCH_USE_SAS: ${{ vars.BATCH_USE_SAS }}
+          BATCH_MANAGE_POOLS: ${{ vars.BATCH_MANAGE_POOLS }}
         run: |
           chmod +x .github/scripts/deploy_apps.sh
           .github/scripts/deploy_apps.sh \

CHANGELOG.md

@@ -12,6 +12,15 @@ Versioning follows the Docker image tags defined in the CI workflows (see [.gith
 ### Added
 - **Shared multi-tenant GPU Batch pools** — For deployments running many environments against scarce GPU quota, HASTE can now share a small set of multi-tenant Batch pools (H100 for training, T4 for inference/imageryprep + spillover) instead of one pool per environment. Data isolation is enforced at the credential boundary: each job mints a short-lived **user-delegation SAS** scoped to its own storage container (the pool identity is used only for ACR pull and holds no storage access), so a tenant's task can never read another tenant's data. Pools autoscale on low-priority nodes and scale to zero when idle. New `hastelib` routing picks a pool from an ordered candidate list **at submit time** (first with an idle node, else the preferred pool). Provisioned by the standalone [`infra/shared-pools.bicep`](infra/shared-pools.bicep) + [`shared-pools.bicepparam`](infra/shared-pools.bicepparam); opted into per environment via `AZURE_BATCH_*_POOL_IDS`, `AZURE_BATCH_USE_SAS`, and `AZURE_BATCH_MANAGE_POOLS` (all default to the legacy single-pool behavior). Full design in [`spec/features/batch-compute-expansion/`](spec/features/batch-compute-expansion). See [docs/configuration.md](docs/configuration.md#shared-multi-tenant-gpu-pools).
 
+### Fixed
+- **Batch application settings were out of sync with the code that reads them** — `hastegeo` reads `AZURE_BATCH_REGISTRY_SERVER`, but every deploy path still emitted `AZURE_BATCH_REGISTRY_SERVER_URL`, which nothing read. The setting therefore fell back to its `<registry-name>.azurecr.io` placeholder and image preprocessing failed with `InvalidPropertyValue: The specified registry is an invalid docker registry server name` before any task reached Batch. Separately, [`deploy_apps.sh`](.github/scripts/deploy_apps.sh) never emitted the shared-pool settings added alongside the multi-tenant pools (`AZURE_BATCH_*_POOL_IDS`, `AZURE_BATCH_USE_SAS`, `AZURE_BATCH_MANAGE_POOLS`) or `EMBEDDING_QUEUE_NAME`, so environments deployed through it silently ran the legacy single-pool path. Both deploy paths now emit the full set, the legacy `AZURE_BATCH_REGISTRY_SERVER_URL` is still honored as a fallback (with any `https://` prefix stripped) so existing deployments keep working, and a new `Config drift` workflow fails any PR that reintroduces this class of drift.
+
+  > **Operator action:** rename the `AZURE_BATCH_REGISTRY_SERVER_URL` application setting to `AZURE_BATCH_REGISTRY_SERVER` (value: the bare login server, e.g. `myacr.azurecr.io`) on the `api` and `queues` Function Apps. Confirm `AZURE_BATCH_TRAINING_POOL_ID` / `AZURE_BATCH_IMAGERYPREP_POOL_ID` name pools that still exist and are **identical across both apps** — they double as the default Batch job ids.
+
+- **A reused Batch job stayed pinned to the pool that created it** — job ids default to the configured pool id, and `create_job` only re-enabled an existing job, never re-pointing it. Capacity-aware spillover was therefore silently ineffective, and once a pool was renamed or deleted every task queued into a job bound to a pool that no longer existed. The runner now rebinds the job to the pool selected for the submission, and fails loudly when Batch refuses.
+- **Missing Batch settings now fail fast** — the runner validates its configuration before the first Batch call and names the specific application setting that is unset, instead of surfacing an opaque Azure error from deep inside pool creation.
+- **Pool creation whitelisted only the training image** — pools created by the runner now list both the training and imageryprep images, matching [`infra/modules/batchPool.bicep`](infra/modules/batchPool.bicep).
+
 ### Changed
 - **`infra/modules/batchPool.bicep` parameterized** — one module now serves fixed-dedicated (dev/prod) and autoscale-low-priority (shared) pools via `scaleMode` / `nodeType` / `minNodes` params, with optional VNet injection. Backward-compatible defaults.
 - **Generic-default IaC for reuse by other partners** — `HASTE_RESOURCE_PREFIX` now defaults to the neutral `haste` (overridable per deployment); the shared-pools template keeps its account/ACR as bring-your-own params. The `api`/`queues` Function App identity is granted **Storage Blob Delegator** (in `functionApp.bicep`) so it can mint user-delegation SAS.