adutra commented on code in PR #2001:
URL: https://github.com/apache/polaris/pull/2001#discussion_r2190419070
##########
helm/polaris/README.md.gotmpl:
##########
@@ -89,22 +71,7 @@ eval $(minikube -p minikube docker-env)
-Dquarkus.container-image.build=true
```
-### Installing the chart locally
-
-The below instructions assume a local Kubernetes cluster is running and Helm
is installed.
-
-#### Common setup
-
-Create the target namespace:
-
-```bash
-kubectl create namespace polaris
-```
-
-Create all the required resources in the `polaris` namespace. This usually
includes a Postgres
Review Comment:
Why remove this paragraph? I think it's still useful. We could instead just
remove the reference to persistence.xml and replace it with a reference to the
jdbc-persistence and the storage secrets.
##########
helm/polaris/README.md.gotmpl:
##########
@@ -113,67 +80,59 @@ Below are two sample deployment models for installing the
chart: one with a non-
> **These files are intended for testing purposes primarily, and may not be
> suitable for production use**.
> For production deployments, create your own values files based on the
> provided examples.
-#### Non-persistent backend
+##### Non-persistent backend
Install the chart with a non-persistent backend. From Polaris repo root:
-
```bash
helm upgrade --install --namespace polaris \
- --debug --values helm/polaris/ci/simple-values.yaml \
- polaris helm/polaris
+ --values helm/polaris/ci/simple-values.yaml \
+ polaris helm/polaris --create-namespace polaris
```
Note: if you are running the tests on a Kind cluster started with the `run.sh`
command explained
above, then you need to run `helm upgrade` as follows:
-
```bash
helm upgrade --install --namespace polaris \
- --debug --values helm/polaris/ci/simple-values.yaml \
+ --values helm/polaris/ci/simple-values.yaml \
--set=image.repository=localhost:5001/apache/polaris \
- polaris helm/polaris
+ polaris helm/polaris --create-namespace polaris
```
#### Persistent backend
> [!WARNING]
> The Postgres deployment set up in the fixtures directory is intended for
> testing purposes only and is not suitable for production use. For production
> deployments, use a managed Postgres service or a properly configured and
> secured Postgres instance.
-Install the chart with a persistent backend. From Polaris repo root:
+Install the dependencies in fixtures directory. From Polaris repo root:
Review Comment:
Hmm I don't like much the idea of telling users to install the fixtures in
the `helm/polaris/ci/fixtures/` directory. These fixtures are for tests only
and they are mostly bogus.
##########
helm/polaris/README.md.gotmpl:
##########
@@ -232,13 +182,11 @@ ct lint --charts helm/polaris
### Running the integration tests
-Integration tests require a Kubernetes cluster. See installation instructions
above for setting up
-a local cluster.
+Integration tests require a Kubernetes cluster. See installation instructions
above for setting upa local cluster.
Review Comment:
nit: unnecessary change? Also, there is a typo:
```suggestion
Integration tests require a Kubernetes cluster. See installation
instructions above for setting up a local cluster.
```
##########
run.sh:
##########
@@ -53,9 +48,11 @@ sh ./kind-registry.sh
# Build and deploy the server image
echo "Building polaris image..."
./gradlew \
- :polaris-server:build \
+ :polaris-server:assemble \
:polaris-server:quarkusAppPartsBuild --rerun \
- $ECLIPSE_LINK_DEPS \
+ :polaris-admin:assemble \
+ :polaris-admin:quarkusAppPartsBuild --rerun \
+ -Dquarkus.container-image.tag=postgres-latest \
Review Comment:
I think we don't need the special tag anymore. The default image is built
with the Postgres JDBC driver afaict.
##########
helm/polaris/templates/configmap.yaml:
##########
@@ -6,9 +6,9 @@
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
-
+
Review Comment:
Not a big deal, but you should strive not to introduce such changes since
it's not related to the PR.
##########
helm/polaris/README.md.gotmpl:
##########
@@ -198,27 +157,18 @@ The following tools are required to run the tests:
* [Chart Testing](https://github.com/helm/chart-testing)
Quick installation instructions for these tools:
-
```bash
helm plugin install https://github.com/helm-unittest/helm-unittest.git
brew install chart-testing
```
-The integration tests also require some fixtures to be deployed. The
`ci/fixtures` directory
-contains the required resources. To deploy them, run the following command:
+The integration tests also require some fixtures to be deployed. Follow the
above commands to setup required resources.
-```bash
-kubectl apply --namespace polaris -f helm/polaris/ci/fixtures/
-kubectl wait --namespace polaris --for=condition=ready pod
--selector=app.kubernetes.io/name=postgres --timeout=120s
-```
-
-The `helm/polaris/ci` contains a number of values files that will be used to
install the chart with
-different configurations.
+The `helm/polaris/ci` contains a number of values files that will be used to
install the chart with different configurations.
### Running the unit tests
-Helm unit tests do not require a Kubernetes cluster. To run the unit tests,
execute Helm Unit from
-the Polaris repo root:
+Helm unit tests do not require a Kubernetes cluster. To run the unit tests,
execute Helm Unit from the Polaris repo root:
Review Comment:
nit: Unnecessary change?
--
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]