Adding an optional deployment to increase AI pickup speed #688

kubeagon · 2025-01-23T20:19:11Z

Description of the change

The aim of this PR is to add the process described in increasing AI pickup speed in a simple but functional way to the helm-chart.

It functions similarly to the cronjob sidecar by mounting the volumes under nextcloud-main.

Benefits

The deployment is completely optional as it only has relevance to those who want to use the nextcloud-assistant or other AI integrations. Additionally this method allows for scaling by increasing the number of replicas of the deployment. (This is why I decided against using a container withing the main pod).

It makes use of the same image and volumes as the main nextcloud pod, so I don't think it will require additional maintenance unless something about how AI Tasks are handled changes in the future. Maybe we won't even need this anymore who knows :)

Possible drawbacks

None that I can think of.

Applicable issues

I didn't find any open issues concerning this functionality.

Additional information

I created a modified version of the nextcloud.volumeMounts helper template to reduce the required volumes that need to be mounted to the pods as much as possible.

Checklist

I have read the CONTRIBUTING.md doc.
DCO has been signed off on the commit.
Chart version bumped in Chart.yaml according to semver.
(optional) Parameters are documented in the README.md

Bumps [helm/chart-testing-action](https://github.com/helm/chart-testing-action) from 2.6.1 to 2.7.0. - [Release notes](https://github.com/helm/chart-testing-action/releases) - [Commits](helm/chart-testing-action@v2.6.1...v2.7.0) --- updated-dependencies: - dependency-name: helm/chart-testing-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Daniel Hendricken <[email protected]>

Signed-off-by: Daniel Hendricken <[email protected]>

…rker deployment Signed-off-by: Daniel Hendricken <[email protected]>

provokateurin

In principle this is a good idea, but the entire system is shifting away from being cron based, so I don't know how much sense it makes to add this now.

charts/nextcloud/README.md

Co-authored-by: Kate <[email protected]> Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon · 2025-01-23T20:50:13Z

Alright no worries, I add these workers per kustomization myself at the moment. Works like a charm. I made the PR because I thought it might increase the ease of use for others that decide to use the assistant.

Ah and I forgot to mention that I tested it with both the apache and fpm-alpine images.

wrenix

A good first try.

charts/nextcloud/templates/nextcloud-ai-worker.yaml

wrenix · 2025-01-24T08:56:26Z

charts/nextcloud/templates/nextcloud-ai-worker.yaml

+          volumeMounts:
+            {{- include "aiWorker.volumeMounts" . | trim | nindent 12 }}
+      volumes:
+        - name: nextcloud-main


I believe we should we move that into _helpers.tpl and reuse it with deployment of nextcloud.

So then we would divide the nextcloud.volumeMounts into for example:

nextcloud.volumeMounts

nextcloud.configMounts

Then include both for the nextcloud deployment and only nextcloud.volumeMounts for the ai-worker deployment

I mean to create a new one with nextcloud.volumes like i have done with: c7a0bc7

charts/nextcloud/templates/nextcloud-ai-worker.yaml

wrenix · 2025-01-24T08:59:57Z

charts/nextcloud/templates/_helpers.tpl

+
+
+{{/*
+Create volume mounts for the nextcloud-aiworker container.


Why is it needed without the defaultConfigs and configs like in nextcloud.volumeMounts ?

I believe we should use the same setup then in the main deployment.

My understanding is that the required config is written to disk by the main deployment. As the ai-worker deployment is dependent on the main deployment being ready I thought mounting them would be unnecessary.

Not all config are written down. The mounted one are read-only in the configmaps / secrets. So I believe you still need them.

Alright then I will make the adjustment

Ok, now it uses nexcloud.volumeMounts. I also corrected some of the templating logic for the init-container.

What do you correct on templating logic?

…me suffix '-ai-worker' Signed-off-by: Daniel Hendricken <[email protected]> chore(ai-worker): changed componant label to ai-worker and removed name suffix '-ai-worker'

Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon · 2025-02-06T16:36:04Z

Hello, this is still on my todo list, I've been rather busy lately. I think I might have time on saturday to implement the suggested changes.

Signed-off-by: Daniel Hendricken <[email protected]>

Signed-off-by: Daniel Hendricken <[email protected]> chore: uncommented dependencies in Chart.yaml

kubeagon · 2025-02-08T10:25:01Z

I can provide the values I used to test, if that's helpful.

Signed-off-by: WrenIX <[email protected]>

wrenix

Could you check if my changes works?
And maybe add your test-values in an test?
- therefore put a values.yaml here: https://github.com/nextcloud/helm/tree/main/charts/nextcloud/test-values
- call them as a testcase job in the github-actions matrix:
  
  helm/.github/workflows/lint-test.yaml
  
  Line 70 in 7785d69
  
  test_cases:

Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon · 2025-02-22T11:44:36Z

Could you check if my changes works?

And maybe add your test-values in an test?

therefore put a values.yaml here: https://github.com/nextcloud/helm/tree/main/charts/nextcloud/test-values

call them as a testcase job in the github-actions matrix:

helm/.github/workflows/lint-test.yaml

Line 70 in 7785d69

test_cases:

Tested your changes and added a test-case with values. Works like a charm :)

wrenix

So the codequality is nice, now. (i rebase is needed to current nextcloud main).

What do you think @provokateurin ?

wrenix · 2025-02-24T11:16:53Z

charts/nextcloud/templates/ai-worker.yaml

+            - sh
+            - -c
+            - >-
+              until curl -sf {{- if .Values.aiWorker.useHostName }}https://{{ .Values.nextcloud.host }}/status.php{{- else }}http://{{ template "nextcloud.fullname" . }}:{{ .Values.service.port }}/status.php{{- end }};


Do not know if it is better:

Suggested change

until curl -sf {{- if .Values.aiWorker.useHostName }}https://{{ .Values.nextcloud.host }}/status.php{{- else }}http://{{ template "nextcloud.fullname" . }}:{{ .Values.service.port }}/status.php{{- end }};

until curl -sf {{ ternary (printf "https://%s/status.php" .Values.nextcloud.host ) (printf "http://%s:%d/status.php" (template "nextcloud.fullname" .) .Values.service.port) .Values.aiWorker.useHostName }};

kubeagon · 2025-03-13T13:54:47Z

I'll rebase once the we agree on the changes. I didn't get a chance to check your suggested change yet.

Signed-off-by: Daniel Hendricken <[email protected]>

dependabot bot and others added 4 commits January 23, 2025 21:20

feat(ai-worker): adding a deployment for AI background workers

483e707

Signed-off-by: Daniel Hendricken <[email protected]>

chore(ai-worker): updated readme with ai-worker values

7fa1641

Signed-off-by: Daniel Hendricken <[email protected]>

chore(ai-worker): incremented chart minor version

3c08ff1

Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon force-pushed the main branch from 8b45080 to 9e6ed7a Compare January 23, 2025 20:21

chore(ai-worker): added more basic configuration options to the ai-wo…

7e92817

…rker deployment Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon force-pushed the main branch from 9e6ed7a to 7e92817 Compare January 23, 2025 20:25

Merge branch 'nextcloud:main' into main

70eea39

provokateurin reviewed Jan 23, 2025

View reviewed changes

charts/nextcloud/README.md Outdated Show resolved Hide resolved

corrected description of documentation link for AI tasks in readme

5985b81

Co-authored-by: Kate <[email protected]> Signed-off-by: Daniel Hendricken <[email protected]>

wrenix requested changes Jan 24, 2025

View reviewed changes

kubeagon force-pushed the main branch from f0cac18 to 0a8004a Compare January 24, 2025 18:04

chore(ai-worker): changed componant label to ai-worker and removed na…

207ab61

…me suffix '-ai-worker' Signed-off-by: Daniel Hendricken <[email protected]> chore(ai-worker): changed componant label to ai-worker and removed name suffix '-ai-worker'

kubeagon force-pushed the main branch from 4d2384d to 207ab61 Compare January 24, 2025 18:11

chore(ai-worker): made initContainer image configurable with values

0c0a47e

Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon force-pushed the main branch from 0e34866 to 0c0a47e Compare January 24, 2025 18:54

chore(ai-worker): corrected the quick-link for ai-tasks

2636c7e

Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon force-pushed the main branch from 34e564e to 2636c7e Compare January 24, 2025 19:54

chore(ai-worker): using nextcloud.volumeMounts for ai-worker deployment

81ba57b

Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon requested a review from wrenix February 8, 2025 10:16

chore(ai-worker): using nextcloud.volumeMounts for ai-worker deployment

505a9b6

Signed-off-by: Daniel Hendricken <[email protected]> chore: uncommented dependencies in Chart.yaml

kubeagon force-pushed the main branch from 1177fd6 to 505a9b6 Compare February 8, 2025 10:22

wrenix force-pushed the main branch 2 times, most recently from b47e79a to c7a0bc7 Compare February 12, 2025 15:25

wrenix added 2 commits February 12, 2025 16:32

chore: docs for helm-docs

939e5be

Signed-off-by: WrenIX <[email protected]>

fix: move volumes to helpers

081b741

Signed-off-by: WrenIX <[email protected]>

wrenix force-pushed the main branch from c7a0bc7 to 081b741 Compare February 12, 2025 15:32

wrenix reviewed Feb 12, 2025

View reviewed changes

chore(ai-worker) added added test-case with values

5e0abee

Signed-off-by: Daniel Hendricken <[email protected]>

kubeagon requested a review from wrenix February 22, 2025 11:46

wrenix approved these changes Feb 24, 2025

View reviewed changes

wrenix requested a review from provokateurin February 24, 2025 11:10

wrenix reviewed Feb 24, 2025

View reviewed changes

wrenix added the 0. Needs triage Pending approval or rejection. This issue is pending approval. label Mar 22, 2025

Merge branch 'main' into main

32c2e7f

Signed-off-by: Daniel Hendricken <[email protected]>



		{{/*
		Create volume mounts for the nextcloud-aiworker container.

	until curl -sf {{- if .Values.aiWorker.useHostName }}https://{{ .Values.nextcloud.host }}/status.php{{- else }}http://{{ template "nextcloud.fullname" . }}:{{ .Values.service.port }}/status.php{{- end }};
	until curl -sf {{ ternary (printf "https://%s/status.php" .Values.nextcloud.host ) (printf "http://%s:%d/status.php" (template "nextcloud.fullname" .) .Values.service.port) .Values.aiWorker.useHostName }};

Adding an optional deployment to increase AI pickup speed #688

Are you sure you want to change the base?

Adding an optional deployment to increase AI pickup speed #688

Uh oh!

Conversation

kubeagon commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

Checklist

Uh oh!

provokateurin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kubeagon commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wrenix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wrenix Jan 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kubeagon commented Feb 6, 2025

Uh oh!

kubeagon commented Feb 8, 2025

Uh oh!

wrenix left a comment

Choose a reason for hiding this comment

Uh oh!

kubeagon commented Feb 22, 2025

Uh oh!

wrenix left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kubeagon commented Mar 13, 2025

Uh oh!

Uh oh!

kubeagon commented Jan 23, 2025 •

edited

Loading

kubeagon commented Jan 23, 2025 •

edited

Loading

wrenix Jan 25, 2025 •

edited

Loading