Skip to content

OCPBUGS-2429: Fix TestReloadInterval min value test cases and race conditions #842

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

Merged
openshift-merge-robot merged 1 commit into from
Oct 15, 2022
Merged

OCPBUGS-2429: Fix TestReloadInterval min value test cases and race conditions #842

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 27 additions & 5 deletions test/e2e/reloadinterval_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ func TestReloadInterval(t *testing.T) {

// set ReloadInterval to a value outside allowed bounds
// expect no error
{"set reloadInterval 200ns", metav1.Duration{Duration: 200 * time.Nanosecond}, "5s", true},
{"set reloadInterval 5us", metav1.Duration{Duration: 5 * time.Microsecond}, "5s", true},
{"set reloadInterval 500ms", metav1.Duration{Duration: 500 * time.Millisecond}, "5s", true},
{"set reloadInterval 200ns", metav1.Duration{Duration: 200 * time.Nanosecond}, "1s", true},
{"set reloadInterval 5us", metav1.Duration{Duration: 5 * time.Microsecond}, "1s", true},
Copy link
Contributor

@miheer miheer Oct 15, 2022

Choose a reason for hiding this comment

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

does setting to 1s really help ? Won't it cause load on the router ?

Copy link
Contributor Author

@gcs278 gcs278 Oct 17, 2022

Choose a reason for hiding this comment

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

Not exactly - it's the value the test is looking for in the router deployment ENV variable. So no load affects, just a comparison value that was wrong.

{"set reloadInterval 500ms", metav1.Duration{Duration: 500 * time.Millisecond}, "1s", true},
{"set reloadInterval 130s", metav1.Duration{Duration: 130 * time.Second}, "2m", true},
{"set reloadInterval 2h", metav1.Duration{Duration: 2 * time.Hour}, "2m", true},
} {
Expand All @@ -99,9 +99,31 @@ func TestReloadInterval(t *testing.T) {
}
}

if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, icName, availableConditionsForPrivateIngressController...); err != nil {
t.Fatalf("failed to observe expected conditions: %v", err)
// Delete the deployment to trigger a new router deployment to ensure we are looking at reconciled values
oldUID := deployment.UID
if err := kclient.Delete(context.TODO(), deployment); err != nil {
t.Fatalf("failed to delete router deployment: %v", err)
}

// Ensure the deployment has been recreated.
err := wait.PollImmediate(1*time.Second, 2*time.Minute, func() (bool, error) {
deployment := &appsv1.Deployment{}
if err := kclient.Get(context.TODO(), controller.RouterDeploymentName(ic), deployment); err != nil {
t.Logf("Get %q failed: %v, retrying ...", controller.RouterDeploymentName(ic), err)
return false, nil
}
// Ensure the UID of the router deployment changed.
if deployment.UID == oldUID {
t.Logf("Waiting for deployment %q to be deleted", controller.RouterDeploymentName(ic))
return false, nil
}
return true, nil
})

if err != nil {
t.Fatalf("failed to delete and recreate the %q deployment", controller.RouterDeploymentName(ic))
}

if err := waitForDeploymentEnvVar(t, kclient, deployment, 1*time.Minute, ingresscontroller.RouterReloadIntervalEnvName, testCase.expectedEnvVar); err != nil {
t.Fatalf("router deployment not updated with %s=%v: %v", ingresscontroller.RouterReloadIntervalEnvName, testCase.expectedEnvVar, err)
}
Expand Down