aicam commented on code in PR #6295:
URL: https://github.com/apache/texera/pull/6295#discussion_r3574049988
##########
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:
Good catch, fixed in 668409f30. Replaced `mc mb ... || true` with `mc mb
--ignore-existing`, which keeps idempotency on the already-exists case but lets
a genuine failure (perms, connectivity) still exit non-zero and abort the job
under `set -e`. Also dropped the now-redundant `mc ls` guard (the block reduces
to `mc mb --ignore-existing` + a "is ready" log) and moved the `mc` client
install into this same `createBucket=true` branch so the external-S3 path no
longer downloads it at all.
##########
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:
Fixed in 668409f30. Added a template-time guard at the top of the job so a
mistyped flavor fails the render at install time with a clear message instead
of at warehouse registration: `{{- if not (has
.Values.lakekeeperInit.warehouse.flavor (list "s3-compat" "aws")) }}{{- fail
... }}{{- end }}`. Verified `helm template` now errors on e.g. `--set
lakekeeperInit.warehouse.flavor=awss`.
##########
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:
Fixed in 668409f30. Added a NOTE above the block spelling out that
`texera-s3-credentials` assumes the release is named `texera` (the chart
generates the Secret as `<release>-s3-credentials`), and to update both `name:`
fields when installing under a different release name or when setting
`storage.s3.existingSecret`. A values file can't template `.Release.Name`, so
documenting the assumption is the safe fix here.
##########
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:
Thanks, both are valid observations. We decided to leave both as-is for now
because each only triggers when `minio.enabled=false`, i.e. the external-S3
path, which is not an officially supported deployment yet (the supported
default is local/on-prem with the in-cluster MinIO enabled, per the discussion
above on #5891). On the default install neither case is reachable. Concretely:
(1) the `minio.enabled=false` + unset `storage.s3.endpoint` combination is a
hand-rolled half-config that a local deployment never produces, and the
`values-aws.yaml` overlay always sets both; (2) the MinIO HTTPRoute (and the
matching Gateway listener in `gateway.yaml`) stay gated on
`minio.gateway.enabled`, which defaults to false, so a local deploy renders no
MinIO route regardless. Happy to add the template-time `fail` and the `and
minio.enabled` gate in a follow-up if/when external S3 becomes a first-class
supported profile.
--
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]
