Add activator DaemonSet #6624

vagababov · 2020-01-24T19:13:01Z

This change switches activator to be run as a daemon set.
The changes:

add daemon set
run with smaller footprint
reduce regular HPA to maxSize=1. This permits user to change the value, should the need arive, but will keep the default to a single instance.
also reduce its footprint, to match the DS ones.

/assign mattmoor

This change switches activator to be run as a daemon set. The changes: - add daemon set - run with smaller footprint - reduce regular HPA to maxSize=1. This permits user to change the value, should the need arive, but will keep the default to a single instance. - also reduce its footprint, to match the DS ones. For knative#6550

vagababov · 2020-01-24T19:20:26Z

An example run of the loadtest (up to 3k QPS) https://mako.dev/run?run_key=6498872555732992&~l=1

mattmoor · 2020-01-24T19:26:03Z

config/activator-ds.yaml

+      # connections.
+      terminationGracePeriodSeconds: 300
+
+---


I'd remove one of these services

mattmoor

/lgtm

mattmoor

/lgtm

config/activator-ds.yaml

dgerd · 2020-01-24T21:35:52Z

config/core/deployments/activator.yaml


        # The numbers are based on performance test results from
        # https://github.com/knative/serving/issues/1625#issuecomment-511930023
        resources:


If I run the instructions here: https://github.com/knative/serving/blob/master/DEVELOPMENT.md#deploy-knative-serving won't I get both the Deployment version and DaemonSet version?

Shouldn't you only run with one type?

On a similar note, how will this manifest in the v0.13.0 release instructions?

It should not change anything there.
But we need to document probably that we can turn this off.

Deployment wise nothing should change. Just execute the standard collective YAML.
But we need to document that you can pick or the other or both.

vagababov · 2020-01-24T21:55:46Z

/retest

config/activator-ds.yaml

vagababov · 2020-01-25T00:25:51Z

Interestingly only ScaleToN tests fail.... which should rely on activator, except scale from 0, which succeeds...

vagababov · 2020-01-25T01:01:04Z

/test pull-knative-serving-integration-tests

vagababov · 2020-01-25T01:46:38Z

OK, I'll take a look to see what's wrong :|

vagababov · 2020-01-28T23:59:34Z

/retest

vagababov · 2020-01-29T21:27:25Z

/retest

vagababov · 2020-01-29T22:38:05Z

/test all

knative-test-reporter-robot · 2020-01-29T23:19:02Z

The following jobs failed:

Test name	Triggers	Retries
pull-knative-serving-integration-tests	pull-knative-serving-integration-tests	1/3

Automatically retrying due to test flakiness...
/test pull-knative-serving-integration-tests

vagababov · 2020-01-29T23:26:40Z

/retest

vagababov · 2020-01-29T23:35:24Z

/retest

knative-metrics-robot · 2020-01-29T23:42:40Z

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/activator/net/revision_backends.go	93.1%	93.7%	0.6

vagababov · 2020-01-30T00:10:49Z

/test pull-knative-serving-istio-1.4-mesh

vagababov · 2020-01-30T00:10:57Z

/test pull-knative-serving-istio-1.3-mesh

knative-prow-robot · 2020-01-30T01:14:22Z

@vagababov: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-knative-serving-contour-latest	`aa84fc9`	link	`/test pull-knative-serving-contour-latest`
pull-knative-serving-ambassador-latest	`aa84fc9`	link	`/test pull-knative-serving-ambassador-latest`
pull-knative-serving-https	`aa84fc9`	link	`/test pull-knative-serving-https`
pull-knative-serving-gloo-0.17.1	`aa84fc9`	link	`/test pull-knative-serving-gloo-0.17.1`
pull-knative-serving-istio-1.3-mesh	`144caab`	link	`/test pull-knative-serving-istio-1.3-mesh`
pull-knative-serving-istio-1.4-mesh	`144caab`	link	`/test pull-knative-serving-istio-1.4-mesh`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

mattmoor · 2020-01-30T02:06:49Z

config/core/deployments/activator-hpa.yaml

+#   namespace: knative-serving
+#   labels:
+#     serving.knative.dev/release: devel
+# spec:


mattmoor · 2020-01-30T02:06:58Z

config/core/deployments/activator.yaml

  labels:
    serving.knative.dev/release: devel
 spec:
+  replicas: 1


mattmoor · 2020-01-30T03:54:23Z

config/activator-ds.yaml

@@ -0,0 +1 @@
+./core/activator-ds.yaml


Add trailing newline:

Suggested change

./core/activator-ds.yaml

./core/activator-ds.yaml

mattmoor

/lgtm

knative-prow-robot assigned mattmoor Jan 24, 2020

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jan 24, 2020

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 24, 2020

knative-prow-robot requested review from dgerd and vaikas January 24, 2020 19:13

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2020

mattmoor reviewed Jan 24, 2020

View reviewed changes

config/activator-ds.yaml Outdated

# connections.

terminationGracePeriodSeconds: 300

---

Copy link

Member

mattmoor Jan 24, 2020

Choose a reason for hiding this comment

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

I'd remove one of these services

licence + remove duplicate

6df6330

mattmoor reviewed Jan 24, 2020

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2020

remove the test patch, since we no longer need it

9757e05

knative-prow-robot added area/test-and-release It flags unit/e2e/conformance/perf test issues for product features and removed lgtm Indicates that a PR is ready to be merged. labels Jan 24, 2020

mattmoor reviewed Jan 24, 2020

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2020

dgerd reviewed Jan 24, 2020

View reviewed changes

make sure the activator DS is in core and aliased

fcb1e33

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2020

mattmoor reviewed Jan 24, 2020

View reviewed changes

config/activator-ds.yaml Show resolved Hide resolved

vagababov commented Jan 24, 2020

View reviewed changes

config/activator-ds.yaml Show resolved Hide resolved

vagababov added 2 commits January 28, 2020 11:05

Merge branch 'master' into 20200124-activator-ds

b82d036

TEST

75ef082

mattmoor reviewed Jan 28, 2020

View reviewed changes

vagababov force-pushed the 20200124-activator-ds branch from f2c518d to aa84fc9 Compare January 29, 2020 19:35

mattmoor reviewed Jan 29, 2020

View reviewed changes

test2

144caab

vagababov force-pushed the 20200124-activator-ds branch from aa84fc9 to 144caab Compare January 29, 2020 23:36

mattmoor reviewed Jan 29, 2020

View reviewed changes

merge

1997d8f

mattmoor reviewed Jan 30, 2020

View reviewed changes

config/core/deployments/activator.yaml Outdated

labels:

serving.knative.dev/release: devel

spec:

replicas: 1

Copy link

Member

mattmoor Jan 30, 2020

Choose a reason for hiding this comment

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

remove

mattmoor reviewed Jan 30, 2020

View reviewed changes

vagababov force-pushed the 20200124-activator-ds branch from 9827ff0 to a4579d3 Compare January 30, 2020 03:53

mattmoor reviewed Jan 30, 2020

View reviewed changes

fix hpa

0c78da6

vagababov force-pushed the 20200124-activator-ds branch from a4579d3 to 0c78da6 Compare January 30, 2020 03:54

mattmoor reviewed Jan 30, 2020

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 30, 2020

knative-prow-robot merged commit 989918a into knative:master Jan 30, 2020

jcrossley3 mentioned this pull request Feb 2, 2020

Extend image override logic to Daemonsets knative/serving-operator#280

Closed

savitaashture mentioned this pull request Feb 3, 2020

Add image override logic for DaemonSet knative/serving-operator#281

Merged

vagababov deleted the 20200124-activator-ds branch June 23, 2020 23:16

		@@ -0,0 +1 @@
		./core/activator-ds.yaml No newline at end of file

Add activator DaemonSet #6624

Add activator DaemonSet #6624

Uh oh!

Conversation

vagababov commented Jan 24, 2020

Uh oh!

vagababov commented Jan 24, 2020

Uh oh!

mattmoor Jan 24, 2020

Choose a reason for hiding this comment

Uh oh!

mattmoor left a comment

Choose a reason for hiding this comment

Uh oh!

mattmoor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dgerd Jan 24, 2020

Choose a reason for hiding this comment

Uh oh!

vagababov Jan 24, 2020

Choose a reason for hiding this comment

Uh oh!

vagababov Jan 24, 2020

Choose a reason for hiding this comment

Uh oh!

vagababov commented Jan 24, 2020

Uh oh!

Uh oh!

Uh oh!

vagababov commented Jan 25, 2020

Uh oh!

vagababov commented Jan 25, 2020

Uh oh!

vagababov commented Jan 25, 2020

Uh oh!

vagababov commented Jan 28, 2020

Uh oh!

vagababov commented Jan 29, 2020

Uh oh!

vagababov commented Jan 29, 2020

Uh oh!

knative-test-reporter-robot commented Jan 29, 2020

Uh oh!

vagababov commented Jan 29, 2020

Uh oh!

vagababov commented Jan 29, 2020

Uh oh!

knative-metrics-robot commented Jan 29, 2020

Uh oh!

vagababov commented Jan 30, 2020

Uh oh!

vagababov commented Jan 30, 2020

Uh oh!

knative-prow-robot commented Jan 30, 2020

Uh oh!

mattmoor Jan 30, 2020

Choose a reason for hiding this comment

Uh oh!

mattmoor Jan 30, 2020

Choose a reason for hiding this comment

Uh oh!

mattmoor Jan 30, 2020

Choose a reason for hiding this comment

Uh oh!

mattmoor left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants