Bug 1912522: test/e2e: Re-enable TestCoreDNSImageUprade #230
Conversation
openshift-ci-robot
added
the
bugzilla/severity-medium
|
@sgreene570: This pull request references Bugzilla bug 1912522, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
openshift-ci-robot
added
the
bugzilla/valid-bug
openshift-ci-robot
added
the
approved
|
/retest |
|
Let's make sure the first pass wasn't a fluke. |
test/e2e/operator_test.go
Outdated
| // Remove cluster version override once the test returns. | ||
| defer func() { | ||
| if err := cl.Get(context.TODO(), cvName, clusterVersion); err != nil { | ||
| t.Logf("failed to get cluster version %s/%s: %v", cvName.Namespace, cvName.Name, err) |
There was a problem hiding this comment.
I really think we should fail harder. Can the next test continue if this fails?
There was a problem hiding this comment.
Arguably yes, the following tests could run, but it's probably best that we ensure cluster version is in a clean state first.
There was a problem hiding this comment.
Also made the prior cluster version override API calls hard fail the test if they fail. Thanks!
sgreene570
force-pushed
the
bz-1912522
branch
2 times, most recently
from
fd8b39b to
1210945
Compare
test/e2e/operator_test.go
Outdated
|
|
||
| func checkCurrentDNSImage(t *testing.T, cl client.Client, expectedImage string) error { | ||
| err := wait.PollImmediate(1*time.Second, 7*time.Minute, func() (bool, error) { | ||
| err := wait.PollImmediate(1*time.Second, 10*time.Minute, func() (bool, error) { |
There was a problem hiding this comment.
What was the motivation for both the timeout changes? CI runs? other?
There was a problem hiding this comment.
I observed checkCurrentDNSImage taking close to the 7 minute timeout when I ran the test on my own cluster. Just added some cushion. Dropped the 5 minute timeout for fetching the deployment since that just seems excessive.
I can revert these changes if you'd prefer.
There was a problem hiding this comment.
I can revert these changes if you'd prefer.
If they're currently OK and passing in CI I'd be inclined to revert.
test/e2e/operator_test.go: Re-enable TestCoreDNSImageUpgrade by adding a cluster version override for the DNS operator deployment. This override allows TestCoreDNSImageUpgrade to safely modify the DNS deployment (specifically the operator's container image field) without contention from the CVO. The cluster version override is removed at the end of the test via a `defer` block.
sgreene570
force-pushed
the
bz-1912522
branch
from
1210945 to
72aef2b
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: frobware, sgreene570 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@sgreene570: All pull requests linked via external trackers have merged: Bugzilla bug 1912522 has been moved to the MODIFIED state. DetailsIn response to this:
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. |
4 participants
test/e2e/operator_test.go:
Re-enable TestCoreDNSImageUpgrade by adding a
cluster version override for the DNS operator deployment. This override
allows TestCoreDNSImageUpgrade to safely modify the DNS deployment
(specifically the operator's container image field) without contention
from the CVO. The cluster version override is removed at the end of the
test via a
deferblock.