Lower threshold on backoff for sanity tests #1087
Conversation
k8s-ci-robot
added
release-note-none
|
Hi @judemars. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
size/S
cmd/gce-pd-csi-driver/main.go
Outdated
| @@ -57,6 +57,8 @@ var ( | |||
|
|
|||
| maxprocs = flag.Int("maxprocs", 1, "GOMAXPROCS override") | |||
|
|
|||
| errorBackoffInitialDuration = 200 * time.Millisecond | |||
There was a problem hiding this comment.
Please make this a flag for errorBackoffMaxDuration and errorBackoffInitialDuration, with default values
There was a problem hiding this comment.
can you also make the errorBackoffMaxDuration configurable through a separate flag
There was a problem hiding this comment.
Woops sorry I missed that, added errorBackoffMaxDurationMs flag. Note I did it in ms to match the initial duration flag, but lmk if you think I should change to minutes/seconds.
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
cmd/gce-pd-csi-driver/main.go
Outdated
| @@ -42,7 +42,8 @@ var ( | |||
| httpEndpoint = flag.String("http-endpoint", "", "The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled.") | |||
| metricsPath = flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.") | |||
|
|
|||
| extraVolumeLabelsStr = flag.String("extra-labels", "", "Extra labels to attach to each PD created. It is a comma separated list of key value pairs like '<key1>=<value1>,<key2>=<value2>'. See https://cloud.google.com/compute/docs/labeling-resources for details") | |||
| errorBackoffInitialDurationMs = flag.Int("backoff-initial-duration-ms", 200, "The amount of ms for the initial duration of the backoff condition. Default is 200.") | |||
There was a problem hiding this comment.
... backoff condition for controller publish/unpublish CSI operations
k8s-ci-robot
added
size/M
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: judemars, mattcary 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| @@ -122,7 +124,9 @@ func handle() { | |||
| if err != nil { | |||
| klog.Fatalf("Failed to get cloud provider: %v", err) | |||
| } | |||
| controllerServer = driver.NewControllerServer(gceDriver, cloudProvider) | |||
| initialBackoffDuration := time.Duration(*errorBackoffInitialDurationMs) * time.Millisecond | |||
| maxBackoffDuration := time.Duration(*errorBackoffMaxDurationMs) * time.Microsecond | |||
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
The scenario of the failing test ("should fail when the volume is already published but is incompatible"):
ControllerUnpublish not permitted on node "projects/test-project/zones/country-region-zone/instances/test-name" due to backoff conditionbecause of Step 3.So, the solution we decided in #990 was to lower the duration of backoff for sanity tests so that the backoff triggered in step 3 above will not cause step 4 to fail.
See #990.
Which issue(s) this PR fixes:
Fixes #990
Special notes for your reviewer:
Does this PR introduce a user-facing change?: