feat: preliminary dapr runtime support #1518

lkingland · 2023-01-23T15:39:05Z

Changes

🎁 adds preliminary support for the dapr runtime

Adds support for the Dapr runtime from within Functions. Note that the control plane must be installed in the Kubernetes cluster targeted for deployment:

dapr init --kubernetes --wait

This PR also integrates installation of the Dapr control plane, along with a Redis-backed pub/sub and persistence store, to test/local clusters via ./hack/allocate.sh.

/kind enhancement

Release Note

Enables Dapr runtime support from within Functions.  Dapr control plane install required.

Docs

TODO

codecov · 2023-01-23T15:43:35Z

Codecov Report

Base: 62.86% // Head: 63.56% // Increases project coverage by +0.70% 🎉

Coverage data is based on head (d674daa) compared to base (6a539e4).
Patch coverage: 75.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1518      +/-   ##
==========================================
+ Coverage   62.86%   63.56%   +0.70%     
==========================================
  Files          74       75       +1     
  Lines       10793    10818      +25     
==========================================
+ Hits         6785     6877      +92     
+ Misses       3447     3376      -71     
- Partials      561      565       +4

Flag	Coverage Δ
integration-tests	`54.36% <75.00%> (+1.09%)`	⬆️
unit-tests	`?`
unit-tests-macos-latest	`52.97% <0.00%> (?)`
unit-tests-ubuntu-latest	`54.37% <0.00%> (?)`
unit-tests-windows-latest	`53.02% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
client.go	`60.73% <ø> (+2.70%)`	⬆️
knative/deployer.go	`63.50% <75.00%> (+2.03%)`	⬆️
docker/docker_client_nonlinux.go	`0.00% <0.00%> (ø)`
knative/logs.go	`85.56% <0.00%> (+1.03%)`	⬆️
docker/creds/credentials.go	`73.06% <0.00%> (+1.34%)`	⬆️
k8s/secrets.go	`19.58% <0.00%> (+3.09%)`	⬆️
invoke.go	`55.39% <0.00%> (+7.19%)`	⬆️
instances.go	`83.87% <0.00%> (+8.06%)`	⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

knative/deployer.go

lance

Overall this lgtm - what's holding it in draft?

matzew · 2023-01-30T09:35:53Z

knative/deployer.go

+	aa = make(map[string]string)
+
+	// Static default: Dapr support
+	aa["dapr.io/enabled"] = "true"


should these be constants, eventually?

I'd perhaps also populate all dapr related annotations in a separated call 🤷‍♂️

+1, the same way we have constants for labels: https://github.com/knative/func/tree/main/k8s/labels

also do we want to apply Dapr annotations to all functions by default?

Thanks for the reviews, please see the latest for:

labels defined in labels.go

Annotations are now extracted and added in the same manner as the functions' annotation map below (looping over the map keys) for consistency

the annotations define how Dapr can connect its control plane to the service if it is installed, otherwise has no effect

lkingland · 2023-01-30T11:07:00Z

Overall this lgtm - what's holding it in draft?

Implemented some basic integration tests ensuring things are working as expected. That led to two new PRs improving the client library API for clarity of those tests. The first of those is currently open for review: #1529 PTAL

- installs Dapr cli in CI - installs Dapr runtime on allocation of test cluster - annotates services to enable dapr sidecar integration - installs redis via helm, enabling state store, pub/sub and distributed lock - integration test added for local invocation - integration test added for service-to-service invocation via the sidecar Note that Dapr runs metrics on port 9002 so as not to collide with Knative metrics.

salaboy

/lgtm awesome work @lkingland ! great to have some integration tests for this, we can expand that later

knative-prow · 2023-02-02T15:56:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lkingland, salaboy

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:

~~OWNERS~~ [lkingland,salaboy]

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

* Dapr runtime support - installs Dapr cli in CI - installs Dapr runtime on allocation of test cluster - annotates services to enable dapr sidecar integration - installs redis via helm, enabling state store, pub/sub and distributed lock - integration test added for local invocation - integration test added for service-to-service invocation via the sidecar Note that Dapr runs metrics on port 9002 so as not to collide with Knative metrics. * create constants for knative service labels * extract dapr annotations and use labels

knative-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/enhancement size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 23, 2023

knative-prow bot requested review from jrangelramos and maximilien January 23, 2023 15:39

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 23, 2023

lance reviewed Jan 27, 2023

View reviewed changes

knative/deployer.go Outdated Show resolved Hide resolved

lance reviewed Jan 28, 2023

View reviewed changes

matzew reviewed Jan 30, 2023

View reviewed changes

lkingland force-pushed the dapr-initial-runtime-support branch from 3927c63 to 92f9cce Compare January 30, 2023 10:01

knative-prow bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 30, 2023

lkingland marked this pull request as ready for review January 30, 2023 10:59

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2023

lkingland added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2023

lkingland marked this pull request as draft January 30, 2023 11:02

knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2023

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 1, 2023

lkingland added 3 commits February 2, 2023 22:39

create constants for knative service labels

e1daa56

extract dapr annotations and use labels

d674daa

lkingland force-pushed the dapr-initial-runtime-support branch from 815d4a3 to d674daa Compare February 2, 2023 13:39

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2023

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 2, 2023

lkingland marked this pull request as ready for review February 2, 2023 13:39

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 2, 2023

knative-prow bot requested a review from dsimansk February 2, 2023 13:39

lkingland requested review from zroubalik, salaboy and lance and removed request for maximilien and dsimansk February 2, 2023 13:41

salaboy approved these changes Feb 2, 2023

View reviewed changes

knative-prow bot assigned salaboy Feb 2, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 2, 2023

lkingland removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 2, 2023

knative-prow bot merged commit 10c9948 into knative:main Feb 2, 2023

lkingland mentioned this pull request Feb 17, 2023

Dapr runtime for functions #1427

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: preliminary dapr runtime support #1518

feat: preliminary dapr runtime support #1518

lkingland commented Jan 23, 2023 •

edited

Loading

codecov bot commented Jan 23, 2023 •

edited

Loading

lance left a comment

matzew Jan 30, 2023

zroubalik Jan 30, 2023

zroubalik Jan 30, 2023

lkingland Jan 30, 2023 •

edited

Loading

lkingland commented Jan 30, 2023 •

edited

Loading

salaboy left a comment

knative-prow bot commented Feb 2, 2023

feat: preliminary dapr runtime support #1518

feat: preliminary dapr runtime support #1518

Conversation

lkingland commented Jan 23, 2023 • edited Loading

Changes

codecov bot commented Jan 23, 2023 • edited Loading

Codecov Report

lance left a comment

Choose a reason for hiding this comment

matzew Jan 30, 2023

Choose a reason for hiding this comment

zroubalik Jan 30, 2023

Choose a reason for hiding this comment

zroubalik Jan 30, 2023

Choose a reason for hiding this comment

lkingland Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

lkingland commented Jan 30, 2023 • edited Loading

salaboy left a comment

Choose a reason for hiding this comment

knative-prow bot commented Feb 2, 2023

lkingland commented Jan 23, 2023 •

edited

Loading

codecov bot commented Jan 23, 2023 •

edited

Loading

lkingland Jan 30, 2023 •

edited

Loading

lkingland commented Jan 30, 2023 •

edited

Loading