Merge pull request 2i2c-org#4460 from sgibson91/gcp-filestore-backups…

…/multiple-filestores Allow gcp-filestore-backups to backup multiple filestores
sean-morris · Jul 19, 2024 · fdbd5c5 · fdbd5c5
2 parents b018732 + 51bb699
commit fdbd5c5
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 29 deletions.
diff --git a/config/clusters/2i2c/support.values.yaml b/config/clusters/2i2c/support.values.yaml
@@ -70,7 +70,8 @@ grafana:
 
 gcpFilestoreBackups:
   enabled: true
-  filestoreName: pilot-hubs-homedirs
+  filestoreNames:
+    - pilot-hubs-homedirs
   project: two-eye-two-see
   zone: us-central1-b
   annotations:

diff --git a/docs/howto/decrease-size-gcp-filestore.md b/docs/howto/decrease-size-gcp-filestore.md
@@ -38,6 +38,12 @@ terraform plan -var-file=projects/$CLUSTER_NAME.tfvars
 terraform apply -var-file=projects/$CLUSTER_NAME.tfvars
 ```
 
+```{note}
+If filestore backups are enabled for this cluster, don't forget to add the name
+of the new filestore to the cluster's support values file, following
+[the instructions](howto:filesystem-backups:enable:gcp).
+```
+
 Open a PR and merge these changes so that other engineers cannot accidentally overwrite them.
 
 ## 2. Create a VM

diff --git a/docs/howto/filesystem-backups/enable-backups.md b/docs/howto/filesystem-backups/enable-backups.md
@@ -39,15 +39,17 @@ export CLUSTER_NAME=<cluster-name>
       ```yaml
       gcpFilestoreBackups:
         enabled: true
-        filestoreName: <filestore-name>
+        filestoreNames:
+          - <filestore-name>
+          - ...
         project: <gcp-project>
         zone: <gcp-zone>
         annotations:
           iam.gke.io/gcp-service-account: <gcp-service-account-email>
       ```
       where:
-      - `filestoreName` is the name of the filestore to be backed up (can be found
-        from the Filestore Instances page in the GCP console)
+      - `filestoreNames` is a list of the filestore names to be backed up (can be
+        found from the Filestore Instances page in the GCP console)
       - `project` is the name of the GCP project in which the filestore exists
       - `zone` is the GCP zone the filestore is deployed to and where the backups
         will be stored (e.g. `us-central-b`)

diff --git a/helm-charts/images/gcp-filestore-backups/gcp-filestore-backups.py b/helm-charts/images/gcp-filestore-backups/gcp-filestore-backups.py
@@ -86,7 +86,9 @@ def filter_backups_into_recent_and_old(
         retention_days (int): The number of days above which a backup is considered
             to be out of date
         day_freq (int, optional): The time period in days for which we create a
-            backup. Defaults to 1 (ie. daily backups).
+            backup. Defaults to 1 (ie. daily backups). NOTE: The frequency at
+            which we make backups is not yet configurable on the command line,
+            but could be if required.
 
     Returns:
         recent_backups (list(dict)): A JSON-like object containing all existing
@@ -192,21 +194,23 @@ def delete_old_backups(backups: list, region: str):
 
 def main(args):
     region = extract_region_from_zone(args.zone)
-    filestore_backups = get_existing_backups(
-        args.project, region, args.filestore_name, args.filestore_share_name
-    )
-    recent_filestore_backups, old_filestore_backups = (
-        filter_backups_into_recent_and_old(filestore_backups, args.retention_days)
-    )
-    create_backup_if_necessary(
-        recent_filestore_backups,
-        args.filestore_name,
-        args.filestore_share_name,
-        args.project,
-        region,
-        args.zone,
-    )
-    delete_old_backups(old_filestore_backups, region)
+
+    for filestore_name in args.filestore_names:
+        filestore_backups = get_existing_backups(
+            args.project, region, filestore_name, args.filestore_share_name
+        )
+        recent_filestore_backups, old_filestore_backups = (
+            filter_backups_into_recent_and_old(filestore_backups, args.retention_days)
+        )
+        create_backup_if_necessary(
+            recent_filestore_backups,
+            filestore_name,
+            args.filestore_share_name,
+            args.project,
+            region,
+            args.zone,
+        )
+        delete_old_backups(old_filestore_backups, region)
 
 
 if __name__ == "__main__":
@@ -217,7 +221,9 @@ def main(args):
     )
 
     parser.add_argument(
-        "filestore_name", type=str, help="The name of the GCP Filestore to backup"
+        "filestore_names",
+        nargs="+",
+        help="The name of one or more GCP Filestores to backup",
     )
     parser.add_argument(
         "project",
@@ -229,6 +235,15 @@ def main(args):
         type=str,
         help="The GCP zone the Filestore is deployed in, e.g. us-central1-b",
     )
+
+    # NOTE: We assume that the share name will be homes on all GCP filestores
+    #       right now, which is a safe assumption given that this is not configurable
+    #       in our terraform code:
+    #
+    #           https://github.com/2i2c-org/infrastructure/blob/HEAD/terraform/gcp/storage.tf
+    #
+    #       We should change this if that value becomes configurable.
+    #
     parser.add_argument(
         "--filestore-share-name",
         type=str,

diff --git a/helm-charts/support/templates/gcp-filestore-backups/deployment.yaml b/helm-charts/support/templates/gcp-filestore-backups/deployment.yaml
@@ -2,7 +2,7 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  name: gcp-filestore-backups
+  name: {{ .Release.Name }}-gcp-filestore-backups
 spec:
   replicas: 1
   strategy:
@@ -18,13 +18,15 @@ spec:
       serviceAccountName: gcp-filestore-backups-sa
       automountServiceAccountToken: false
       containers:
-        - name: gcp-filestore-backups
+        - name: {{ .Release.Name }}-gcp-filestore-backups
           image: '{{ .Values.gcpFilestoreBackups.image }}'
           command:
             - python
             - gcp-filestore-backups.py
           args:
-            - '{{ .Values.gcpFilestoreBackups.filestoreName | required "gcpFilestoreBackups.filestoreName is required with gcpFilestoreBackups.enabled set to true" }}'
+            {{- range .Values.gcpFilestoreBackups.filestoreNames | required "gcpFilestoreBackups.filestoreNames is required with gcpFilestoreBackups.enabled set to true" }}
+            - '{{ . }}'
+            {{- end }}
             - '{{ .Values.gcpFilestoreBackups.project | required "gcpFilestoreBackups.project is required with gcpFilestoreBackups.enabled set to true" }}'
             - '{{ .Values.gcpFilestoreBackups.zone | required "gcpFilestoreBackups.zone is required with gcpFilestoreBackups.enabled set to true" }}'
           securityContext:

diff --git a/helm-charts/support/values.schema.yaml b/helm-charts/support/values.schema.yaml
@@ -179,7 +179,7 @@ properties:
           const: true
     then:
       required:
-        - filestoreName
+        - filestoreNames
         - project
         - zone
         - annotations
@@ -193,10 +193,10 @@ properties:
         description: |
           The image name and tag to use for the gcp-filestore-backups pod.
           Will be set by chartpress.
-      filestoreName:
-        type: string
+      filestoreNames:
+        type: array
         description: |
-          The name of the GCP Filestore to backup
+          The name of one or more GCP Filestores to backup as a list
       project:
         type: string
         description: |

diff --git a/helm-charts/support/values.yaml b/helm-charts/support/values.yaml
@@ -504,7 +504,7 @@ prometheusStorageClass:
 # Setup a deployment that will periodically backup the Filestore contents
 gcpFilestoreBackups:
   enabled: false
-  image: "quay.io/2i2c/gcp-filestore-backups:0.0.1-0.dev.git.9882.h6f05b0fa"
+  image: "quay.io/2i2c/gcp-filestore-backups:0.0.1-0.dev.git.9908.hcc20334f"
 
 # A placeholder as global values that can be referenced from the same location
 # of any chart should be possible to provide, but aren't necessarily provided or