Skip to content

CI | Adding eventually for some KMS test to avoid timing failures #1710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

CI | Adding eventually for some KMS test to avoid timing failures #1710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
868b2f5
CI | Increasing the timeout of `NooBaaCondition`
liranmauda Sep 28, 2025
deda87c
s
liranmauda Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/util/kms/test/kmip/kms_kmip_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ func TestKMIPKMS(t *testing.T) {
}

RegisterFailHandler(Fail)
RunSpecs(t, "KMS KMIP Suite")
suiteConfig, reporterConfig := GinkgoConfiguration()
suiteConfig.FailFast = true
RunSpecs(t, "KMS KMIP Suite", suiteConfig, reporterConfig)
}

var _ = BeforeSuite(func(ctx context.Context) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/util/kms/test/rotate/kms_rotate_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ func TestRotateKMS(t *testing.T) {
}

RegisterFailHandler(Fail)
RunSpecs(t, "KMS (K8S Key Rotate) Suite")
suiteConfig, reporterConfig := GinkgoConfiguration()
suiteConfig.FailFast = true
RunSpecs(t, "KMS (K8S Key Rotate) Suite", suiteConfig, reporterConfig)
}

var _ = BeforeSuite(func(ctx context.Context) {
Expand Down
5 changes: 5 additions & 0 deletions pkg/util/kms/test/rotate/kms_rotate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ var _ = Describe("KMS - K8S Key Rotate", func() {
Context("Verify Rotate", func() {
noobaa := getSchedMiniNooBaa()

secret := &corev1.Secret{}
secret.Name = noobaa.Name + "-root-master-key-backend"
secret.Namespace = noobaa.Namespace
_ = util.KubeDelete(secret)
Comment on lines +84 to +87
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fail fast on backend-secret cleanup

util.KubeDelete returns false when the API delete fails for reasons other than NotFound. By discarding that result we can sail past a real failure and still enter the rotation specs with a stale backend key, reintroducing the very flake we’re trying to eliminate. Please move this cleanup into a BeforeEach/JustBeforeEach block and assert on the outcome so the suite stops immediately when the delete cannot be confirmed. For example:

-       secret := &corev1.Secret{}
-       secret.Name = noobaa.Name + "-root-master-key-backend"
-       secret.Namespace = noobaa.Namespace
-       _ = util.KubeDelete(secret)
+       BeforeEach(func() {
+               backendSecret := &corev1.Secret{}
+               backendSecret.Name = noobaa.Name + "-root-master-key-backend"
+               backendSecret.Namespace = noobaa.Namespace
+               Expect(util.KubeDelete(backendSecret)).To(
+                       BeTrue(),
+                       "cleanup stale backend key secret before running rotation checks",
+               )
+       })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
secret := &corev1.Secret{}
secret.Name = noobaa.Name + "-root-master-key-backend"
secret.Namespace = noobaa.Namespace
_ = util.KubeDelete(secret)
BeforeEach(func() {
backendSecret := &corev1.Secret{}
backendSecret.Name = noobaa.Name + "-root-master-key-backend"
backendSecret.Namespace = noobaa.Namespace
Expect(util.KubeDelete(backendSecret)).To(
BeTrue(),
"cleanup stale backend key secret before running rotation checks",
)
})
🤖 Prompt for AI Agents 
In pkg/util/kms/test/rotate/kms_rotate_test.go around lines 84-87, the call to
util.KubeDelete on the backend secret ignores its boolean result so a delete
failure (other than NotFound) is swallowed; move this cleanup into a BeforeEach
or JustBeforeEach block and assert the delete succeeded (e.g.,
require.True/Expect on util.KubeDelete returning true or fail the test with a
clear message) so the suite fails fast if the secret cannot be removed,
preventing tests from running against a stale backend key.


Specify("Create key rotate schedule system", func() {
Expect(util.KubeCreateFailExisting(noobaa)).To(BeTrue())
})
Expand Down
Loading