From 7be12a92206beb1ee11e5e9dcf4a09dd778605f8 Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Mon, 21 Feb 2022 22:04:21 +0800 Subject: [PATCH 1/2] [fix] Add regional PV support for GKE fix #4663. For GKE pv, when create backup, return all zones retrived from node affinity. Signed-off-by: Xun Jiang --- changelogs/unreleased/4680-jxun | 1 + pkg/backup/item_backupper.go | 16 +++++++++++++++- pkg/backup/item_backupper_test.go | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/4680-jxun diff --git a/changelogs/unreleased/4680-jxun b/changelogs/unreleased/4680-jxun new file mode 100644 index 000000000..9d6b56ba2 --- /dev/null +++ b/changelogs/unreleased/4680-jxun @@ -0,0 +1 @@ +Support regional pv for GKE \ No newline at end of file diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index a2830484a..648447625 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -21,6 +21,7 @@ import ( "encoding/json" "fmt" "path/filepath" + "strings" "time" "github.com/pkg/errors" @@ -384,6 +385,7 @@ const ( awsEbsCsiZoneKey = "topology.ebs.csi.aws.com/zone" azureCsiZoneKey = "topology.disk.csi.azure.com/zone" gkeCsiZoneKey = "topology.gke.io/zone" + zoneSeparator = "__" ) // takePVSnapshot triggers a snapshot for the volume/disk underlying a PersistentVolume if the provided @@ -539,15 +541,27 @@ func zoneFromPVNodeAffinity(res *corev1api.PersistentVolume, topologyKeys ...str return "", "" } keySet := sets.NewString(topologyKeys...) + providerGke := false + zones := make([]string, 0) for _, term := range nodeAffinity.Required.NodeSelectorTerms { if term.MatchExpressions == nil { continue } for _, exp := range term.MatchExpressions { if keySet.Has(exp.Key) && exp.Operator == "In" && len(exp.Values) > 0 { - return exp.Key, exp.Values[0] + if exp.Key == gkeCsiZoneKey { + providerGke = true + zones = append(zones, exp.Values[0]) + } else { + return exp.Key, exp.Values[0] + } } } } + + if providerGke { + return gkeCsiZoneKey, strings.Join(zones, zoneSeparator) + } + return "", "" } diff --git a/pkg/backup/item_backupper_test.go b/pkg/backup/item_backupper_test.go index 819ca54d8..03ed7c017 100644 --- a/pkg/backup/item_backupper_test.go +++ b/pkg/backup/item_backupper_test.go @@ -131,6 +131,21 @@ func Test_zoneFromPVNodeAffinity(t *testing.T) { wantKey: "topology.disk.csi.azure.com/zone", wantValue: "us-central", }, + { + name: "Volume with multiple valid keys, and provider is gke, returns the all match", // it should never happen + pv: builder.ForPersistentVolume("multi-matching-pv").NodeAffinityRequired( + builder.ForNodeSelector( + *builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.gke.io/zone", + "In", "us-central1-c").Result(), + *builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.gke.io/zone", + "In", "us-east-2c", "us-east-2b").Result(), + *builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.gke.io/zone", + "In", "europe-north1-a").Result(), + ).Result(), + ).Result(), + wantKey: "topology.gke.io/zone", + wantValue: "us-central1-c__us-east-2c__europe-north1-a", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 4c8d1c2693ba12a1b86feb0c1dea316643c6b3c0 Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Tue, 22 Feb 2022 16:06:39 +0800 Subject: [PATCH 2/2] Modify according to comments 1. rename zoneSeparator to gkeZoneSeparator 2. add example of regional PV's node affinity. modify test case description. Signed-off-by: Xun Jiang --- pkg/backup/item_backupper.go | 4 ++-- pkg/backup/item_backupper_test.go | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index 648447625..32a9f5504 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -385,7 +385,7 @@ const ( awsEbsCsiZoneKey = "topology.ebs.csi.aws.com/zone" azureCsiZoneKey = "topology.disk.csi.azure.com/zone" gkeCsiZoneKey = "topology.gke.io/zone" - zoneSeparator = "__" + gkeZoneSeparator = "__" ) // takePVSnapshot triggers a snapshot for the volume/disk underlying a PersistentVolume if the provided @@ -560,7 +560,7 @@ func zoneFromPVNodeAffinity(res *corev1api.PersistentVolume, topologyKeys ...str } if providerGke { - return gkeCsiZoneKey, strings.Join(zones, zoneSeparator) + return gkeCsiZoneKey, strings.Join(zones, gkeZoneSeparator) } return "", "" diff --git a/pkg/backup/item_backupper_test.go b/pkg/backup/item_backupper_test.go index 03ed7c017..2152a5301 100644 --- a/pkg/backup/item_backupper_test.go +++ b/pkg/backup/item_backupper_test.go @@ -132,7 +132,22 @@ func Test_zoneFromPVNodeAffinity(t *testing.T) { wantValue: "us-central", }, { - name: "Volume with multiple valid keys, and provider is gke, returns the all match", // it should never happen + /* an valid example of node affinity in a GKE's regional PV + nodeAffinity: + required: + nodeSelectorTerms: + - matchExpressions: + - key: topology.gke.io/zone + operator: In + values: + - us-central1-a + - matchExpressions: + - key: topology.gke.io/zone + operator: In + values: + - us-central1-c + */ + name: "Volume with multiple valid keys, and provider is gke, returns all valid entries's first zone value", pv: builder.ForPersistentVolume("multi-matching-pv").NodeAffinityRequired( builder.ForNodeSelector( *builder.NewNodeSelectorTermBuilder().WithMatchExpression("topology.gke.io/zone",