Implementation for event driven tink #301

gauravgahlot · 2020-09-21T13:10:03Z

Description

This PR provides the implementation for event-driven Tinkerbell base model proposed here.

Note
We would like to keep Watch as the function used to stream events for the new event-driven tink. Therefore, the existing hardware.Watch has been renamed to hardware.DeprecatedWatch.

This will also require some changes in Hegel, so that it can use the hardware.DeprecatedWatch.

Why is this needed

Fixes: #294

How Has This Been Tested?

Unit tests and manual testing.

How are existing users impacted? What migration steps/scripts do we need?

This contains a bc-break. Watch has been renamed to DeprecatedWatch, clients will need to be regenerated and upgraded.

Checklist:

I have:

updated the documentation and/or roadmap (if required)
added unit or e2e tests
provided instructions on how to upgrade

Known issues in progress (fixed)

an event being received twice on the same stream
tink hardware watch <id> fails to correctly print the hardware data for new events

codecov · 2020-09-21T13:11:50Z

Codecov Report

Merging #301 (14badba) into master (1992c1c) will increase coverage by 2.64%.
The diff coverage is 60.95%.

@@            Coverage Diff             @@
##           master     #301      +/-   ##
==========================================
+ Coverage   26.09%   28.73%   +2.64%     
==========================================
  Files          14       19       +5     
  Lines        1280     1385     +105     
==========================================
+ Hits          334      398      +64     
- Misses        921      960      +39     
- Partials       25       27       +2

Impacted Files	Coverage Δ
grpc-server/events.go	`0.00% <0.00%> (ø)`
grpc-server/grpc_server.go	`0.00% <0.00%> (ø)`
grpc-server/hardware.go	`1.55% <ø> (ø)`
client/informers/informer.go	`20.00% <20.00%> (ø)`
cmd/tink-cli/cmd/events.go	`33.33% <33.33%> (ø)`
client/informers/notification.go	`88.67% <88.67%> (ø)`
client/informers/mock.go	`100.00% <100.00%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1992c1c...14badba. Read the comment docs.

deploy/db/tinkerbell-init.sql

protos/event/event.proto

gianarb · 2020-09-22T14:35:36Z

According to the demo @gauravgahlot gave today here some feedback.
I have a couple of things to say about the UX and on the feature itself.

I think we should not use an integer for event types and resource_type we should use strings. They are way more readable as constant and it won't require to actually have a catalog of them. A few bytes to store but who cares. If we start supporting custom events and types it will be impossible to keep a centralized index.

We have to figure out how to effectively deserialize data from bytes to struct and JSON.

The CLI I think should reflect that filters we will have to implement something like --resource-type --resource-id --event-type. The --frame can be a more clear --from 5m (that's should be the default value)

Other than that, great start!

protos/events/events.proto

mmlb · 2020-09-25T14:40:04Z

@splaspood fyi

protos/events/events.proto

deploy/db/tinkerbell-init.sql

mmlb · 2020-09-25T15:00:37Z

deploy/db/tinkerbell-init.sql

+
+CREATE TRIGGER hardware_trigger
+AFTER INSERT OR UPDATE ON hardware
+FOR EACH ROW EXECUTE PROCEDURE hardware_event();


we should also ensure the old entries are trimmed away right?

protos/protoc.sh

gauravgahlot · 2020-12-07T07:54:35Z

Updated branch with a rebase, no changes.

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

gauravgahlot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge Signal to Mergify to block merging of the PR. labels Sep 21, 2020

gauravgahlot self-assigned this Sep 21, 2020

gauravgahlot force-pushed the event-driven-tink branch from d8a26b2 to 3580dc4 Compare September 21, 2020 13:10

gauravgahlot force-pushed the event-driven-tink branch 3 times, most recently from 67b9cbc to 4782daf Compare September 21, 2020 14:27

mmlb suggested changes Sep 21, 2020

View reviewed changes

parauliya force-pushed the event-driven-tink branch 2 times, most recently from be4a874 to 57cdc4e Compare September 22, 2020 05:14

gauravgahlot force-pushed the event-driven-tink branch from 57cdc4e to 08b5a24 Compare September 22, 2020 05:26

gianarb requested a review from mmlb September 22, 2020 07:49

parauliya force-pushed the event-driven-tink branch from 6ec4f5c to 7e3bd2e Compare September 22, 2020 12:59

gauravgahlot force-pushed the event-driven-tink branch 2 times, most recently from ebf0993 to 94c0177 Compare September 22, 2020 13:17

gauravgahlot force-pushed the event-driven-tink branch from 6d4ceaf to 1b00099 Compare September 23, 2020 10:31

nathangoulding previously requested changes Sep 23, 2020

View reviewed changes

protos/events/events.proto Outdated Show resolved Hide resolved

protos/events/events.proto Outdated Show resolved Hide resolved

gauravgahlot force-pushed the event-driven-tink branch 6 times, most recently from a637b63 to b42b2c7 Compare September 25, 2020 10:02

mmlb reviewed Sep 25, 2020

View reviewed changes

gauravgahlot force-pushed the event-driven-tink branch 2 times, most recently from 9d4faa2 to cf5fc78 Compare September 29, 2020 04:03

gianarb requested review from nathangoulding and removed request for nathangoulding December 3, 2020 10:48

mmlb previously approved these changes Dec 4, 2020

View reviewed changes

gauravgahlot dismissed stale reviews from mmlb and gianarb via a87aa36 December 7, 2020 07:53

gauravgahlot force-pushed the event-driven-tink branch from a0d634b to a87aa36 Compare December 7, 2020 07:53

gianarb previously approved these changes Dec 7, 2020

View reviewed changes

gianarb requested a review from mmlb December 7, 2020 13:32

gauravgahlot added 13 commits December 8, 2020 00:12

rename hardware.Watch to hardware.DeprecatedWatch

c3538e6

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

added events.proto

3208686

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

gRPC client and server for events

bc98522

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

adding informers and listener

6098275

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

CLI: tink event watch

60a34fd

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

added 'eventtypes' to codespell-whitelist

4ec28fe

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

go mod tidy

6a67c95

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

db migration script for events

125a40c

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

CLI: tink hardware watch

e25da81

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

adding tests

5baa34f

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

purging events after a TTL

9e87913

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

review fixes

fafe3bf

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

formatting events to be readable

14badba

Signed-off-by: Gaurav Gahlot <gauravgahlot0107@gmail.com>

gauravgahlot dismissed gianarb’s stale review via 14badba December 7, 2020 18:42

gauravgahlot force-pushed the event-driven-tink branch from a87aa36 to 14badba Compare December 7, 2020 18:42

mmlb approved these changes Dec 7, 2020

View reviewed changes

gauravgahlot removed the request for review from nathangoulding December 7, 2020 18:47

mergify bot merged commit 9c52cca into tinkerbell:master Dec 7, 2020

gauravgahlot deleted the event-driven-tink branch December 7, 2020 18:48

mmlb removed the ready-to-merge Signal to Mergify to merge the PR. label Jan 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation for event driven tink #301

Implementation for event driven tink #301

gauravgahlot commented Sep 21, 2020 •

edited

Loading

codecov bot commented Sep 21, 2020 •

edited

Loading

gianarb commented Sep 22, 2020 •

edited

Loading

mmlb commented Sep 25, 2020

mmlb Sep 25, 2020

gauravgahlot commented Dec 7, 2020

Implementation for event driven tink #301

Implementation for event driven tink #301

Conversation

gauravgahlot commented Sep 21, 2020 • edited Loading

Description

Why is this needed

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

Known issues in progress (fixed)

codecov bot commented Sep 21, 2020 • edited Loading

Codecov Report

gianarb commented Sep 22, 2020 • edited Loading

mmlb commented Sep 25, 2020

mmlb Sep 25, 2020

Choose a reason for hiding this comment

gauravgahlot commented Dec 7, 2020

gauravgahlot commented Sep 21, 2020 •

edited

Loading

codecov bot commented Sep 21, 2020 •

edited

Loading

gianarb commented Sep 22, 2020 •

edited

Loading