Create cli command

[gianarb]

Currently, the CLI does not have a common way to create a new resource. Hardware gets pushed, template and workflows get created for example.

Even between templates and workflows, the features are different. You can create a template via a file with --path but not a workflow.

It is time to have a consistently create command as we have for delete #425 and get.

---

Input channel

The create command should accept a --path:

tink hardware create --path ./hardware.json

--path needs to be a file. If it is a directory for now you can fail to say that directory is not supported for now.

Or it can get input from stdin:

cat ./template.yaml | tink template create - 

Input format

Things can get messy (because they are messy already). Hardware is the main issue I presume and we know why #359. Personally and when looking at examples I visualize in my mind hardware like JSON. Template and workflows as YAML.

My current suggestion is to give up, for now, let's derive the input format from the --path, if the file is .json let's unmarshal with JSON. If it is .yaml let's use YAML.

In case of input via stdin let's do cascade, let's start with JSON, fallback to YAML. if nothing works it is a not valid input format.

We only support YAML and JSON.

Output

The output is the same as the get command but with only the created resources (reuse the same code please). It means that create should support:

--format

As a way to select the output format (and this makes clear why kubectl uses --output and not --format, because it makes clear that --output manipulate the output format... --format here opens the question: "is it for input or for output?" @mmlb )

[mmlb]

I'd rather not see 2 different forms of specifying paths, normal - is a unix short hand for stdin where ever a filepath is expected. For example:

man cat
...
EXAMPLES
       cat f - g
              Output f's contents, then standard input, then g's contents.

also see man curl for "-", there are many others :D.

So I'd rather see tink template create path, if path is -` read from stdin. If path is not given read from stdin (if stdin is a tty print a "reading from stdin" message to stderr). Otherwise if path is given then read that.

[mmlb]

Even between templates and workflows, the features are different. You can create a template via a file with --path but not a workflow.

This is currently working as designed right? Workflows are only ever meant to be created from a preexisting template so it doesn't make sense create a workflow based off of a file.

Input format

Things can get messy (because they are messy already). Hardware is the main issue I presume and we know why #359. Personally and when looking at examples I visualize in my mind hardware like JSON. Template and workflows as YAML.

My current suggestion is to give up, for now, let's derive the input format from the --path, if the file is .json let's unmarshal with JSON. If it is .yaml let's use YAML.

In case of input via stdin let's do cascade, let's start with JSON, fallback to YAML. if nothing works it is a not valid input format.

We only support YAML and JSON.

Well yaml is a superset of JSON so we can probably just parse the json with yaml reader :D. I tried the following and it worked:

[~/t/go]─[manny@dellnix]> 
go run t.go yaml
(map[string]interface {}) (len=3) {
 (string) (len=3) "foo": (bool) true,
 (string) (len=3) "bar": (int) 1,
 (string) (len=9) "something": ([]interface {}) (len=2 cap=2) {
  (int) 1,
  (int) 2
 }
}
                                                                                                                                                                                      
[~/t/go]─[manny@dellnix]> 
go run t.go json
(map[string]interface {}) (len=3) {
 (string) (len=3) "bar": (int) 1,
 (string) (len=9) "something": ([]interface {}) (len=2 cap=2) {
  (int) 1,
  (int) 2
 },
 (string) (len=3) "foo": (bool) true
}

package main

import (
        "os"

        "github.com/davecgh/go-spew/spew"
        "gopkg.in/yaml.v2"
)

func main() {
        var content []byte
        switch os.Args[1] {
        case "json":
                content = []byte(`{"foo":true,"bar":1,"something":[1,2]}`)
        case "yaml":
                content = []byte(`
foo:
  true
bar: 1
something:
  - 1
  - 2
`)
        }
        var m map[string]interface{}
        yaml.Unmarshal(content, &m)
        spew.Dump(m)
}

[gianarb]

This is currently working as designed right? Workflows are only ever meant to be created from a preexisting template so it doesn't make sense create a workflow based off of a file.

You should be able to describe workflows as a file like:

name: bla
templateID: bla-uuid-bla
data: maybe

[parauliya]

This is currently working as designed right? Workflows are only ever meant to be created from a preexisting template so it doesn't make sense create a workflow based off of a file.

Yes @mmlb , I agree with you here because Workflows are nothing but a template for a designated hardware and so it is created on run time and fetch the required template during creation.

[parauliya]

You should be able to describe workflows as a file like:

name: bla
templateID: bla-uuid-bla
data: maybe

@gianarb , For now workflow required only templateID and device which is targeted hardware. Also we do not define the workflow with it's name because it is being created from a template which already has a name. I don't see a requirement of a file until we decide to have some more parameters which are workflow specific. If you have such use cases in mind please elaborate them here.

[gianarb]

@parauliya @mmlb I think you are confusing declarative vs imperative. And we do not know what we are (not true https://tinkerbell.org/ this site says "declarative")...

Random link from google: https://tech.ovoenergy.com/imperative-vs-declarative/

So, here the question: "do we want to implement declarative and imperative at the same time or do we want to take a step back and do only one?"

I think declarative is easier in the creation phase, we say: "pass a file that represents the structure you want to save and we serialize, validate, and save it."

Declarative or imperative does not depend on resources. If we go with declarative we are declarative even with workflows. (workflow can have multiple values based on how many data a template requires I suppose)

if we want to do a mix, we should define that mix or we will end up with today's inconsistency (or something worst).

[parauliya]

@gianarb , I agree with your point that we should have declarative or imperative or both for all the resources. Right now for hardware and template creation we required a file which is declarative and for creation of workflows we have imperative approach/command since it does not need a file atleast for now. But as you correctly said we would like to execute a workflow with multiple workers in that case there would be multiple variable need to be passed in the imperative command. In that case having a file would be a good option I mean declarative. @mmlb Could you please provide your opinion as well and think if we are heading in the right direction?.

[mmlb]

@gianarb interesting, I didn't think about the workflow runs being declarative. I understand why you want to do it but idk it feels a little forced or taking it too far? Also tink create is by definition imperative so if we wanted a fully declarative setup I think we'd should go with declare or some other verb instead. And if we did go with the declarative command I'd think it be best to do it all in one go. But really I think that maybe that is just better off doing a terraform provider instead.

[gianarb]

You are right! So probably we should say: "tink create is imperative" so we have to figure out flags and things like that. Right?

[mmlb]

Yep, I think hardware and template can most likely share the same args, worfklow will have its own. Maybe we want other subcommand to start a workflow? run?

[gianarb]

Spoke with @mmlb for now we are gonna stay with create, and we are not ready to rename workflows to something else. So... Let's recap:

Create is an imperative command, which means that we need the ability to declare flags per resource. @parauliya can you analyze and write a comment with the various flag we should have per resource? And when we agree with that you can start the actual implementation.

Thanks

[parauliya]

@gianarb @mmlb ,

Following are the command and flags for each resource for Create cli:

1. Hardware: For hardware there can be two ways: 1.1. Provide a file which contain hardware data in json format using --file flag:

        tink hardware create --file <path to the file which contains hardware data in json format>
   

   1.2. Provide the data of the file as an input through stdin as follows:

       cat <hardware data file> | tink hardware create
       OR
       tink hardware create < <path to a file which contains hardware data in json format>
   

2. Template For tempate also there can be two ways same as hardware: 2.1. Provide a file which contain template data in yaml format using --file flag:

        tink template create --file <path to the file which contains template data in json format>
   

   2.2. Provide the data of the file as an input through stdin as follows:

       cat <template data file> | tink template create
       OR
       tink template create < <path to a file which contains template data in yml format>
   

3. Workflow For workflow creation two flags are required one for providing template id and one for providing the mac/ip of the targeted hardware:

      tink workflow create --template <template id> --hardware '{"device_1" : "00:99:88:77:66:54", "device_2" : "00:11:22:33:44:55"}'   
   OR we can use the short hands of flags which can be `-t` for `--template` and `-h` for `--hardware` 
      tink workflow create -t <template id> -d '{"device_1" : "00:99:88:77:66:54", "device_2" : "00:11:22:33:44:55"}'  
   

Please let me know if this seems ok to you so I can start implementation.

[gianarb]

Thanks @parauliya ! Great job summarizing what we have today. I think it is a good start. I would like to know if we can do better than what we have today. Those are my concerns:

1. Some create command take JSON (like hardware)
2. Other takes YAML (Template)
3. Others take random strings that looks like JSON (workflow -d)

How can we make it better? I pinged @mmlb and @thebsdbox privately but I think we need more people to help us with this: @invidian @detiber @displague !!

[thebsdbox]

(For workflows)

I would suggest we leverage the something like the StringArray feature of cobra and have a repeating flag. That would remove the need for yaml/json/ini/xml etc..

The end result would look like:

 tink workflow create --template <template id> \
  --device dev0,00:11:22:33:44:55 \
  --device dev1,11:22:33:44:55:66

We'd iterate over the string array from the devices flag and then do a strings.Split(devices[x], ",") to find both device and Mac address

[invidian]

Well yaml is a superset of JSON so we can probably just parse the json with yaml reader :D. I tried the following and it worked:

:+1: Similar to what K8s is doing.

We'd iterate over the string array from the devices flag and then do a strings.Split(devices[x], ",") to find both device and Mac address

Generally speaking, I'd expect CLI to do the same what "frontend" library offers, so CLI is very tiny wrapper over something better testable. If it allows you to create multiple workflows with single call, I think the benefit of it should be justified, as right now the complexity is obvious with anonymous format (dev1,11:22:33:44:55:66), but the benefit is vague.

If you consume YAML bytes, do note, that one may provide multiple documents in a single output, separated by ---. Supporting such things would also be nice.

tink template create --file

If create takes no other arguments, --file seems not needed.

tink template create --file vs tink template create is already inconsistent proposal IMO.

[mmlb]

I like most of this, just a few thoughts:

• hardware input is json, but json being a subset of yaml we can just do yaml too. Switching from encoding/json to yaml should work right now with json input. Then we can start to transition docs to show yaml format instead of json.
• Do we even need a --path/--file? We can easily support ... create (-|/some/file/path|). create -: reads from stdin create /some/file/path: reads from path create: reads from stdin, if stdout is a tty print a message to stderr to the effect of "reading from stdin"
• Should be --template-id instead of just template to be obvious its not a template yaml.
• Should not be --device since it can be any template value that the user wants to fill in, could be userdata or w/e too. Maybe --params, --data, --values?

[gianarb]

Generally speaking, I'd expect CLI to do the same what "frontend" library offers, so CLI is very tiny wrapper over something better testable.

This is maybe true, ideally. But we are not there and this is an opportunity to set the stage for how we want to interact with the system and how the system will look in the near future. it means that maybe it will diverge a bit from the actual client/api but it will converge if the UX is good.

[splaspood]

Looks like there have been some good points made by a number of people back in March. What would it take for us to come to consensus around these changes?

[jacobweinstock]

closed by https://github.com/tinkerbell/tink/pull/654