Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use plugins to manage nodes #62

Closed
travisperson opened this issue Jul 17, 2018 · 12 comments
Closed

Use plugins to manage nodes #62

travisperson opened this issue Jul 17, 2018 · 12 comments
Labels
help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature

Comments

@travisperson
Copy link
Member

Currently iptb isn’t very flexible, and can be cumbersome to add to and extend. There are a lot of ipfs and go-ipfs specific details to the way it handles managing nodes.

I think it would be great to make iptb a little more generic in the way that it handles the construction of nodes, such that, it can be used to manage anything that fits into an interface. This will most of the time, and be design with in mind, libp2p services.

Adding different node types (ipfs), deployments (local, docker, browser, k8s, remote), and implementations (go, js, rust) should be easy. By exposing a single node interface that works with any libp2p service (has a swarm, peerid, etc), we can use golang plugins to allow anyone to create their own node implementation (private or public) that works with iptb.

This will turn iptb into a more generic tool that can be used to help anyone building a libp2p based service that fits into our interface, helping grow the community.

It's important that we maintain feature parity. Commands may change, but specifying additional options to help configure nodes is important.

The first new additions to iptb will most likely be jsipfs, which needs to run as a daemon as well as in a browser. New kinds of deployments may require additions to the Node interface, but keeping the Node interface small is important.

One of the first steps towards this goal would be to move all ipfs / go-ipfs specifics outside of util/util.go and replace them with methods on the Node interface.

@frrist and I have done a bit of work to showcase what this might look like in #61. It's a bit stripped, but I will be bringing it back up to feature parity soon.

@travisperson travisperson added kind/enhancement A net-new feature or improvement to an existing feature help wanted Seeking public contribution on this issue labels Jul 17, 2018
@travisperson
Copy link
Member Author

travisperson commented Jul 24, 2018

With this change I think it might be appropriate to also take some time to address the cli itself. The interface / plugin changes require some breaking changes to the cli, mostly around attrs (get, set) as it needs to take a node, and a lot of new commands (metrics, nodespec stuff).

Some information on how to read the usages

  • <arg> - required
  • [arg] - optional
  • [node], <node> - Single node by index
  • [nodes], <nodes> - Single node, or range ([m-n])

Arguments after a -- are pass-through (not parsed by iptb), examples when used with a nodespec for ipfs. This helps address #55, as you can just pass along arguments for init.

  • iptb start [0-9] -- --debug => ipfs daemon --debug
  • iptb init [0-3] -- -f --profile foo => ipfs init -f --profile foo
  • iptb init [4-9] -- -f --profile bar => ipfs init -f --profile bar
  • iptb run 0 -- ipfs --foo --bar id => ipfs --foo --bar id

In #61 @whyrusleeping asked me to incorporate some new commands from #60.

New commands were create-nodespec and init-nodespec, which basically broke apart init so create nodespecs, and initialize them separately. With these cli changes I've moved them around a bit.

  • create-nodespec is create
  • init-nodespec is init
  • old init is create --init

Attributes help shape process execution, though they are kind of a catch all to configure plugins (ex docker image). They can be specified as key,value pairs on create (iptb create -n 10 -type ipfsdocker -attr ram,512M -attr ws), or set via iptb attr.

Plugins themselves determine what attributes they can use and how to use them. The iptb attr commands pass directly through to the node while running, or read from the nodespec otherwise.

https://gist.github.com/travisperson/de8c8ca6abd09cd74f41a04dc2a19fbe

@whyrusleeping
Copy link
Member

Agree with all of this except the breaking of iptb init. I think we should strive to keep it the same as it is now, for the sake of backwards compatibility.

If others are fine with breaking that command, then I could be persuaded. cc @Stebalien @magik6k @dignifiedquire @frrist

@travisperson
Copy link
Member Author

travisperson commented Jul 24, 2018

Additional Note:

There is a new global flag --nodespec, which was added to make it so you could name / create different nodespecs in the same IPTB_ROOT. This is mostly important with regard to plugins. We will need a place to store and load plugins for a user. I think a IPTB_ROOT structure like the following will help with that.

.
├── nodespecs
│   └── default
│       ├── 0
│       ├── 1
│       ├── 2
│       ├── 3
│       ├── 4
│       └── nodespec
└── plugins
    ├── ipfsdocker.so
    └── ipfslocal.so

@whyrusleeping
Copy link
Member

On the 'init' command, i guess since we're breaking a bunch of commands already, splitting init into two commands isnt that bad.

On the IPTB_ROOT structure, what does nodespecs/default/0 represent?

@travisperson
Copy link
Member Author

travisperson commented Jul 24, 2018

nodespecs/default is similar to the current IPTB_ROOT structure, so nodespecs/default/0 is the directory that node 0 can use for whatever (storing repo, pid, std* files, etc). Mainly this is to help with forced initialization nodespec creation (can just remove nodespecs/default), and helping preserve plugins after.

@whyrusleeping
Copy link
Member

Ah, cool. Makes sense.

@dignifiedquire
Copy link
Member

I like the new init + create commands, though to be honest they are very similar words and at least as a non native speaker I would have trouble knowing which one to call first, and which one to call second. So maybe we could have sth that is a bit more distinctive?

@frrist
Copy link
Member

frrist commented Jul 24, 2018

Since we are in the spirit of breaking stuff, I propose we rename a few more things:

Interplanetary Testbed => Interplanetary Testbench
nodespecs => testbenches

The commands would then look something like:

  1. iptb bench - parent command to manipulate a testbench directory
    • create - will create a directory under testbenchs
      • e.g. iptb bench create -name dockerBench -c 3 -type ipfsdocker -attr ram,512M
    • ls - will list all benches
      • e.g. iptb bench list
        name: default, count: 5
        name: dockerBench, count: 3
        (could also consider having metadata about each node in the bench, omitted for simplicity here)
  2. iptb init - runs the dameons initialization command
  3. iptb start - runs the daemons start command

Basically create becomes a subcommand of bench - bench is used to manipulate and view testbench directories.

.
├── testbenches
│   └── default
│       ├── 0
│       ├── 1
│       ├── 2
│       ├── 3
│       ├── 4
│       └── nodespec
|  └── dockerBench
│       ├── 0
│       ├── 1
│       ├── 2
│       └── nodespec
└── plugins
    ├── ipfsdocker.so
    └── ipfslocal.so

@dignifiedquire does this make things clearer?

@whyrusleeping
Copy link
Member

@frrist That seems pretty nice, though I always prefer a 'fast path' default to get things working quickly. iptb init -n 5 is really fast and easy, and I think that is critically important. More expressive options are always great, but don't kill the simple :)

@travisperson
Copy link
Member Author

We could create another fast path as we've taken away init as it maps directly to core#init, and it would be nice to keep it exposed. If we are just going after a top level command, we could do something like

  • iptb go -n 5
  • iptb boot -n 5

It would basically be an alias to bench create --init --name default (under @frrist proposal), note that --name would be optional in the create command, as if not specified default is used, and all commands run against the default bench if no bench name is specified.

iptb start => default bench
iptb --bench foo start => foo bench.

This means that iptb [fast] -n 5 would setup a bench so that iptb start would start those 5 news nodes.

@travisperson
Copy link
Member Author

https://asciinema.org/a/xoEoUo9QjDP3saVVXRyHYu4hN

(note: I've defaulted type to localipfs as it's a built in atm)

@frrist
Copy link
Member

frrist commented Sep 10, 2018

Addressed in #61

@frrist frrist closed this as completed Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants