Add route consistency retry to new logging E2E test.#8324
Merged
knative-prow-robot merged 1 commit intoknative:masterfrom Jul 13, 2020
Merged
Add route consistency retry to new logging E2E test.#8324knative-prow-robot merged 1 commit intoknative:masterfrom
knative-prow-robot merged 1 commit intoknative:masterfrom
Conversation
knative-prow-robot
added
the
size/S
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: markusthoemmes 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 |
knative-prow-robot
added
the
approved
Contributor
knative-prow-robot
left a comment
knative-prow-robot
left a comment
There was a problem hiding this comment.
@markusthoemmes: 0 warnings.
Details
In response to this:
Ref #5573
Proposed Changes
Not much more to say. We should maybe think about adding this to the
CreateServiceReadyhelpers so it isn't forgotten in the future.Release Note
NONE/assign @vagababov @yanweiguo
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.
knative-prow-robot
added
the
area/test-and-release
Contributor
|
Integrate into https://github.com/knative/serving/pull/8326/files perhaps? |
yanweiguo
reviewed
Jun 15, 2020
| } | ||
|
|
||
| _, err = sendRequest(t, clients, test.ServingFlags.ResolvableDomain, resources.Route.Status.URL.URL()) | ||
| _, err = pkgTest.WaitForEndpointState( |
Contributor
There was a problem hiding this comment.
I still don't know why we need this.
Contributor
Author
There was a problem hiding this comment.
Because downstream projects might use external domains that actually need DNS setup for example, or external loadbalancers. It's a bandaid I brought up multiple times maybe I should bring it up to Networking WG again. It needs to be well understood and harder to hold wrong.
Contributor
There was a problem hiding this comment.
I think you were thinking of keeping the bandaid behind a flag?
Contributor
There was a problem hiding this comment.
Could you add a comment for it?
Contributor
Author
|
@yanweiguo @tcnghia Sooooo... should we look at reworking this completely then? Seems like we need a better way of doing things downstream. We could still merge this one for consistency or close it. I'll leave that up to you. |
Contributor
|
I think we can merge this one. /lgtm |
whaught
added a commit
to whaught/serving
that referenced
this pull request
Jul 14, 2020
alpha note added update checksum fix comments checksum and validation Update config/core/configmaps/gc.yaml Co-authored-by: Victor Agababov <vagababov@gmail.com> fix comments fix unit test fix both check include unit test make new settings for v2 v2 gc settings fix defaults default values fix unit test. expand comment fix validation error Widen collector test timeouts and some cleansing. (knative#8607) Make sorting consistent in GC test. (knative#8606) One of the tests ("keep recent lastPinned") failed fairly often because it used two revisions with the same LastPinned timestamp. Sorting a slice is unstable so in a lot of occasions, the given order was flipped and revision 5555 was tried to be deleted, even though it shouldn't even have been considered to be deleted. Note: sort.SliceStable doesn't help because the incoming list's order is already non-consistent. [master] Auto-update dependencies (knative#8611) Produced via: `./hack/update-deps.sh --upgrade && ./hack/update-codegen.sh` /assign dprotaso vagababov /cc dprotaso vagababov Add route consistency retry to new logging E2E test. (knative#8324) Handle InitialScale != 1 when creating multiScaler and some tidy-ups (knative#8612) Now InitialScale has landed, take it into account when calculating initial EBC in multiscaler. Also: - Tidy up some comments. - Fix up some test error messages. - Move mutexes above the things they're guarding. - Use %d and %0.3f for printing numbers, consistently with existing Debug statement at start of same method. Enable HA by default (knative#8602) * Enable HA by default * Centralize the bucket/replica information for running the HA testing. Add a test to ensure the count of the controller's reconciler stays in sync with the actual controller binary. Remove endpoints from KPA as well. (knative#8616) * Remove endpoints from KPA as well. Today I noticed that I missed the endpoints code in the KPA itself, when removing that informer. So now really get rid of those. The tests are of course nightmarish :) * remove old cruft Assorted fixes to enable chaos duck. (knative#8619) * Assorted fixes to enable chaos duck. This lands a handful of fixes that I uncovered preparing to run controlplane chaos testing during our e2e tests. * Drop sleep to 10s
markusthoemmes
added a commit
to openshift/knative-serving
that referenced
this pull request
Jul 21, 2020
knative-prow-robot
pushed a commit
that referenced
this pull request
Jul 21, 2020
* Create a maximum revision GC setting * update yaml alpha note added update checksum fix comments checksum and validation Update config/core/configmaps/gc.yaml Co-authored-by: Victor Agababov <vagababov@gmail.com> fix comments fix unit test fix both check include unit test make new settings for v2 v2 gc settings fix defaults default values fix unit test. expand comment fix validation error Widen collector test timeouts and some cleansing. (#8607) Make sorting consistent in GC test. (#8606) One of the tests ("keep recent lastPinned") failed fairly often because it used two revisions with the same LastPinned timestamp. Sorting a slice is unstable so in a lot of occasions, the given order was flipped and revision 5555 was tried to be deleted, even though it shouldn't even have been considered to be deleted. Note: sort.SliceStable doesn't help because the incoming list's order is already non-consistent. [master] Auto-update dependencies (#8611) Produced via: `./hack/update-deps.sh --upgrade && ./hack/update-codegen.sh` /assign dprotaso vagababov /cc dprotaso vagababov Add route consistency retry to new logging E2E test. (#8324) Handle InitialScale != 1 when creating multiScaler and some tidy-ups (#8612) Now InitialScale has landed, take it into account when calculating initial EBC in multiscaler. Also: - Tidy up some comments. - Fix up some test error messages. - Move mutexes above the things they're guarding. - Use %d and %0.3f for printing numbers, consistently with existing Debug statement at start of same method. Enable HA by default (#8602) * Enable HA by default * Centralize the bucket/replica information for running the HA testing. Add a test to ensure the count of the controller's reconciler stays in sync with the actual controller binary. Remove endpoints from KPA as well. (#8616) * Remove endpoints from KPA as well. Today I noticed that I missed the endpoints code in the KPA itself, when removing that informer. So now really get rid of those. The tests are of course nightmarish :) * remove old cruft Assorted fixes to enable chaos duck. (#8619) * Assorted fixes to enable chaos duck. This lands a handful of fixes that I uncovered preparing to run controlplane chaos testing during our e2e tests. * Drop sleep to 10s * Use the new GC settings * actually use the new settings * simplify * now it compiles * fix config references * test with min settings * max tests * change name of max to max-non-active * comments and validate positive * parse forever constant * fixing the unit test, better examples * include disabled setinel * merge in settings * disabled logic * update comments * review suggestions * fix const * update with logging changes * fix unit test * comment settings * unit test update * nits * fix unit test names * more consistent name * fix max boundary * nit * remove ref to lastpinned * review suggestions * fix sign * use nonactive as min * first filter out active * comment nit on config * swap if/else ordering * simplify * whitespace
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ref #5573
Proposed Changes
Not much more to say. We should maybe think about adding this to the
CreateServiceReadyhelpers so it isn't forgotten in the future.Release Note
/assign @vagababov @yanweiguo