Move atlantis-data volume to a separate PVC

[BrunoTarijon]

what

Remove the atlantis-data volume creation from the sts and add a separate PVC.

why

We can not change the storage size without deleting the deployed sts.

tests

I have tested the logic using the helm template function to generate the manifests with the volumeClaim interface and the deprecated .Values.dataStorage.

[GenPage]

@BrunoTarijon LGTM, can you bump the chart version? It can be just a patch bump.

[BrunoTarijon]

@BrunoTarijon LGTM, can you bump the chart version? It can be just a patch bump.

@GenPage ok, done

[bsvartz]

@BrunoTarijon, @GMartinez-Sisti - is it possible to add the option to edit on this file:

https://github.com/runatlantis/helm-charts/blob/f56c4caa0fe5c83052d4f0b42f67c0d7f0e9d122/charts/atlantis/templates/pvc.yaml

spec:
  accessModes:
    - ReadWriteOnce

to ReadWriteMany - in order to support the following feature? runatlantis/atlantis#3795?

and this:

  volumeClaimTemplates:
  - metadata:
      name: atlantis-data
    spec:
      accessModes: ["ReadWriteOnce"] # Volume should not be shared by multiple nodes.

on this file:
https://github.com/runatlantis/helm-charts/blob/f56c4caa0fe5c83052d4f0b42f67c0d7f0e9d122/charts/atlantis/templates/statefulset.yaml

Generally - i want to be able to share the same volume between multiple atlantis instances.

[BrunoTarijon]

@bsvartz I have added the requested feature.

@GenPage. Does it look good to you? Can we merge it?

[GMartinez-Sisti]

@bsvartz I have added the requested feature.

@GenPage. Does it look good to you? Can we merge it?

I think the idea was to make it configurable in the values file. Leaving the default as it was before, but you configure what you want.

[jamengual]

@BrunoTarijon could you fix the conflicts? Thanks.

[matilote]

Any chance to resolve the conflicts and get this merged?

[BrunoTarijon]

Sorry for not seeing this for a while. I have made the custom access mode in the PVC and chose not to change the STS configuration for the access mode because it is marked as deprecated.

@jamengual I have resolved the conflicts.

[github-actions]

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

[BrunoTarijon]

Hey guys (@jamengual @GenPage) just receive the Stale label, could you review this PR?

[jamengual]

@GMartinez-Sisti

[jamengual]

@BrunoTarijon we have unit test now, could you take a look?

[BrunoTarijon]

Done @jamengual

[GMartinez-Sisti]

@BrunoTarijon looks like something needs to be fixed. The error is:

4m33s Warning FailedCreate statefulset/atlantis-wbp015wolz statefulset-controller create Pod atlantis-wbp015wolz-0 in StatefulSet atlantis-wbp015wolz failed error: Pod "atlantis-wbp015wolz-0" is invalid: spec.containers[0].volumeMounts[0].name: Not found: "atlantis-data" 10m 17 atlantis-wbp015wolz.17b9e455ea79e80f

Please adjust the ci files on https://github.com/runatlantis/helm-charts/tree/main/charts/atlantis/ci and create a new one to include your change. The only requirement is that filenames end with *-values.yaml.

Let us know if you need help 😃

[BrunoTarijon]

Fix the test by adding the volume to the sts.

[BrunoTarijon]

Testing the specific volume for each test.

[BrunoTarijon]

Please adjust the ci files on https://github.com/runatlantis/helm-charts/tree/main/charts/atlantis/ci and create a new one to include your change. The only requirement is that filenames end with *-values.yaml.

Don't get it. This feat should work with the default values.

cc: @GMartinez-Sisti

[GMartinez-Sisti]

Please adjust the ci files on https://github.com/runatlantis/helm-charts/tree/main/charts/atlantis/ci and create a new one to include your change. The only requirement is that filenames end with *-values.yaml.

Don't get it. This feat should work with the default values.

cc: @GMartinez-Sisti

Fair enough. Now the tests are passing.

[cilindrox]

This should've been a major release - breaks existing installs using the STS pv

[bergemalm]

Yep, breaking change!

[GMartinez-Sisti]

This should've been a major release - breaks existing installs using the STS pv

Yep, breaking change!

In hindsight, you are both right. Our tests install everything from scratch, but they don't contemplate upgrades.

I tested by installing 4.21.1 and then tried to upgrade to 4.22.0 and got an error:

→ helm install atlantis runatlantis/atlantis --version 4.21.1
NAME: atlantis
LAST DEPLOYED: Fri Apr  5 18:59:30 2024
NAMESPACE: default
STATUS: deployed
REVISION: 1
NOTES:
1. Get the application URL by running these commands:
2. Atlantis will not start successfully unless at least one of the following sets of credentials are specified (see values.yaml for detailed usage):
  - github
  - githubApp
  - gitlab
  - bitbucket

→ helm upgrade atlantis runatlantis/atlantis --version 4.22.0
W0405 18:59:46.772125   48368 warnings.go:70] unknown field "labels"
Error: UPGRADE FAILED: cannot patch "atlantis" with kind StatefulSet: StatefulSet.apps "atlantis" is invalid: spec: Forbidden: updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden

We might create a new major release with update steps to fix this. Discussing with the other maintainers. 🙏