Skip to content
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

RHOAIENG-15772: tests(odh-nbc) significantly reduce the amount of sleeping in tests #465

Merged
merged 4 commits into from
Mar 14, 2025
Merged

RHOAIENG-15772: tests(odh-nbc) significantly reduce the amount of sleeping in tests #465

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81273bd
fix typo
jiridanek Nov 17, 2024
a0618aa
remove some sleeps
jiridanek Nov 17, 2024
11c91ab
sortof remove sleep, but need it because of RHOAIENG-15907
jiridanek Nov 17, 2024
931740e
shorten the check interval
jiridanek Nov 17, 2024
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
27 changes: 6 additions & 21 deletions components/odh-notebook-controller/controllers/notebook_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Describe("The Openshift Notebook controller", func() {
// Define utility constants for testing timeouts/durations and intervals.
const (
duration = 10 * time.Second
interval = 2 * time.Second
interval = 200 * time.Millisecond
)

When("Creating a Notebook", func() {
Expand Down Expand Up @@ -92,7 +92,6 @@ var _ = Describe("The Openshift Notebook controller", func() {

By("By creating a new Notebook")
Expect(cli.Create(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has created the Route")
Eventually(func() error {
Expand All @@ -106,7 +105,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
By("By simulating a manual Route modification")
patch := client.RawPatch(types.MergePatchType, []byte(`{"spec":{"to":{"name":"foo"}}}`))
Expect(cli.Patch(ctx, route, patch)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has restored the Route spec")
Eventually(func() (string, error) {
Expand All @@ -123,7 +121,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
It("Should recreate the Route when deleted", func() {
By("By deleting the notebook route")
Expect(cli.Delete(ctx, route)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has recreated the Route")
Eventually(func() error {
Expand Down Expand Up @@ -152,7 +149,6 @@ var _ = Describe("The Openshift Notebook controller", func() {

By("By deleting the recently created Notebook")
Expect(cli.Delete(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that the Notebook is deleted")
Eventually(func() error {
Expand Down Expand Up @@ -275,7 +271,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
By("By creating a new Notebook")
notebook := createNotebook(Name, Namespace)
Expect(cli.Create(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that trusted-ca bundle is mounted")
// Assert that the volume mount and volume are added correctly
Expand Down Expand Up @@ -310,6 +305,8 @@ var _ = Describe("The Openshift Notebook controller", func() {
// Check the content in workbench-trusted-ca-bundle matches what we expect:
// - have 2 certificates there in ca-bundle.crt
// - both certificates are valid
// TODO(RHOAIENG-15907): adding sleep to reduce flakiness
time.Sleep(2 * time.Second)
configMapName := "workbench-trusted-ca-bundle"
checkCertConfigMap(ctx, notebook.Namespace, configMapName, "ca-bundle.crt", 2)
})
Expand All @@ -330,7 +327,6 @@ var _ = Describe("The Openshift Notebook controller", func() {

By("By creating a new Notebook")
Expect(cli.Create(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By updating the Notebook's image")
key := types.NamespacedName{Name: Name, Namespace: Namespace}
Expand All @@ -339,7 +335,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
updatedImage := "registry.redhat.io/ubi8/ubi:updated"
notebook.Spec.Template.Spec.Containers[0].Image = updatedImage
Expect(cli.Update(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that the Notebook's image is updated")
Eventually(func() string {
Expand Down Expand Up @@ -382,7 +377,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
updatedImage := "registry.redhat.io/ubi8/ubi:updated"
notebook.Spec.Template.Spec.Containers[0].Image = updatedImage
Expect(cli.Update(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that trusted-ca bundle is mounted")
// Assert that the volume mount and volume are added correctly
Expand Down Expand Up @@ -415,6 +409,8 @@ var _ = Describe("The Openshift Notebook controller", func() {
// Check the content in workbench-trusted-ca-bundle matches what we expect:
// - have 2 certificates there in ca-bundle.crt
// - both certificates are valid
// TODO(RHOAIENG-15907): adding sleep to reduce flakiness
time.Sleep(2 * time.Second)
configMapName := "workbench-trusted-ca-bundle"
checkCertConfigMap(ctx, notebook.Namespace, configMapName, "ca-bundle.crt", 2)
})
Expand Down Expand Up @@ -482,7 +478,6 @@ var _ = Describe("The Openshift Notebook controller", func() {

By("By creating a new Notebook")
Expect(cli.Create(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has created Network policy to allow only controller traffic")
Eventually(func() error {
Expand All @@ -503,7 +498,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
By("By simulating a manual NetworkPolicy modification")
patch := client.RawPatch(types.MergePatchType, []byte(`{"spec":{"policyTypes":["Egress"]}}`))
Expect(cli.Patch(ctx, notebookNetworkPolicy, patch)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has restored the network policy spec")
Eventually(func() (string, error) {
Expand All @@ -520,7 +514,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
It("Should recreate the Network Policy when deleted", func() {
By("By deleting the notebook OAuth Network Policy")
Expect(cli.Delete(ctx, notebookOAuthNetworkPolicy)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has recreated the OAuth Network policy")
Eventually(func() error {
Expand Down Expand Up @@ -657,7 +650,6 @@ var _ = Describe("The Openshift Notebook controller", func() {

By("By creating a new Notebook")
Expect(cli.Create(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that the webhook has injected the sidecar container")
Expect(*notebook).To(BeMatchingK8sResource(expectedNotebook, CompareNotebooks))
Expand All @@ -679,7 +671,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
notebook.Spec.Template.Spec.Containers[1].Image = "bar"
notebook.Spec.Template.Spec.Volumes[1].VolumeSource = corev1.VolumeSource{}
Expect(cli.Update(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that the webhook has restored the Notebook spec")
Eventually(func() error {
Expand All @@ -704,7 +695,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
It("Should recreate the Service Account when deleted", func() {
By("By deleting the notebook Service Account")
Expect(cli.Delete(ctx, serviceAccount)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has recreated the Service Account")
Eventually(func() error {
Expand Down Expand Up @@ -748,7 +738,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
It("Should recreate the Service when deleted", func() {
By("By deleting the notebook Service")
Expect(cli.Delete(ctx, service)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has recreated the Service")
Eventually(func() error {
Expand All @@ -774,7 +763,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
It("Should recreate the Secret when deleted", func() {
By("By deleting the notebook Secret")
Expect(cli.Delete(ctx, secret)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has recreated the Secret")
Eventually(func() error {
Expand Down Expand Up @@ -824,7 +812,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
It("Should recreate the Route when deleted", func() {
By("By deleting the notebook Route")
Expect(cli.Delete(ctx, route)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has recreated the Route")
Eventually(func() error {
Expand All @@ -838,7 +825,6 @@ var _ = Describe("The Openshift Notebook controller", func() {
By("By simulating a manual Route modification")
patch := client.RawPatch(types.MergePatchType, []byte(`{"spec":{"to":{"name":"foo"}}}`))
Expect(cli.Patch(ctx, route, patch)).Should(Succeed())
time.Sleep(interval)

By("By checking that the controller has restored the Route spec")
Eventually(func() (string, error) {
Expand Down Expand Up @@ -880,7 +866,6 @@ var _ = Describe("The Openshift Notebook controller", func() {

By("By deleting the recently created Notebook")
Expect(cli.Delete(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that the Notebook is deleted")
Eventually(func() error {
Expand Down Expand Up @@ -1120,7 +1105,7 @@ func createOAuthConfigmap(name, namespace string, label map[string]string, confi
}

// checkCertConfigMap checks the content of a config map defined by the name and namespace
// It triest to parse the given certFileName and checks that all certificates can be parsed there and that the number of the certificates matches what we expect.
// It tries to parse the given certFileName and checks that all certificates can be parsed there and that the number of the certificates matches what we expect.
func checkCertConfigMap(ctx context.Context, namespace string, configMapName string, certFileName string, expNumberCerts int) {
configMap := &corev1.ConfigMap{}
key := types.NamespacedName{Namespace: namespace, Name: configMapName}
Expand Down