Feature: Set NC_serverid per instance

[pschichtel]

Description of the change

Since NC 33 the UI recommends setting NC_serverid to a unique value per server of a nextcloud cluster. Since the ids should be stable and unique it suggests using a StatefulSet and using its ordinal as the serverid value.

Docs: https://docs.nextcloud.com/server/33/admin_manual/configuration_server/config_sample_php_parameters.html#serverid

Benefits

Not sure what the actual benefit is, but since NC suggests/recommends it, I guess the chart should comply.

Possible drawbacks

No idea if setting NC_serverid has drawbacks, but switching to a StatefulSet does make things a little more complicated.

[csantv]

I managed to silence it for now with this snippet, I'm also not sure the benefits of having a "proper" value.

nextcloud:
  configs:
    serverid.config.php: |
      <?php
      $podName = getenv('K8S_POD_ID') ?: '0';
      $numericId = abs(crc32($podName)) % 1024;
      $CONFIG = array( 
        'serverid' => $numericId,
      );
  extraEnv:
    - name: K8S_POD_ID
      valueFrom:
        fieldRef:
          fieldPath: metadata.name

It won't change for as long as the pod lives. Setting NC_serverid is not appropriate I think as each pod is a nextcloud server with its own config.

[csantv]

Ignore my previous comment. This should be a proper config using init containers.

nextcloud:
  extraInitContainers:
    - name: generate-serverid-config
      image: busybox
      env:
        - name: POD_NAME
          valueFrom:
            fieldRef:
              fieldPath: metadata.name
      command: ["/bin/sh", "-c"]
      args:
        - |
          ID=$(echo $POD_NAME | cksum | awk '{print $1 % 1024}')
          echo "<?php \$CONFIG = array('serverid' => $ID);" > /var/local/serverid.config.php
      volumeMounts:
        - name: nextcloud-serverid-config
          mountPath: "/var/local/"

  extraVolumes:
  - name: nextcloud-serverid-config
    emptyDir: {}

  extraVolumeMounts:
  - name: nextcloud-serverid-config
    mountPath: "/var/www/html/config/serverid.config.php"
    subPath: serverid.config.php

[pschichtel]

I wonder if it'd a problem, that this will produce new ids on every restart

[csantv]

That makes sense I think as pods are new nextcloud servers. You could also checksum the node name instead and have more stable values.

[lukashartl]

I also think that it would be best to use a StatefulSet instead. The ordinal numbers used in StatefulSets are exactly what would be needed for the NC_serverid variable.

I think there are two options here:

1. Switch the deployment to a StatefulSet by default. I don't know if there are any downsides to this for other users.
2. Add a variable to the values.md, called kind (or something similar) which is "Deployment" by default, or "StatefulSet". This would ensure that current setups don't break. If it is set to "StatefulSet", we could add the NC_serverid variable.

{{- if eq (include "nextcloud.workloadKind" .) "StatefulSet" }}
- name: NC_serverid
  valueFrom:
    fieldRef:
      fieldPath: metadata.labels['apps.kubernetes.io/pod-index']
{{- end }}

If we (and potentially the maintainers) can agree on an implementation, I would create the merge request and the tests for it, because I also need and want this change. 😄

[pschichtel]

probably saver to have it opt-in.