From 6ad7dbe123aa1c2fb3ab0257a4bfd61d83950f69 Mon Sep 17 00:00:00 2001 From: Connor Catlett Date: Fri, 16 Aug 2024 17:01:45 +0000 Subject: [PATCH 1/4] Fix TestNameAllocatorLikelyBadName to prevent code coverage flapping The existing test does not correctly test the "only existing" behavior (and only functions because of a typo in the test), fix that by moving it to before the fallback test (where it needs to be) and correctly setting up the existing names map Also, added some comments to make what is being tested more clear Signed-off-by: Connor Catlett --- pkg/cloud/devicemanager/allocator_test.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/cloud/devicemanager/allocator_test.go b/pkg/cloud/devicemanager/allocator_test.go index eb05e2e889..a0d68003b5 100644 --- a/pkg/cloud/devicemanager/allocator_test.go +++ b/pkg/cloud/devicemanager/allocator_test.go @@ -68,17 +68,22 @@ func TestNameAllocatorLikelyBadName(t *testing.T) { }) } + // Test likely bad name fallback when it is the only device name available + // We should receive the likely bad device name because it is the only option left + lastName, _ := allocator.GetNext(existingNames, likelyBadNames) + if lastName != skippedNameNew { + t.Errorf("test %q: expected %q, got %q (likelyBadNames fallback)", skippedNameNew, skippedNameNew, lastName) + } + existingNames[skippedNameNew] = "" + + // Test likely bad name fallback when the likely bad device name already exists + // Because the device name already exists, this should return an error onlyExisting := new(sync.Map) onlyExisting.Store(skippedNameExisting, struct{}{}) _, err := allocator.GetNext(existingNames, onlyExisting) - if err != nil { + if err == nil { t.Errorf("got nil when error expected (likelyBadNames with only existing names)") } - - lastName, _ := allocator.GetNext(existingNames, likelyBadNames) - if lastName != skippedNameNew { - t.Errorf("test %q: expected %q, got %q (likelyBadNames fallback)", skippedNameNew, skippedNameNew, lastName) - } } func TestNameAllocatorError(t *testing.T) { From 71e3ce7dd73f0bd95b44296075000f283bcb4f72 Mon Sep 17 00:00:00 2001 From: Connor Catlett Date: Fri, 16 Aug 2024 17:03:19 +0000 Subject: [PATCH 2/4] Adjust TestExpiringCache timeouts to decrease CI flakes Signed-off-by: Connor Catlett --- pkg/expiringcache/expiring_cache_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/expiringcache/expiring_cache_test.go b/pkg/expiringcache/expiring_cache_test.go index 64f8169cba..4b6716879d 100644 --- a/pkg/expiringcache/expiring_cache_test.go +++ b/pkg/expiringcache/expiring_cache_test.go @@ -24,8 +24,8 @@ import ( ) const ( - testExpiration = time.Millisecond * 50 - testSleep = time.Millisecond * 35 + testExpiration = time.Millisecond * 100 + testSleep = time.Millisecond * 80 testKey = "key" ) From a5edb4effc420a27f5f88e62300929a27f790346 Mon Sep 17 00:00:00 2001 From: Connor Catlett Date: Fri, 16 Aug 2024 18:59:52 +0000 Subject: [PATCH 3/4] Standardize deployment methods by removing Kustomize-specific changes Signed-off-by: Connor Catlett --- charts/aws-ebs-csi-driver/templates/controller.yaml | 4 ---- .../templates/serviceaccount-csi-controller.yaml | 5 ----- deploy/kubernetes/base/controller.yaml | 2 +- deploy/kubernetes/base/serviceaccount-csi-controller.yaml | 3 --- 4 files changed, 1 insertion(+), 13 deletions(-) diff --git a/charts/aws-ebs-csi-driver/templates/controller.yaml b/charts/aws-ebs-csi-driver/templates/controller.yaml index f5d42f89ca..0b61cd280b 100644 --- a/charts/aws-ebs-csi-driver/templates/controller.yaml +++ b/charts/aws-ebs-csi-driver/templates/controller.yaml @@ -74,11 +74,7 @@ spec: image: {{ printf "%s%s:%s" (default "" .Values.image.containerRegistry) .Values.image.repository (default (printf "v%s" .Chart.AppVersion) (.Values.image.tag | toString)) }} imagePullPolicy: {{ .Values.image.pullPolicy }} args: - {{- if ne .Release.Name "kustomize" }} - controller - {{- else }} - # - {all,controller,node} # specify the driver mode - {{- end }} - --endpoint=$(CSI_ENDPOINT) {{- if .Values.controller.extraVolumeTags }} {{- include "aws-ebs-csi-driver.extra-volume-tags" . | nindent 12 }} diff --git a/charts/aws-ebs-csi-driver/templates/serviceaccount-csi-controller.yaml b/charts/aws-ebs-csi-driver/templates/serviceaccount-csi-controller.yaml index bfb8c4bc1a..d252f26f0d 100644 --- a/charts/aws-ebs-csi-driver/templates/serviceaccount-csi-controller.yaml +++ b/charts/aws-ebs-csi-driver/templates/serviceaccount-csi-controller.yaml @@ -10,11 +10,6 @@ metadata: annotations: {{- toYaml . | nindent 4 }} {{- end }} - {{- if eq .Release.Name "kustomize" }} - #Enable if EKS IAM roles for service accounts (IRSA) is used. See https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html for details. - #annotations: - # eks.amazonaws.com/role-arn: arn::iam:::role/ebs-csi-role - {{- end }} {{- if .Values.controller.serviceAccount.automountServiceAccountToken }} automountServiceAccountToken: {{ .Values.controller.serviceAccount.automountServiceAccountToken }} {{- end }} diff --git a/deploy/kubernetes/base/controller.yaml b/deploy/kubernetes/base/controller.yaml index f27d809c24..ce7902611a 100644 --- a/deploy/kubernetes/base/controller.yaml +++ b/deploy/kubernetes/base/controller.yaml @@ -65,7 +65,7 @@ spec: image: public.ecr.aws/ebs-csi-driver/aws-ebs-csi-driver:v1.34.0 imagePullPolicy: IfNotPresent args: - # - {all,controller,node} # specify the driver mode + - controller - --endpoint=$(CSI_ENDPOINT) - --batching=true - --logging-format=text diff --git a/deploy/kubernetes/base/serviceaccount-csi-controller.yaml b/deploy/kubernetes/base/serviceaccount-csi-controller.yaml index 1768ff890f..b1926a9add 100644 --- a/deploy/kubernetes/base/serviceaccount-csi-controller.yaml +++ b/deploy/kubernetes/base/serviceaccount-csi-controller.yaml @@ -6,7 +6,4 @@ metadata: name: ebs-csi-controller-sa labels: app.kubernetes.io/name: aws-ebs-csi-driver - #Enable if EKS IAM roles for service accounts (IRSA) is used. See https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html for details. - #annotations: - # eks.amazonaws.com/role-arn: arn::iam:::role/ebs-csi-role automountServiceAccountToken: true From 6386a2c701f9f3e247009a7395c7b92cc09f40c3 Mon Sep 17 00:00:00 2001 From: Connor Catlett Date: Fri, 16 Aug 2024 20:37:17 +0000 Subject: [PATCH 4/4] Set PATH for kubetest2 so it can find helpers Signed-off-by: Connor Catlett --- hack/e2e/run.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hack/e2e/run.sh b/hack/e2e/run.sh index fc67698522..5d715227b7 100755 --- a/hack/e2e/run.sh +++ b/hack/e2e/run.sh @@ -116,7 +116,8 @@ else set -x set +e - "${BIN}/kubetest2" noop \ + # kubetest2 looks for deployers/testers in $PATH + PATH="${BIN}:${PATH}" "${BIN}/kubetest2" noop \ --run-id="e2e-kubernetes" \ --test=ginkgo \ -- \