-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Adapt local setup documentations #297
docs: Adapt local setup documentations #297
Conversation
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -64,10 +64,9 @@ helm install plato charts/tractusx-connector \ | |||
--namespace cx \ | |||
--create-namespace \ | |||
--set fullnameOverride=plato \ | |||
--set controlplane.image.tag=0.2.0 \ | |||
--set controlplane.image.tag=latest \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using latest
is dangerous, because the latest
tag gets overwritten on every build we do on tractusx-edc, which may result in unstable runtime behaviour for users
@@ -14,7 +14,7 @@ Prerequisites: | |||
## 2. Install the all-in-one supporting infrastructure environment (Daps, Vault, PostgreSql, Minio, Backend-Service) | |||
|
|||
```shel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we fix the format type as "sh" or "shell"?
@@ -33,7 +33,7 @@ Before the connectors can be setup, the Supporting Infrastructure must be in pla | |||
to run two connectors independently. | |||
|
|||
For this local test scenario, | |||
the [Supporting Infrastructure](../../edc-tests/src/main/resources/deployment/helm/supporting-infrastructure/README.md) | |||
the [Supporting Infrastructure](../../edc-tests/cucumber/src/main/resources/deployment/helm/supporting-infrastructure/README.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just like this fix, should we also insert "cucumber" into the following two paths?
@@ -64,10 +64,9 @@ helm install plato charts/tractusx-connector \ | |||
--namespace cx \ | |||
--create-namespace \ | |||
--set fullnameOverride=plato \ | |||
--set controlplane.image.tag=0.2.0 \ | |||
--set controlplane.image.tag=latest \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried the fixed command, but ran into the following error unfortunately.
Error: INSTALLATION FAILED: execution error at (tractusx-connector/templates/deployment-controlplane.yaml:191:54): .Values.postgresql.username is required
Considering the context that PostgreSQL is disabled in the preceding infrastructure setup, how about adding the following option?
--set controlplane.image.repository=edc-controlplane-memory-hashicorp-vault
In addition, even if specifying the option above, postgresql.username
, postgresql.password
, and postgresql.jdbcUrl
still seem to be required, though any value is OK for them. According to the document, there is a postgresql.enabled
option, but it doesn't seem to be used in any Helm chart.
$ git grep postgresql.enabled
.github/workflows/business-tests.yaml: --set postgresql.enabled=true \
.github/workflows/business-tests.yaml: --set postgresql.enabled=true \
charts/tractusx-connector-app/README.md:| runtime.postgresql.enabled | bool | `false` | |
charts/tractusx-connector/README.md:| postgresql.enabled | bool | `false` | |
docs/development/Run-business-tests-local.md: --set postgresql.enabled=true \
docs/development/Run-business-tests-local.md: --set postgresql.enabled=true \
Is this a documentation bug, and if so, should we fix it in this PR?
dfe0701
to
71f4660
Compare
WHAT
Adapt local setup documentation.
WHY
Some values in some local setup document were outdated.
FURTHER NOTES
Closes #286