From cc20334f6df05c4e841716aa13bd327d0c62bad1 Mon Sep 17 00:00:00 2001 From: Sarah Gibson Date: Fri, 19 Jul 2024 14:29:40 +0100 Subject: [PATCH 1/8] Allow gcp-filestore-backups.py to accept multiple filestore names to backup to --- .../gcp-filestore-backups.py | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/helm-charts/images/gcp-filestore-backups/gcp-filestore-backups.py b/helm-charts/images/gcp-filestore-backups/gcp-filestore-backups.py index 59be5238ea..eb9647a05e 100644 --- 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, From ed27a6cdfc06b8144d8d20b1b718c90519eae0be Mon Sep 17 00:00:00 2001 From: Sarah Gibson Date: Fri, 19 Jul 2024 14:32:16 +0100 Subject: [PATCH 2/8] Rebuild and bump gcp-filestore-backups image tag --- helm-charts/support/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm-charts/support/values.yaml b/helm-charts/support/values.yaml index c7d297236d..fa47b3c1b7 100644 --- 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 From 2e309f53467c0847335228df263b9a5b4fbb02ab Mon Sep 17 00:00:00 2001 From: Sarah Gibson Date: Fri, 19 Jul 2024 16:35:38 +0100 Subject: [PATCH 3/8] Update support values schema to accept array of filestoreNames --- helm-charts/support/values.schema.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helm-charts/support/values.schema.yaml b/helm-charts/support/values.schema.yaml index dd48c12494..3b29dfe2cc 100644 --- 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: | From 13f8546ae22b24ead67501942c74c294c020984d Mon Sep 17 00:00:00 2001 From: Sarah Gibson Date: Fri, 19 Jul 2024 16:36:26 +0100 Subject: [PATCH 4/8] Update gcp-filestore-backups deployment to parse multiple filestore names to command in the container --- .../support/templates/gcp-filestore-backups/deployment.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/helm-charts/support/templates/gcp-filestore-backups/deployment.yaml b/helm-charts/support/templates/gcp-filestore-backups/deployment.yaml index 7e55115bf7..a0b483e8ab 100644 --- a/helm-charts/support/templates/gcp-filestore-backups/deployment.yaml +++ b/helm-charts/support/templates/gcp-filestore-backups/deployment.yaml @@ -24,7 +24,9 @@ spec: - 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: From 8f89bab43677c7c1a6b3878a59df47debd8bd3a6 Mon Sep 17 00:00:00 2001 From: Sarah Gibson Date: Fri, 19 Jul 2024 16:36:58 +0100 Subject: [PATCH 5/8] Update format of 2i2c's support values file to match new schema --- config/clusters/2i2c/support.values.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/clusters/2i2c/support.values.yaml b/config/clusters/2i2c/support.values.yaml index 401847c5bc..ca8a0810db 100644 --- 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: From 717f69e810eddf3321c62e259e22aa7353090a69 Mon Sep 17 00:00:00 2001 From: Sarah Gibson Date: Fri, 19 Jul 2024 16:41:42 +0100 Subject: [PATCH 6/8] Add release name into deployment and pod names to match convention --- .../support/templates/gcp-filestore-backups/deployment.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helm-charts/support/templates/gcp-filestore-backups/deployment.yaml b/helm-charts/support/templates/gcp-filestore-backups/deployment.yaml index a0b483e8ab..a958ec06d9 100644 --- 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,7 +18,7 @@ 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 From 56cdad7d1f87ff76fa8c02c235eed8834e99b87e Mon Sep 17 00:00:00 2001 From: Sarah Gibson Date: Fri, 19 Jul 2024 16:44:38 +0100 Subject: [PATCH 7/8] Update docs --- docs/howto/decrease-size-gcp-filestore.md | 6 ++++++ docs/howto/filesystem-backups/enable-backups.md | 8 +++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/howto/decrease-size-gcp-filestore.md b/docs/howto/decrease-size-gcp-filestore.md index 50aaf515ea..75e79e5cb9 100644 --- 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 index 9148b5c6c3..1d898a0e5b 100644 --- a/docs/howto/filesystem-backups/enable-backups.md +++ b/docs/howto/filesystem-backups/enable-backups.md @@ -39,15 +39,17 @@ export CLUSTER_NAME= ```yaml gcpFilestoreBackups: enabled: true - filestoreName: + filestoreNames: + - + - ... project: zone: annotations: iam.gke.io/gcp-service-account: ``` 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`) From 51bb6993d5076767f8af995fa00727a51192f2a7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 19 Jul 2024 15:46:14 +0000 Subject: [PATCH 8/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../images/gcp-filestore-backups/gcp-filestore-backups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm-charts/images/gcp-filestore-backups/gcp-filestore-backups.py b/helm-charts/images/gcp-filestore-backups/gcp-filestore-backups.py index eb9647a05e..34d8aeae1a 100644 --- a/helm-charts/images/gcp-filestore-backups/gcp-filestore-backups.py +++ b/helm-charts/images/gcp-filestore-backups/gcp-filestore-backups.py @@ -239,7 +239,7 @@ def main(args): # 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.