fix: glance S3 location churn and undersized memory defaults#726
Merged
Conversation
Glance's S3 credential-rotation repair (_update_s3_location_and_store_id in glance/common/store_utils.py) rebuilds the expected location URL by concatenating the raw s3_store_host option. That option must carry its http:// or https:// prefix for non-AWS endpoints — boto3 requires a scheme in endpoint_url — but the location URLs stored per image embed only the bare authority, because the store strips the prefix when it builds Store._url_prefix. The two URLs therefore never converge: every API request that touches image locations logs "S3 URL mismatch for image ..., updating URL" and rewrites the location row in the database. Against the operator-rendered Garage backend that is one spurious UPDATE per GET/PATCH, and a genuine credential rotation is indistinguishable from the permanent false positive. Introduce the repo's first source patches (the patches/ mechanism has existed in the Build Images workflow since the start but was unused): strip the scheme prefix in _construct_s3_url exactly the way Store._url_prefix does. The function is identical in glance 31.1.0 (2025.2) and 32.0.0 (2026.1), so both release directories carry the same patch, validated with git apply against both upstream tags. Also teach hack/ci-build-service-image.sh to apply patches after cloning, mirroring .github/actions/checkout-service-source — without this the e2e- and locally-built service images would silently diverge from the published GHCR images the moment a patch exists. Sync the script's step numbering and the CI reference page. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt <berendt@23technologies.cloud>
The shared DeploymentSpec baseline (256Mi request / 512Mi limit) is calibrated for keystone and horizon. The glance-api container carries the S3 store driver on top: boto3/botocore raise the per-process import footprint (three Python processes with the bounded two workers) and add per-request client-construction churn. Measured on the quick-start stack, the container idles near 360Mi of the 512Mi limit and, under dizzy's concurrent image traffic, is OOM-killed within a minute — a crash loop the gateway surfaces as waves of 503s (416 of 929 image operations failed in a five-minute soak). With a 1Gi limit the same soak passes 1034/1034 with a 576Mi peak. Fill spec.deployment.resources in the glance defaulting webhook with a 512Mi request / 1Gi limit (CPU keeps the shared 100m/500m) before the shared DeploymentSpec defaults run, using the same nil-or-empty guard so an explicit user value is never clobbered. The request moves up with the limit because the shared 256Mi request sits below glance's idle footprint. Pre-existing CRs keep their materialized 512Mi limit — the webhook only fills empty resource blocks. The shared Resources godoc hardcoded the 256Mi/512Mi literals for every operator; reword it to point at the per-operator CRD reference and regenerate the keystone/horizon/glance CRDs plus their Helm chart copies. Document the concrete values in the glance and horizon CRD references (keystone already lists its own) and cover the new defaulting and the explicit-value passthrough with webhook unit tests. On-behalf-of: @SAP Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt <berendt@23technologies.cloud>
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
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.
Problem
make dizzy-glanceagainst the quick-start ControlPlane kept failing even after #725 bounded the eventlet worker count: 416 of 929 image operations returned HTTP 503, in waves where whole 30-second buckets failed completely. Live diagnosis showed two independent defects:S3 URL mismatch for image ..., updating URLand rewrites the location row. Glance's S3 credential-rotation repair (_construct_s3_url) concatenates the raws3_store_host— which must keep itshttp://prefix for boto3'sendpoint_urlon non-AWS endpoints like Garage — while the stored location URLs embed only the bare authority (Store._url_prefixstrips the prefix). The comparison never converges, so the repair fires on every request, forever.Fix
Commit 1 introduces the repo's first source patches under
patches/glance/{2025.2,2026.1}/: strip the scheme prefix in_construct_s3_urlexactly the wayStore._url_prefixdoes. The function is identical in glance 31.1.0 and 32.0.0; both patch files were validated withgit apply --checkagainst both upstream tags. It also teacheshack/ci-build-service-image.shto apply patches after cloning, mirroring.github/actions/checkout-service-source— without this, e2e- and locally-built images would silently diverge from the published GHCR images the moment a patch exists.Commit 2 gives glance service-specific memory defaults: the defaulting webhook fills an unset
spec.deployment.resourceswith a 512Mi request / 1Gi limit (CPU keeps the shared 100m/500m) before the sharedDeploymentSpecdefaults run, with the same nil-or-empty guard so explicit values are never clobbered. The sharedResourcesgodoc no longer hardcodes 256Mi/512Mi for every operator; the keystone/horizon/glance CRDs and their Helm chart copies are regenerated, and the concrete values are documented in the per-operator CRD references. Pre-existing CRs keep their materialized resources — the webhook only fills empty blocks.Verification
Reproduced and verified live on the quick-start stack (kind, Garage S3 backend, dizzy small chaos scenario, 5-minute soak):
The 576Mi peak sits above the old limit and comfortably below the new one, confirming the old default could not survive the soak regardless of worker count.
Test plan
go test ./operators/glance/... ./internal/common/...(new webhook unit tests cover the glance-specific defaulting and the explicit-value passthrough)make verify-crd-sync,gofumpt -l,shellcheck hack/ci-build-service-image.shall cleangit apply --checkagainst pristine glance 31.1.0 and 32.0.0 trees (including the SPDX preamble)🤖 Generated with Claude Code