Skip to content

Remove endpoints from KPA as well.#8616

Merged
knative-prow-robot merged 2 commits intoknative:masterfrom
vagababov:20200713-remove-eps
Jul 13, 2020
Merged

Remove endpoints from KPA as well.#8616
knative-prow-robot merged 2 commits intoknative:masterfrom
vagababov:20200713-remove-eps

Conversation

@vagababov
Copy link
Copy Markdown
Contributor

@vagababov vagababov commented Jul 13, 2020

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 :)

As a follow up I want to add a new method to pods accessor to fetch all 4 types at the same time to save us some computations.

/lint

/assign mattmoor @julz @yanweiguo

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 :)
@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/L Denotes a PR that changes 100-499 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: vagababov

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.

@vagababov: 0 warnings.

Details

In response to this:

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 :)

/lint

/assign mattmoor @julz @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 knative-prow-robot added area/API API objects and controllers area/autoscale labels Jul 13, 2020
Comment thread pkg/reconciler/autoscaling/kpa/kpa_test.go Outdated
Comment thread pkg/reconciler/autoscaling/kpa/kpa_test.go
@knative-metrics-robot
Copy link
Copy Markdown

The following is the coverage report on the affected files.
Say /test pull-knative-serving-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/autoscaling/kpa/controller.go 94.1% 93.9% -0.2
pkg/reconciler/autoscaling/kpa/kpa.go 93.9% 92.8% -1.1

@yanweiguo
Copy link
Copy Markdown
Contributor

/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 9e69ff8 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/API API objects and controllers area/autoscale cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants