From 91b97d9cd12e2b0b009624a5baa08527ac39ea1d Mon Sep 17 00:00:00 2001 From: shreddedbacon Date: Fri, 26 Jul 2024 10:29:02 +1000 Subject: [PATCH] refactor: dont create volumes that arent referenced by a service --- internal/generator/buildvalues.go | 5 ++-- internal/generator/services.go | 2 +- internal/generator/volumes.go | 9 ++++-- internal/templating/services/templates_pvc.go | 26 +++++++++-------- .../docker-compose.multiple-volumes-2.yml | 2 +- .../basic/docker-compose.multiple-volumes.yml | 2 +- .../service7/pvc-custom-notused.yaml | 29 ------------------- .../service8/pvc-custom-notused.yaml | 29 ------------------- 8 files changed, 26 insertions(+), 78 deletions(-) delete mode 100644 internal/testdata/basic/service-templates/service7/pvc-custom-notused.yaml delete mode 100644 internal/testdata/basic/service-templates/service8/pvc-custom-notused.yaml diff --git a/internal/generator/buildvalues.go b/internal/generator/buildvalues.go index 417ca393..db2da9be 100644 --- a/internal/generator/buildvalues.go +++ b/internal/generator/buildvalues.go @@ -146,8 +146,9 @@ type ImageCacheBuildArguments struct { } type ComposeVolume struct { - Name string `json:"name" description:"name is the name the volume, when creating in kubernetes will have a prefix"` - Size string `json:"size" description:"the size of the volume to request if the system enforces it"` + Name string `json:"name" description:"name is the name the volume, when creating in kubernetes will have a prefix"` + Size string `json:"size" description:"the size of the volume to request if the system enforces it"` + Create bool `json:"unused" description:"flag to determine if this volume is to be created or not"` } type ServiceVolume struct { diff --git a/internal/generator/services.go b/internal/generator/services.go index 76ff54b9..44736115 100644 --- a/internal/generator/services.go +++ b/internal/generator/services.go @@ -372,7 +372,7 @@ func composeToServiceValues( // calculate if this service needs any additional volumes attached from the calculated build volumes // additional volumes can only be attached to certain - serviceVolumes, err := calculateServiceVolumes(buildValues.Volumes, lagoonType, servicePersistentName, composeServiceValues.Labels) + serviceVolumes, err := calculateServiceVolumes(buildValues, lagoonType, servicePersistentName, composeServiceValues.Labels) if err != nil { return nil, err } diff --git a/internal/generator/volumes.go b/internal/generator/volumes.go index 43f9dd5d..8b95842c 100644 --- a/internal/generator/volumes.go +++ b/internal/generator/volumes.go @@ -88,10 +88,11 @@ func composeToVolumeValues( // calculateServiceVolumes checks if a service type is allowed to have additional volumes attached, and if any volumes from docker compose // are to be attached to this service or not -func calculateServiceVolumes(buildVolumes []ComposeVolume, lagoonType, servicePersistentName string, serviceLabels composetypes.Labels) ([]ServiceVolume, error) { +func calculateServiceVolumes(buildValues *BuildValues, lagoonType, servicePersistentName string, serviceLabels composetypes.Labels) ([]ServiceVolume, error) { serviceVolumes := []ServiceVolume{} if val, ok := servicetypes.ServiceTypes[lagoonType]; ok { - for _, vol := range buildVolumes { + for i := 0; i < len(buildValues.Volumes); i++ { + vol := &buildValues.Volumes[i] volName := lagoon.GetVolumeNameFromLagoonVolume(vol.Name) // if there is a `lagoon.volumes..path` for a custom volume that matches the default persistent volume // don't add it again as a service volume @@ -100,9 +101,11 @@ func calculateServiceVolumes(buildVolumes []ComposeVolume, lagoonType, servicePe if volumePath != "" { if val.AllowAdditionalVolumes { sVol := ServiceVolume{ - ComposeVolume: vol, + ComposeVolume: *vol, Path: volumePath, } + // if the volume is attached to a service, then add the flag to create the persistent volume claim + vol.Create = true serviceVolumes = append(serviceVolumes, sVol) } else { // if the service type is not allowed additional volumes, return an error diff --git a/internal/templating/services/templates_pvc.go b/internal/templating/services/templates_pvc.go index fb9827ef..99d47b24 100644 --- a/internal/templating/services/templates_pvc.go +++ b/internal/templating/services/templates_pvc.go @@ -65,20 +65,22 @@ func GeneratePVCTemplate( } // check for any additional volumes for _, vol := range buildValues.Volumes { - exists := false - // check if an existing pvc that will be created as a default persistent volume hasn't also been defined as an additional volume - // this will prevent creating another volume named the same - for _, cpvc := range result { - if lagoon.GetLagoonVolumeName(cpvc.Name) == vol.Name { - exists = true + if vol.Create { + exists := false + // check if an existing pvc that will be created as a default persistent volume hasn't also been defined as an additional volume + // this will prevent creating another volume named the same + for _, cpvc := range result { + if lagoon.GetLagoonVolumeName(cpvc.Name) == vol.Name { + exists = true + } } - } - if !exists { - pvc, err := generateAdditionalPVC(buildValues, vol, labels, annotations) - if err != nil { - return nil, err + if !exists { + pvc, err := generateAdditionalPVC(buildValues, vol, labels, annotations) + if err != nil { + return nil, err + } + result = append(result, *pvc) } - result = append(result, *pvc) } } return result, nil diff --git a/internal/testdata/basic/docker-compose.multiple-volumes-2.yml b/internal/testdata/basic/docker-compose.multiple-volumes-2.yml index 86bea115..59076c3d 100644 --- a/internal/testdata/basic/docker-compose.multiple-volumes-2.yml +++ b/internal/testdata/basic/docker-compose.multiple-volumes-2.yml @@ -45,7 +45,7 @@ volumes: lagoon.type: none notused: labels: - # will create a pvc, but as there is no `lagoon.volumes.notused.path` defined anywhere, it will not be mounted + # as there is no `lagoon.volumes.notused.path` defined anywhere, it will not be created or mounted lagoon.type: persistent logs: {} \ No newline at end of file diff --git a/internal/testdata/basic/docker-compose.multiple-volumes.yml b/internal/testdata/basic/docker-compose.multiple-volumes.yml index 90f7ec34..851a7612 100644 --- a/internal/testdata/basic/docker-compose.multiple-volumes.yml +++ b/internal/testdata/basic/docker-compose.multiple-volumes.yml @@ -47,7 +47,7 @@ volumes: lagoon.type: none notused: labels: - # will create a pvc, but as there is no `lagoon.volumes.notused.path` defined anywhere, it will not be mounted + # as there is no `lagoon.volumes.notused.path` defined anywhere, it will not be created or mounted lagoon.type: persistent logs: {} \ No newline at end of file diff --git a/internal/testdata/basic/service-templates/service7/pvc-custom-notused.yaml b/internal/testdata/basic/service-templates/service7/pvc-custom-notused.yaml deleted file mode 100644 index 3b092073..00000000 --- a/internal/testdata/basic/service-templates/service7/pvc-custom-notused.yaml +++ /dev/null @@ -1,29 +0,0 @@ ---- -apiVersion: v1 -kind: PersistentVolumeClaim -metadata: - annotations: - k8up.io/backup: "true" - k8up.syn.tools/backup: "true" - lagoon.sh/branch: main - lagoon.sh/version: v2.7.x - creationTimestamp: null - labels: - app.kubernetes.io/instance: custom-notused - app.kubernetes.io/managed-by: build-deploy-tool - app.kubernetes.io/name: notused - lagoon.sh/buildType: branch - lagoon.sh/environment: main - lagoon.sh/environmentType: production - lagoon.sh/project: example-project - lagoon.sh/service-type: additional-volume - lagoon.sh/template: additional-volume-0.1.0 - name: custom-notused -spec: - accessModes: - - ReadWriteMany - resources: - requests: - storage: 5Gi - storageClassName: bulk -status: {} diff --git a/internal/testdata/basic/service-templates/service8/pvc-custom-notused.yaml b/internal/testdata/basic/service-templates/service8/pvc-custom-notused.yaml deleted file mode 100644 index 3b092073..00000000 --- a/internal/testdata/basic/service-templates/service8/pvc-custom-notused.yaml +++ /dev/null @@ -1,29 +0,0 @@ ---- -apiVersion: v1 -kind: PersistentVolumeClaim -metadata: - annotations: - k8up.io/backup: "true" - k8up.syn.tools/backup: "true" - lagoon.sh/branch: main - lagoon.sh/version: v2.7.x - creationTimestamp: null - labels: - app.kubernetes.io/instance: custom-notused - app.kubernetes.io/managed-by: build-deploy-tool - app.kubernetes.io/name: notused - lagoon.sh/buildType: branch - lagoon.sh/environment: main - lagoon.sh/environmentType: production - lagoon.sh/project: example-project - lagoon.sh/service-type: additional-volume - lagoon.sh/template: additional-volume-0.1.0 - name: custom-notused -spec: - accessModes: - - ReadWriteMany - resources: - requests: - storage: 5Gi - storageClassName: bulk -status: {}