-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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.
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
e2e: do not parse resourceVersion #108638
e2e: do not parse resourceVersion #108638
Conversation
9e6d167
to
94939c5
Compare
/triage accepted |
@liggitt the framework will create an individual namespace per |
This test wishes to observe a watch event. In order to do this in the past, the test chose a well-known `Service` object, fetched it, and did arithmetic on the returned `resourceVersion` in order to start a watch that was guaranteed to see an event. It is not valid to parse the `resourceVersion` as an integer or to do arithmetic on it, so in order to make the test conformant to an appropriate use of the API it now: - creates a namespace - fetches the current `resourceVersion` - creates an object - watches from the previous `resourceVersion` that was read This ensures that an event is seen by the watch, but uses the publically supported API. `ConfigMap`s are used instead of `Service`s as they do not require a valid `spec` for creation and make the test terser. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
94939c5
to
305652a
Compare
almost... it wouldn't have been happy with a delete current code lgtm, since this is modifying a conformance test, mind triggering e2es a few times (maybe get 10 clean runs?) to demonstrate it passes reliably |
@liggitt sorry, yes - that is what I meant. No deletes with the way this works now, either. I just realized this is not a conformance test, just a normal e2e. Is there a particular job you want to see repeated? |
ah, in that case /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, stevekuznetsov 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 |
TFTR jordan |
This test wishes to observe a watch event. In order to do this in the
past, the test chose a well-known
Service
object, fetched it, and didarithmetic on the returned
resourceVersion
in order to start a watchthat was guaranteed to see an event. It is not valid to parse the
resourceVersion
as an integer or to do arithmetic on it, so in orderto make the test conformant to an appropriate use of the API it now:
resourceVersion
resourceVersion
that was readThis ensures that an event is seen by the watch, but uses the publically
supported API.
ConfigMap
s are used instead ofService
s as they do not require avalid
spec
for creation and make the test terser.Signed-off-by: Steve Kuznetsov skuznets@redhat.com
/kind bug
/kind cleanup
/sig api-machinery
/assign @liggitt @smarterclayton @deads2k @sttts