From c8d778e7a8797e05805f4df846fefe64d2c36998 Mon Sep 17 00:00:00 2001 From: hc-github-team-nomad-core <82989552+hc-github-team-nomad-core@users.noreply.github.com> Date: Fri, 22 Nov 2024 18:56:33 +0100 Subject: [PATCH] Backport of Fix CSITopology matching logic for csi storage driver into release/1.9.x (#24539) Co-authored-by: Yucong Sun --- .changelog/24522.txt | 3 + e2e/csi/input/plugin-aws-ebs-controller.nomad | 2 +- e2e/csi/input/plugin-aws-ebs-nodes.nomad | 2 +- nomad/structs/node.go | 16 +++- nomad/structs/node_test.go | 91 +++++++++++++++++++ 5 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 .changelog/24522.txt diff --git a/.changelog/24522.txt b/.changelog/24522.txt new file mode 100644 index 00000000000..f783b584b6c --- /dev/null +++ b/.changelog/24522.txt @@ -0,0 +1,3 @@ +```release-note:bug +csi: Fixed a bug where drivers that emit multiple topology segments would cause placements to fail +``` diff --git a/e2e/csi/input/plugin-aws-ebs-controller.nomad b/e2e/csi/input/plugin-aws-ebs-controller.nomad index 74fee480d9f..d4dfcc1323c 100644 --- a/e2e/csi/input/plugin-aws-ebs-controller.nomad +++ b/e2e/csi/input/plugin-aws-ebs-controller.nomad @@ -25,7 +25,7 @@ job "plugin-aws-ebs-controller" { driver = "docker" config { - image = "public.ecr.aws/ebs-csi-driver/aws-ebs-csi-driver:v1.5.1" + image = "public.ecr.aws/ebs-csi-driver/aws-ebs-csi-driver:v1.33.0" args = [ "controller", diff --git a/e2e/csi/input/plugin-aws-ebs-nodes.nomad b/e2e/csi/input/plugin-aws-ebs-nodes.nomad index d5f4414289c..61a68949c44 100644 --- a/e2e/csi/input/plugin-aws-ebs-nodes.nomad +++ b/e2e/csi/input/plugin-aws-ebs-nodes.nomad @@ -22,7 +22,7 @@ job "plugin-aws-ebs-nodes" { driver = "docker" config { - image = "public.ecr.aws/ebs-csi-driver/aws-ebs-csi-driver:v1.5.1" + image = "public.ecr.aws/ebs-csi-driver/aws-ebs-csi-driver:v1.33.0" args = [ "node", diff --git a/nomad/structs/node.go b/nomad/structs/node.go index c7a334f249b..baa809c122e 100644 --- a/nomad/structs/node.go +++ b/nomad/structs/node.go @@ -68,13 +68,27 @@ func (t *CSITopology) Equal(o *CSITopology) bool { return maps.Equal(t.Segments, o.Segments) } +func (t *CSITopology) Contains(o *CSITopology) bool { + if t == nil || o == nil { + return t == o + } + + for k, ov := range o.Segments { + if tv, ok := t.Segments[k]; !ok || tv != ov { + return false + } + } + + return true +} + func (t *CSITopology) MatchFound(o []*CSITopology) bool { if t == nil || o == nil || len(o) == 0 { return false } for _, other := range o { - if t.Equal(other) { + if t.Contains(other) { return true } } diff --git a/nomad/structs/node_test.go b/nomad/structs/node_test.go index 1404b3ff2b4..d9185a672e0 100644 --- a/nomad/structs/node_test.go +++ b/nomad/structs/node_test.go @@ -163,3 +163,94 @@ func TestNodeMeta_Validate(t *testing.T) { }) } } + +func TestCSITopology_Contains(t *testing.T) { + ci.Parallel(t) + + cases := []struct { + name string + this *CSITopology + other *CSITopology + expected bool + }{ + { + name: "AWS EBS pre 1.27 behavior", + this: &CSITopology{ + Segments: map[string]string{ + "topology.ebs.csi.aws.com/zone": "us-east-1a", + }, + }, + other: &CSITopology{ + Segments: map[string]string{ + "topology.ebs.csi.aws.com/zone": "us-east-1a", + }, + }, + expected: true, + }, + { + name: "AWS EBS post 1.27 behavior", + this: &CSITopology{ + Segments: map[string]string{ + "topology.kubernetes.io/zone": "us-east-1a", + "topology.ebs.csi.aws.com/zone": "us-east-1a", + "kubernetes.io/os": "linux", + }, + }, + other: &CSITopology{ + Segments: map[string]string{ + "topology.kubernetes.io/zone": "us-east-1a", + }, + }, + expected: true, + }, + { + name: "other contains invalid segment value for matched key", + this: &CSITopology{ + Segments: map[string]string{ + "topology.kubernetes.io/zone": "us-east-1a", + "topology.ebs.csi.aws.com/zone": "us-east-1a", + "kubernetes.io/os": "linux", + }, + }, + other: &CSITopology{ + Segments: map[string]string{ + "topology.kubernetes.io/zone": "us-east-1a", + "kubernetes.io/os": "windows", + }, + }, + expected: false, + }, + { + name: "other contains invalid segment key", + this: &CSITopology{ + Segments: map[string]string{ + "topology.kubernetes.io/zone": "us-east-1a", + }, + }, + other: &CSITopology{ + Segments: map[string]string{ + "topology.kubernetes.io/zone": "us-east-1a", + "kubernetes.io/os": "linux", + }, + }, + expected: false, + }, + { + name: "other is nil", + this: &CSITopology{ + Segments: map[string]string{ + "topology.kubernetes.io/zone": "us-east-1a", + }, + }, + other: nil, + expected: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + must.Eq(t, tc.expected, tc.this.Contains(tc.other)) + }) + } + +}