Skip to content

Widen collector test timeouts and some cleansing.#8607

Merged
knative-prow-robot merged 1 commit intoknative:masterfrom
markusthoemmes:clean-metrics-collector-test
Jul 13, 2020
Merged

Widen collector test timeouts and some cleansing.#8607
knative-prow-robot merged 1 commit intoknative:masterfrom
markusthoemmes:clean-metrics-collector-test

Conversation

@markusthoemmes
Copy link
Copy Markdown
Contributor

Proposed Changes

Trying to fix failures like https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/8590/pull-knative-serving-unit-tests/1281749564260356098. Wasn't able to reproduce this locally in 100k runs so I'm assuming a consistency issue. It doesn't fail in CI often either so.

Also added a few touchups to the test code itself.

Release Note

NONE

/assign @julz

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 13, 2020
@knative-prow-robot knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 13, 2020
@knative-prow-robot
Copy link
Copy Markdown
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

Details Needs approval from an approver in each of these files:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 13, 2020
Copy link
Copy Markdown
Contributor

@knative-prow-robot knative-prow-robot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markusthoemmes: 0 warnings.

Details

In response to this:

Proposed Changes

Trying to fix failures like https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/8590/pull-knative-serving-unit-tests/1281749564260356098. Wasn't able to reproduce this locally in 100k runs so I'm assuming a consistency issue. It doesn't fail in CI often either so.

Also added a few touchups to the test code itself.

Release Note

NONE

/assign @julz

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.

Copy link
Copy Markdown
Contributor

@julz julz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2020
@knative-prow-robot knative-prow-robot merged commit 07a3f0a into knative:master Jul 13, 2020
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/autoscale cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants