mengw15 commented on code in PR #6295: URL: https://github.com/apache/texera/pull/6295#discussion_r3564115648
########## bin/k8s/values-aws.yaml: ########## @@ -0,0 +1,81 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# --------------------------------------------------------------------------- +# Example overlay for running Texera against an external S3 store (e.g. AWS S3) +# instead of the bundled in-cluster MinIO. +# +# helm install texera bin/k8s -f bin/k8s/values-aws.yaml +# +# Replace the placeholder bucket names, region, and credentials below with your +# own. The buckets must already exist (the init job does not create them). +# --------------------------------------------------------------------------- + +# Turn off the bundled MinIO object store; the services talk to external S3. +minio: + enabled: false + persistence: + enabled: false + +# External S3 object store shared by the app tier, LakeFS, and Lakekeeper. +storage: + s3: + # Regional S3 endpoint. For AWS S3 use https://s3.<region>.amazonaws.com. + endpoint: "https://s3.us-west-2.amazonaws.com"; + region: "us-west-2" + # Recommended: reference a pre-created Secret with keys access-key-id / + # secret-access-key (e.g. one synced from AWS Secrets Manager), and leave + # accessKeyId / secretAccessKey empty. Otherwise the chart creates a Secret + # named "<release>-s3-credentials" from the inline values below. + existingSecret: "" + accessKeyId: "REPLACE_WITH_ACCESS_KEY_ID" + secretAccessKey: "REPLACE_WITH_SECRET_ACCESS_KEY" + +# Point LakeFS at external S3: a region-only blockstore (the AWS SDK resolves +# the endpoint from the region) with credentials injected via the same Secret +# the chart/app uses. Override the secretKeyRef name if you set +# storage.s3.existingSecret above. +lakefs: + lakefsConfig: | + database: + type: postgres + blockstore: + type: s3 + s3: + region: us-west-2 + pre_signed_expiry: 15m + extraEnvVars: + - name: AWS_ACCESS_KEY_ID + valueFrom: + secretKeyRef: + name: texera-s3-credentials Review Comment: This hardcodes the release name (`texera-s3-credentials` = `<release>-s3-credentials` with release `texera`); with a different release name the secretKeyRef dangles and the LakeFS pod hits CreateContainerConfigError. Since a values file can't template `.Release.Name`, worth one more line in the comment above calling out the release-name assumption alongside the existingSecret one. ########## bin/k8s/templates/base/lakekeeper/lakekeeper-init-job.yaml: ########## @@ -85,14 +85,18 @@ spec: sleep 3 done - echo "Step 1: Initializing MinIO bucket '${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET}'..." +{{- if .Values.lakekeeperInit.createBucket }} + echo "Step 1: Initializing object-store bucket '${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET}'..." mc alias set minio "${STORAGE_S3_ENDPOINT}" "${STORAGE_S3_AUTH_USERNAME}" "${STORAGE_S3_AUTH_PASSWORD}" || true if mc ls minio/${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET} > /dev/null 2>&1; then - echo "MinIO bucket '${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET}' already exists." + echo "Bucket '${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET}' already exists." else - mc mb minio/${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET} - echo "MinIO bucket '${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET}' created successfully." + mc mb minio/${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET} || true Review Comment: The new `|| true` makes the "created successfully" log unconditional: on the default MinIO path, a genuine `mc mb` failure (perms, connectivity) now logs success and the error only surfaces later at warehouse registration, which is harder to trace back. Previously this failed fast. If the `|| true` is for an ls/mb race, `mc mb --ignore-existing` keeps idempotency without swallowing real failures. ########## bin/k8s/values.yaml: ########## @@ -78,6 +78,10 @@ storage: secretAccessKey: "" minio: + # Set to false to disable the in-cluster MinIO and point the services at an + # external S3 store instead (configure storage.s3 above and the lakefs/ + # lakekeeperInit blocks below). See values-aws.yaml for a complete example. + enabled: true Review Comment: Two more spots the new switch probably wants to cover: (1) `minio.enabled: false` with `storage.s3.endpoint` unset renders everything against `http://<release>-minio:9000` and the `<release>-minio` Secret via the `texera.s3.*` fallbacks — all dangling since the subchart is gone; a template-time `fail` on that combination would catch the misconfiguration at install time. (2) The MinIO HTTPRoute in `templates/base/gateway/gateway-routes.yaml` is gated only on `minio.gateway.enabled`, so with MinIO disabled and that flag on it renders a route to a nonexistent Service — `and`-ing it with `minio.enabled` would match what this PR already does for persistence. Not blocking. ########## bin/k8s/templates/base/lakekeeper/lakekeeper-init-job.yaml: ########## @@ -112,9 +116,14 @@ spec: "type": "s3", "bucket": "${STORAGE_ICEBERG_CATALOG_REST_S3_BUCKET}", "region": "${STORAGE_ICEBERG_CATALOG_REST_REGION}", +{{- if .Values.lakekeeperInit.warehouse.keyPrefix }} + "key-prefix": {{ .Values.lakekeeperInit.warehouse.keyPrefix | quote }}, +{{- end }} +{{- if eq .Values.lakekeeperInit.warehouse.flavor "s3-compat" }} "endpoint": "${STORAGE_S3_ENDPOINT}", - "flavor": "s3-compat", "path-style-access": true, +{{- end }} + "flavor": {{ .Values.lakekeeperInit.warehouse.flavor | quote }}, Review Comment: A mistyped `flavor` (anything other than `s3-compat`/`aws`) renders a warehouse profile with no endpoint and an unknown flavor string, and only fails at the registration call inside the job. A template-time guard would surface it at install time instead, e.g. `{{ if not (has .Values.lakekeeperInit.warehouse.flavor (list "s3-compat" "aws")) }}{{ fail "..." }}{{ end }}`. Not blocking. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
