[Fixes #160] Bug: Error in tasks.py when initiating GeoNode container #161
Conversation
|
Looks good to me |
charts/geonode/README.md
Outdated
| @@ -147,7 +147,7 @@ Helm Chart for Geonode. Supported versions: Geonode: 4.1.3, Geoserver: 2.23.0, p | |||
| | geoserver.resources.limits.memory | string | `"4Gi"` | limits memory as in resource.limits.memory (https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) | | |||
| | geoserver.resources.requests.cpu | int | `1` | requested cpu as in resource.requests.cpu (https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) | | |||
| | geoserver.resources.requests.memory | string | `"1Gi"` | requested memory as in resource.requests.memory (https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) | | |||
| | geoserver.secret.admin_password | string | `"geoserver"` | geoserver admin password | | |||
| | geoserver.secret.admin_password | string | `"geoserver"` | geoserver admin password only gets only changed when the previous username/password combination is the default one (admin/geoserver) | | |||
There was a problem hiding this comment.
To be picky here: there is also GEOSERVER_FACTORY_PASSWORD. IMO, the naming could be better here, because this setting can be seen (more or less) as currently set password when you want to set a new GeoServer admin password.
If we want to limit this configuration to-be-set-once (as documented) I am fine with it, though.
There was a problem hiding this comment.
This thing got a new spin now. The password now gets changed by the Geserver entrypoint.sh, which wasn't working before because of the missing envvar: FORCE_REINIT. Therefor i removed the code to set the password from the geonode-k8s/tasks.py.
Further I added the factory_admin_password variable and gave some documentation on how to use it
There was a problem hiding this comment.
Do you want to stick with the factory_ part? To me it is actually the currently set password (initial or not) which can be used to set a new password. However, it is also very valid to keep naming in sync with the upstream.
There was a problem hiding this comment.
I would stick to the naming as it is used in the official image
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
…ting_GeoNode_container
| @@ -19,3 +19,4 @@ data: | |||
| DATABASE_PORT: "{{ include "database_port" . }}" | |||
| GEONODE_GEODATABASE: {{ .Values.postgres.geodata_databasename_and_username | quote }} | |||
| GEONODE_GEODATABASE_SCHEMA: {{ .Values.postgres.schema | quote }} | |||
| FORCE_REINIT: 'true' | |||
There was a problem hiding this comment.
Not sure if you really want to reconfigure GeoServer always .. TBH I do not know how geonode-k8s handles that flag on the other containers. It is used in the entrypoint.sh of geonode/celery containers at least.
There was a problem hiding this comment.
@ridoo for geoserver I don't think its a big deal. atm it only updates the password of the geoserver admin and updates the timestamp of the .lock file. And for celery its kind of the same, updating fixtures, updating the timestamp of the .lock file and updating the geonode admin password if it has changed.
In my opinion, except the .lock update everything is required at each pod start anyway, or at least dosn't destroys anything. Building a switch arround this variable would bring another layer of complexity into this chart. What i could image is to add an values.yaml parameter which is enabled by default but can be disabled like:
geonode:
force_reinit: true
geoserver:
force_reinit: true
|
Accidentally re-requested review .. you can just ignore it |
|
ignore readthedocs check for now. this will work as soon as PR #163 is merged |
…ting_GeoNode_container
Description
ref #160 fixes bug, while initiating geonode inside tasks.py. Fixing bugs in datastore creation and changing default geoserver admin password.
Type of Change
Please select the relevant option:
Related Issue
If there is an existing issue related to this pull request, please reference it here.
closes #160
Checklist
Please ensure that your pull request meets the following requirements:
Additional Notes
Any additional information or context regarding the pull request can be provided here.
Thank you for creating this pull request