[Merged by Bors] - Decouple cluster components from Fluvio cluster #928

nicholastmosher · 2021-04-05T12:30:17Z

Closes #866

Removes fluvio-runner-local as a dependency from fluvio-cluster
Renames fluvio-runner-local to fluvio-run and makes it a standalone executable
Adds fluvio-run as an independent plugin on packages.fluvio.io
Updates install.sh to include fluvio-run as a default plugin during installation

nicholastmosher · 2021-04-12T21:03:19Z

I can update the PR description so that it becomes a nice commit message when Bors squashes it 👍

sehz

great work. few minor comments

src/cluster/src/delete.rs

src/runner/Cargo.toml

src/sc/build.rs

src/cluster/Cargo.toml

First part of #928. Re-organize Leader and Follower replica such that it can be better refactored. Decouple replication logic from storage logic much as possible. Make it easier to write unit test replication logic

src/command/src/lib.rs

src/runner/Cargo.toml

sehz · 2021-04-13T16:49:32Z

Running smoke test didn't delete run components:

make smoke-test
cargo build   --bin fluvio 
    Finished dev [unoptimized + debuginfo] target(s) in 0.19s
cargo build   --bin fluvio-run 
    Finished dev [unoptimized + debuginfo] target(s) in 0.20s
cargo build   --bin flv-test 
    Finished dev [unoptimized + debuginfo] target(s) in 0.20s
echo "clean up previous installation"
clean up previous installation
./target/debug/fluvio cluster delete
./target/debug/fluvio cluster delete --local
kubectl delete configmap authorization --ignore-not-found
FLUVIO_CMD=true ./target/debug/flv-test smoke --spu 1 --local --client-log warn --server-log fluvio=debug --skip-checks  -- --producer-iteration=1000
Start running fluvio test runner
deleting cluster
installing cluster
Performing pre-flight checks
✅ ok: Supported helm version is installed
✅ ok: Supported kubernetes version is installed
✅ ok: Kubernetes config is loadable
✅ ok: Fluvio system charts are installed
Creating the topic: test
topic "test" created
found topic: test offset: 0
starting fetch stream for: test base offset: 0, expected new records: 1000
<<consume test done for: test >>>>
consume message validated!, records: 1000
Test completed
deleting cluster
+--------------+---------------+
| Test Results |               |
+--------------+---------------+
| Pass?        | true          |
+--------------+---------------+
| Duration     | 12.267184004s |
+--------------+---------------+

ubuntu@ip-172-31-20-82:~/fluvio$ ps -ef | grep fluvio
ubuntu   3645556       1  0 16:45 pts/11   00:00:00 /home/ubuntu/fluvio/target/debug/fluvio run sc
ubuntu   3645566 3645556 88 16:45 pts/11   00:02:45 /home/ubuntu/fluvio/target/debug/fluvio-run sc
ubuntu   3645583       1  0 16:45 pts/11   00:00:00 /home/ubuntu/fluvio/target/debug/fluvio run spu -i 5001 -p 0.0.0.0:9010 -v 0.0.0.0:9011 --log-base-dir /tmp/fluvio
ubuntu   3645593 3645583  6 16:45 pts/11   00:00:11 /home/ubuntu/fluvio/target/debug/fluvio-run spu -i 5001 -p 0.0.0.0:9010 -v 0.0.0.0:9011 --log-base-dir /tmp/fluvio
ubuntu   3652910 3000105  0 16:48 pts/11   00:00:00 grep --color=auto fluvio

sehz · 2021-04-13T16:50:48Z

Also, can you update DEVELOPER.md?

sehz · 2021-04-13T20:10:11Z

Delete still doesn't work in Linux:

flvd cluster delete --local
ubuntu@ip-172-31-20-82:~/fluvio$ ps -ef | grep fluvio
ubuntu   3645556       1  0 16:45 pts/11   00:00:00 /home/ubuntu/fluvio/target/debug/fluvio run sc
ubuntu   3645566 3645556  1 16:45 pts/11   00:02:52 /home/ubuntu/fluvio/target/debug/fluvio-run sc
ubuntu   3645583       1  0 16:45 pts/11   00:00:00 /home/ubuntu/fluvio/target/debug/fluvio run spu -i 5001 -p 0.0.0.0:9010 -v 0.0.0.0:9011 --log-base-dir /tmp/fluvio
ubuntu   3645593 3645583  0 16:45 pts/11   00:00:36 /home/ubuntu/fluvio/target/debug/fluvio-run spu -i 5001 -p 0.0.0.0:9010 -v 0.0.0.0:9011 --log-base-dir /tmp/fluvio
ubuntu   3737568 3000105  0 20:09 pts/11   00:00:00 grep --color=auto fluvio

sehz · 2021-04-13T20:14:38Z

Also, there are two process spawn for each SC and SPU. Can we collapse into a single process for each?

For example, running sc spawn two process.

ubuntu   3738481       1  0 20:10 pts/11   00:00:00 /home/ubuntu/fluvio/target/debug/fluvio run sc
ubuntu   3738493 3738481  2 20:10 pts/11   00:00:00 /home/ubuntu/fluvio/target/debug/fluvio-run sc

nicholastmosher · 2021-04-13T20:20:43Z

I noticed that as well, the reason that there are two processes is because the actual invocation for cluster components is now

fluvio run spu -> fluvio-run spu, and
fluvio run sc -> fluvio-run sc.

This has to do with how we use fluvio to call out to other subcommands, and right now that simply runs the subcommand inside itself rather than spawning as an independent process. I think this is better for now because otherwise, running any kind of external fluvio subcommand would appear to just quit the prompt early, even though the subcommand is still running. I think this would really hurt plugins like fluvio-cloud.

So I think it's fine how it is right now. I'll make a quick change that will hopefully fix the pkill on linux.

sehz · 2021-04-13T20:31:15Z

Hooray, cluster delete finally work!

nicholastmosher · 2021-04-13T20:34:27Z

Whoops, I broke local test, one sec

nicholastmosher · 2021-04-13T20:49:17Z

bors r+

Closes #866 - Removes `fluvio-runner-local` as a dependency from `fluvio-cluster` - Renames `fluvio-runner-local` to `fluvio-run` and makes it a standalone executable - Adds `fluvio-run` as an independent plugin on packages.fluvio.io - Updates install.sh to include `fluvio-run` as a default plugin during installation

bors · 2021-04-13T21:00:28Z

Pull request successfully merged into master.

Build succeeded:

nicholastmosher changed the title ~~Decouple cluster components from Fluvio runner~~ Decouple cluster components from Fluvio cluster Apr 5, 2021

nicholastmosher marked this pull request as draft April 5, 2021 12:30

nicholastmosher force-pushed the cc-plugin branch 8 times, most recently from 51cbb63 to 7b3e606 Compare April 12, 2021 16:01

nicholastmosher added 18 commits April 12, 2021 16:33

Separate cluster components into independent plugin

eff0ab4

Rename extension-runner-local to runner

7315b2b

WIP

042228e

Use fluvio-run in test

d46b27f

Make DEFAULT_RUNNER_PATH a static, not const

33a7afd

Add fluvio-run to PATH before running smoke tests

98d9e3a

Update cluster-start-local to launch via fluvio run

e5344dd

Change package name to fluvio-run to match binary

97c2a13

Install fluvio-run plugin by default

14b926a

Update makefile and docker build script

15dbab1

WIP

27c93fa

Add .log() to CommandExt

26469b3

Set test PATH

ec57a69

Temp: Print SC and SPU logs to terminal

7f8da3d

Print path from fluvio

d0d7603

Ensure test runner uses test 'fluvio' for cluster setup

87d52bc

Fix entrypoint commands for cluster images

b05123d

Prioritize PATH before default dirs

4f2c3c8

nicholastmosher force-pushed the cc-plugin branch from 36c66b9 to 4f2c3c8 Compare April 12, 2021 20:41

nicholastmosher requested a review from sehz April 12, 2021 20:41

Publish latest fluvio-runner on merge to master

17495be

sehz requested changes Apr 12, 2021

View reviewed changes

src/cluster/src/delete.rs Show resolved Hide resolved

src/runner/Cargo.toml Outdated Show resolved Hide resolved

src/sc/build.rs Outdated Show resolved Hide resolved

src/cluster/Cargo.toml Show resolved Hide resolved

sehz mentioned this pull request Apr 13, 2021

Refactoring of SPU Replica management #958

Merged

Do not copy VERSION to SC since SC is not published

a6e28ae

nicholastmosher requested a review from sehz April 13, 2021 13:33

sehz force-pushed the master branch from c949216 to b9a1d86 Compare April 13, 2021 16:18

sehz requested changes Apr 13, 2021

View reviewed changes

src/command/src/lib.rs Show resolved Hide resolved

src/runner/Cargo.toml Outdated Show resolved Hide resolved

Bump versions

2ac7187

nicholastmosher requested a review from sehz April 13, 2021 16:38

nicholastmosher added 2 commits April 13, 2021 14:15

Make extra sure 'fluvio delete' cleans up local

5903cd1

Update DEVELOPER.md

5543418

Use unique pkill for local delete

b725112

sehz approved these changes Apr 13, 2021

View reviewed changes

Add back --keep-cluster for policy test

17a5e00

bors bot changed the title ~~Decouple cluster components from Fluvio cluster~~ [Merged by Bors] - Decouple cluster components from Fluvio cluster Apr 13, 2021

bors bot closed this Apr 13, 2021

nicholastmosher deleted the cc-plugin branch April 13, 2021 21:00

sehz mentioned this pull request Apr 16, 2021

Made fluvio-cluster an optional dependency #858

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Decouple cluster components from Fluvio cluster #928

[Merged by Bors] - Decouple cluster components from Fluvio cluster #928

nicholastmosher commented Apr 5, 2021 •

edited

nicholastmosher commented Apr 12, 2021

sehz left a comment

sehz commented Apr 13, 2021

sehz commented Apr 13, 2021

sehz commented Apr 13, 2021

sehz commented Apr 13, 2021

nicholastmosher commented Apr 13, 2021

sehz commented Apr 13, 2021

nicholastmosher commented Apr 13, 2021

nicholastmosher commented Apr 13, 2021

bors bot commented Apr 13, 2021

[Merged by Bors] - Decouple cluster components from Fluvio cluster #928

[Merged by Bors] - Decouple cluster components from Fluvio cluster #928

Conversation

nicholastmosher commented Apr 5, 2021 • edited

nicholastmosher commented Apr 12, 2021

sehz left a comment

Choose a reason for hiding this comment

sehz commented Apr 13, 2021

sehz commented Apr 13, 2021

sehz commented Apr 13, 2021

sehz commented Apr 13, 2021

nicholastmosher commented Apr 13, 2021

sehz commented Apr 13, 2021

nicholastmosher commented Apr 13, 2021

nicholastmosher commented Apr 13, 2021

bors bot commented Apr 13, 2021

nicholastmosher commented Apr 5, 2021 •

edited