Add unsupported config override for reload interval #619

Miciah · 2021-05-24T13:39:46Z

pkg/operator/controller/ingress/deployment.go (RouterReloadIntervalEnvName): New const.
(desiredRouterDeployment): Add unsupported config override for RELOAD_INTERVAL.
pkg/operator/controller/ingress/deployment_test.go (TestDesiredRouterDeployment): Verify the desiredRouterDeployment sets RELOAD_INTERVAL as expected.

sgreene570 · 2021-05-24T21:42:28Z

pkg/operator/controller/ingress/deployment.go

@@ -421,6 +424,14 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
 		Name:  RouterLoadBalancingAlgorithmEnvName,
 		Value: loadBalancingAlgorithm,
 	})
+	reloadInterval := 5
+	if unsupportedConfigOverrides.ReloadInterval != 0 {


Suggested change

if unsupportedConfigOverrides.ReloadInterval != 0 {

if unsupportedConfigOverrides.ReloadInterval > 0 {

Might as well avoid negative numbers, right?

Fixed. Thanks!

sgreene570 · 2021-05-26T13:43:56Z

Do we have a bug we can use for this fix (or should we open a new one)?

Miciah · 2021-06-23T18:02:40Z

The second to last push added an E2E test and changed the logic to ignore an unsupported config override with ~~zero~~ a negative value. The last push rebased to resolve a conflict in operator_test.go.

frobware · 2021-06-23T18:15:15Z

test/e2e/operator_test.go

+		t.Fatalf("failed to update ingresscontroller: %v", err)
+	}
+	if err := waitForDeploymentEnvVar(t, kclient, deployment, 1*time.Minute, "RELOAD_INTERVAL", "60"); err != nil {
+		t.Fatalf("expected initial deployment to set RELOAD_INTERVAL=60: %v", err)


Is it the "initial" deployment or the now "updated" deployment?

Right, it's "updated". Fixed. Thanks!

frobware · 2021-06-23T18:15:18Z

test/e2e/operator_test.go

+	defer assertIngressControllerDeleted(t, kclient, ic)
+
+	if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, icName, availableConditionsForPrivateIngressController...); err != nil {
+		t.Errorf("failed to observe expected conditions: %w", err)


Shouldn't this be Fatal? Can the test continue?

Changed to Fatalf. Thanks!

Miciah · 2021-06-26T03:38:03Z

Rebased to resolve a conflict in operator_test.go.

frobware · 2021-06-28T07:20:33Z

/lgtm

frobware · 2021-06-28T07:20:38Z

/retest

sgreene570 · 2021-06-28T13:56:00Z

/lgtm
/retest

openshift-bot · 2021-06-28T15:22:15Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-06-28T19:35:44Z

/retest

Please review the full test history for this PR and help us cut down flakes.

sgreene570 · 2021-06-28T20:52:10Z

test/e2e/operator_test.go

+	defer assertIngressControllerDeleted(t, kclient, ic)
+
+	if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, icName, availableConditionsForPrivateIngressController...); err != nil {
+		t.Fatalf("failed to observe expected conditions: %w", err)


Suggested change

t.Fatalf("failed to observe expected conditions: %w", err)

t.Fatalf("failed to observe expected conditions: %v", err)

Could this be whats causing the e2e-aws-operator job to keep failing?

Fixed. Thanks!

sgreene570 · 2021-06-28T20:53:30Z

This PR is perma-failing e2e-aws-operator due to a t.Fatalf called with %w.
/hold

frobware · 2021-07-19T14:54:59Z

/lgtm

* pkg/operator/controller/ingress/deployment.go (RouterReloadIntervalEnvName): New const. (desiredRouterDeployment): Add unsupported config override for RELOAD_INTERVAL. * pkg/operator/controller/ingress/deployment_test.go (TestDesiredRouterDeployment): Verify that desiredRouterDeployment sets RELOAD_INTERVAL as expected. * test/e2e/operator_test.go (TestReloadIntervalUnsupportedConfigOverride): Verify that the operator sets RELOAD_INTERVAL to the value specified in the unsupported configuration override or to the default value of 5 if no override is specified.

Miciah · 2021-07-19T21:13:27Z

Rebased to resolve conflicts from #628.

Miciah · 2021-07-20T13:43:46Z

[sig-auth][Feature:SCC][Early] should not have pod creation failures during install [Suite:openshift/conformance/parallel]

/test e2e-upgrade

frobware · 2021-07-20T15:08:17Z

/lgtm

openshift-ci · 2021-07-20T15:08:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: frobware, Miciah, sgreene570

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Miciah,frobware,sgreene570]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Miciah · 2021-07-20T15:13:24Z

/hold cancel

openshift-ci bot requested review from danehans and sgreene570 May 24, 2021 13:40

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 24, 2021

sgreene570 mentioned this pull request May 24, 2021

[release-4.7] operator/ingress: Add unsupportedConfigOverrides openshift/api#928

Closed

sgreene570 reviewed May 24, 2021

View reviewed changes

Miciah force-pushed the add-unsupported-config-override-for-reload-interval branch from 070740c to 1267b30 Compare June 23, 2021 17:47

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2021

Miciah force-pushed the add-unsupported-config-override-for-reload-interval branch from 1267b30 to 4221ea7 Compare June 23, 2021 18:01

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2021

frobware reviewed Jun 23, 2021

View reviewed changes

Miciah force-pushed the add-unsupported-config-override-for-reload-interval branch from 4221ea7 to 5629091 Compare June 26, 2021 03:37

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2021

Miciah force-pushed the add-unsupported-config-override-for-reload-interval branch from 5629091 to c0a7ca5 Compare June 26, 2021 03:38

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2021

openshift-ci bot assigned frobware Jun 28, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2021

openshift-ci bot assigned sgreene570 Jun 28, 2021

sgreene570 reviewed Jun 28, 2021

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2021

Miciah force-pushed the add-unsupported-config-override-for-reload-interval branch from c0a7ca5 to a95b8b1 Compare July 19, 2021 14:07

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2021

Miciah force-pushed the add-unsupported-config-override-for-reload-interval branch from a95b8b1 to 7cc62d3 Compare July 19, 2021 18:30

openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels Jul 19, 2021

Miciah force-pushed the add-unsupported-config-override-for-reload-interval branch from 7cc62d3 to a849a88 Compare July 19, 2021 21:13

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 19, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2021

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 20, 2021

openshift-merge-robot merged commit 21a6703 into openshift:master Jul 20, 2021

Miciah mentioned this pull request Jul 22, 2021

Implement configuration for handling empty requests #452

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unsupported config override for reload interval #619

Add unsupported config override for reload interval #619

Miciah commented May 24, 2021

sgreene570 May 24, 2021

Miciah Jun 23, 2021

sgreene570 commented May 26, 2021

Miciah commented Jun 23, 2021 •

edited

frobware Jun 23, 2021

Miciah Jun 26, 2021

frobware Jun 23, 2021 •

edited

Miciah Jun 26, 2021

Miciah commented Jun 26, 2021

frobware commented Jun 28, 2021

frobware commented Jun 28, 2021

sgreene570 commented Jun 28, 2021

openshift-bot commented Jun 28, 2021

openshift-bot commented Jun 28, 2021

sgreene570 Jun 28, 2021

Miciah Jul 19, 2021

sgreene570 commented Jun 28, 2021

frobware commented Jul 19, 2021

Miciah commented Jul 19, 2021

Miciah commented Jul 20, 2021

frobware commented Jul 20, 2021

openshift-ci bot commented Jul 20, 2021

Miciah commented Jul 20, 2021

	if unsupportedConfigOverrides.ReloadInterval != 0 {
	if unsupportedConfigOverrides.ReloadInterval > 0 {

	t.Fatalf("failed to observe expected conditions: %w", err)
	t.Fatalf("failed to observe expected conditions: %v", err)

Add unsupported config override for reload interval #619

Add unsupported config override for reload interval #619

Conversation

Miciah commented May 24, 2021

sgreene570 May 24, 2021

Choose a reason for hiding this comment

Miciah Jun 23, 2021

Choose a reason for hiding this comment

sgreene570 commented May 26, 2021

Miciah commented Jun 23, 2021 • edited

frobware Jun 23, 2021

Choose a reason for hiding this comment

Miciah Jun 26, 2021

Choose a reason for hiding this comment

frobware Jun 23, 2021 • edited

Choose a reason for hiding this comment

Miciah Jun 26, 2021

Choose a reason for hiding this comment

Miciah commented Jun 26, 2021

frobware commented Jun 28, 2021

frobware commented Jun 28, 2021

sgreene570 commented Jun 28, 2021

openshift-bot commented Jun 28, 2021

openshift-bot commented Jun 28, 2021

sgreene570 Jun 28, 2021

Choose a reason for hiding this comment

Miciah Jul 19, 2021

Choose a reason for hiding this comment

sgreene570 commented Jun 28, 2021

frobware commented Jul 19, 2021

Miciah commented Jul 19, 2021

Miciah commented Jul 20, 2021

frobware commented Jul 20, 2021

openshift-ci bot commented Jul 20, 2021

Miciah commented Jul 20, 2021

Miciah commented Jun 23, 2021 •

edited

frobware Jun 23, 2021 •

edited