Add Helm job for OpenSearch index creation #36
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,66 @@ | ||||||||||
| # Copyright The Linux Foundation and each contributor to LFX. | ||||||||||
| # SPDX-License-Identifier: MIT | ||||||||||
| {{- if .Values.opensearch.indexingJob.enabled }} | ||||||||||
| --- | ||||||||||
| apiVersion: v1 | ||||||||||
| kind: ConfigMap | ||||||||||
| metadata: | ||||||||||
| name: opensearch-index-config | ||||||||||
|
bramwelt marked this conversation as resolved.
Outdated
|
||||||||||
| namespace: {{ .Release.Namespace }} | ||||||||||
|
Comment on lines
+8
to
+9
Contributor
Author
There was a problem hiding this comment. The Job and ConfigMap use
Comment on lines
+8
to
+9
Contributor
Author
There was a problem hiding this comment. The Job and ConfigMap use |
||||||||||
| data: | ||||||||||
| resources-index.json: | | ||||||||||
|
Contributor
There was a problem hiding this comment. This index json content doesn't quite match what we should be actually using, which is https://github.com/linuxfoundation/lfx-architecture-scratch/blob/main/2024-12%20ReBAC%20Demo/opensearch-resources-index.json. But on that note, could we have this json content in a file instead and then have it read into the k8 template? I think that would be better.
Contributor
Author
There was a problem hiding this comment. Thanks for the feedback! I realized I hadn't pulled the repo for a while and was using an outdated index. I've moved the index JSON to it's own file and have it being included now with
Contributor
Author
There was a problem hiding this comment. Both concerns are fully addressed — the index JSON was updated (commit 8971941) to match the correct schema and moved to |
||||||||||
| { | ||||||||||
| "settings": { | ||||||||||
| "index": { | ||||||||||
| "number_of_shards": 1, | ||||||||||
| "number_of_replicas": 1 | ||||||||||
|
There was a problem hiding this comment. The index settings (number_of_shards and number_of_replicas) are hardcoded in the ConfigMap. Consider making these configurable through values.yaml to allow users to adjust shard and replica counts based on their cluster size and requirements without modifying the ConfigMap template directly.
Suggested change
Contributor
Author
There was a problem hiding this comment. Addressed — the index JSON was moved to |
||||||||||
| } | ||||||||||
| }, | ||||||||||
| "mappings": { | ||||||||||
| "properties": { | ||||||||||
| "object_ref": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "object_type": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "object_id": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "latest": { | ||||||||||
| "type": "boolean" | ||||||||||
| }, | ||||||||||
| "public": { | ||||||||||
| "type": "boolean" | ||||||||||
| }, | ||||||||||
| "parent_refs": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "name_and_aliases": { | ||||||||||
| "type": "search_as_you_type" | ||||||||||
| }, | ||||||||||
| "tags": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "access_check_object": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "access_check_relation": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "history_check_object": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "history_check_relation": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "access_check_query": { | ||||||||||
| "type": "keyword" | ||||||||||
| }, | ||||||||||
| "history_check_query": { | ||||||||||
| "type": "keyword" | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| {{- end }} | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,34 @@ | ||||||||||
| # Copyright The Linux Foundation and each contributor to LFX. | ||||||||||
| # SPDX-License-Identifier: MIT | ||||||||||
| {{- if .Values.opensearch.indexingJob.enabled }} | ||||||||||
| --- | ||||||||||
| apiVersion: batch/v1 | ||||||||||
| kind: Job | ||||||||||
| metadata: | ||||||||||
| name: opensearch-index-setup | ||||||||||
|
There was a problem hiding this comment. The Job name is hardcoded and doesn't include the Release name, which could cause conflicts if multiple releases of this chart are installed in the same namespace. Consider using a template that includes the release name, such as
Suggested change
Contributor
Author
There was a problem hiding this comment. Already addressed — both the Job and ConfigMap names use
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||
| namespace: {{ .Release.Namespace }} | ||||||||||
| spec: | ||||||||||
| ttlSecondsAfterFinished: 300 | ||||||||||
| backoffLimit: 3 | ||||||||||
|
There was a problem hiding this comment. The Job's ttlSecondsAfterFinished (300 seconds) and backoffLimit (3) are hardcoded. Consider making these values configurable through values.yaml (e.g.,
Suggested change
Contributor
Author
There was a problem hiding this comment. Already addressed — |
||||||||||
| template: | ||||||||||
| spec: | ||||||||||
| restartPolicy: Never | ||||||||||
| volumes: | ||||||||||
| - name: index-config | ||||||||||
| configMap: | ||||||||||
| name: opensearch-index-config | ||||||||||
|
There was a problem hiding this comment. The ConfigMap reference uses a hardcoded name that should match the ConfigMap resource name. If the ConfigMap name is updated to include the Release name (as recommended), this reference must also be updated to use the same templated name to ensure proper binding.
Suggested change
Contributor
Author
There was a problem hiding this comment. Already addressed — the ConfigMap volume reference in the Job matches the ConfigMap name, both using
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||
| containers: | ||||||||||
| - name: curl | ||||||||||
| image: curlimages/curl:8.11.1 | ||||||||||
|
There was a problem hiding this comment. The curl image and version are hardcoded in the Job template. Consider making the image repository and tag configurable through values.yaml (e.g.,
Suggested change
Contributor
Author
There was a problem hiding this comment. Already addressed — the curl image is configurable via |
||||||||||
| volumeMounts: | ||||||||||
| - name: index-config | ||||||||||
| mountPath: /config | ||||||||||
| readOnly: true | ||||||||||
|
Comment on lines
+29
to
+62
There was a problem hiding this comment. The Job container doesn't define a securityContext, while the main application deployment includes one with
Contributor
Author
There was a problem hiding this comment. Fixed in commit 1a85468 — added |
||||||||||
| command: | ||||||||||
| - sh | ||||||||||
| - -c | ||||||||||
| - | | ||||||||||
| curl -X PUT "{{ .Values.opensearch.url | trimSuffix "/" }}/{{ .Values.opensearch.index }}" \ | ||||||||||
|
bramwelt marked this conversation as resolved.
Outdated
bramwelt marked this conversation as resolved.
Outdated
|
||||||||||
| -H 'Content-Type: application/json' \ | ||||||||||
| -d @/config/resources-index.json | ||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||
| {{- end }} | ||||||||||
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.
The ConfigMap name is hardcoded and doesn't include the Release name, which could cause conflicts if multiple releases of this chart are installed in the same namespace. Consider using a template that includes the release name, such as
{{ include "lfx-v2-indexer-service.fullname" . }}-index-configor{{ .Release.Name }}-opensearch-index-configto ensure uniqueness across releases.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.
Already addressed — the ConfigMap name uses
{{ .Release.Name }}-opensearch-index-config.