Scrapligo as transport #737

iprouteget · 2021-12-28T22:14:38Z

This aims to replace the embedded SSH transport used for the config command with scrapligo's various network drivers (#668).

This might have been more overkill, but I thought it would be neat to be able to set various driver options directly in the yaml file (e.g. username, password, ssh config, private key file, etc). At the same time, I wanted to make sure only certain options (e.g. most everything here) were available.

I still need to test among the various platforms, but this seems to work on crpd so far:

topology:
  kinds:
    crpd:
      enforce-startup-config: true
      startup-config: ./startup.conf
      driver:
        ssh-config-file: <path to my ssh config>
        secondary: clab123
        port: 22

I was able to then use the following command to push generated configs from templates using:

sudo ~/test/containerlab/bin/containerlab config -t junos-routed-fabric.clab.yml template -p ./templates/ -l net-cfg

This is my first ever PR to an open source project, so I welcome any and all scrutiny!

hellt · 2021-12-28T22:33:43Z

Hey @iprouteget
Awesome initiative! Without looking at the code yet I have a few suggestions

Let's move what you named a 'driver' config under the 'config.transport.scrapli' struct.
The idea is to extend the existing config structure that currently has just Vars field with a transport container that will have configurations for various supported transports. Scrapli is one of them, but gnmi or scrapli-netconf will be the others

iprouteget · 2021-12-28T23:34:56Z

Thanks @hellt! It's funny, I actually tried doing something similar the first time. Just to be clear of your plan, the file would look something like (as an example):

  nodes:
    sp1:
      kind: crpd
      image: crpd:21.3R1.9
      config:
        vars:
          var1: <value>
          var2: <value>
        transport:
          scrapli:
            ssh-config-file: <file path>
            username: <username>
          gnmi:

Which could be populated by at the node level or a higher up (e.g. Kinds) level?

hellt · 2021-12-28T23:53:16Z

Yes, exactly like you showed in the snippet. The config structure follows the usual descendency chain: defaults -> kind -> node Meaning that for example you could have defined a transport for the whole crpd kind and it would have been inherited by all the crpd nodes. The most specific config wins

…

On Wed, 29 Dec 2021 at 00:35, Alex Kirby ***@***.***> wrote: Thanks @hellt <https://github.com/hellt>! It's funny, I actually tried doing something similar the first time. Just to be clear of your plan, the file would look something like (as an example): nodes: sp1: kind: crpd image: crpd:21.3R1.9 config: vars: var1: <value> var2: <value> transport: scrapli: ssh-config-file: <file path> username: <username> gnmi: Which could be populated by at the node level or a higher up (e.g. Kinds) level? — Reply to this email directly, view it on GitHub <#737 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABLKV5ITTAL2DO7K5A6G2FTUTJCSVANCNFSM5K43O5CA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

iprouteget · 2021-12-30T17:28:03Z

Sounds good. This latest push does the following:

Create a struct ConfigTransport that includes any and all transports that could be available, starting with Scrapli for now. This is moved within the ConfigDispatcher struct so we can unmarshal within the config higher level key.
Moves transport/driver.go file to transport.go. I think my intent is to allow containerlab to parse all specified transports (scrapli, gnmi, etc) and create the driver needed.
Updates the json schema to include scrapli and available options.

I mentioned in a comment about setting default values when parsing the yaml file. I don't think the way I did it for scrapli's default port is best; I thought setting a struct tag would work, but I am probably missing something easy here.

Examples of output:

Yaml file (only part to show the updates):

name: junos-ip-fabric
prefix: dc01

topology:
  kinds:
    crpd:
      enforce-startup-config: true
      startup-config: ./startup.conf
      config:
        transport:
          scrapli:
            secondary: clab123
            ssh-config-file: <ssh config path>
  nodes:
    sp1:
      kind: crpd
      image: crpd:21.3R1.9
      config:
        vars:
          loopback: 10.10.10.1
          asn: 65001
          bgp_speaker: true

Topology:

+---+------------------------------+--------------+-------------------------+-------+---------+-----------------+----------------------+
| # |             Name             | Container ID |          Image          | Kind  |  State  |  IPv4 Address   |     IPv6 Address     |
+---+------------------------------+--------------+-------------------------+-------+---------+-----------------+----------------------+
| 1 | dc01-junos-ip-fabric-batfish | 823b2b4699a7 | batfish/allinone:latest | linux | running | 172.20.20.5/24  | 2001:172:20:20::5/64 |
| 2 | dc01-junos-ip-fabric-host1   | 7bb4c078fefe | alpine:latest           | linux | running | 172.20.20.2/24  | 2001:172:20:20::2/64 |
| 3 | dc01-junos-ip-fabric-host2   | 5c3f9f9a3e44 | alpine:latest           | linux | running | 172.20.20.3/24  | 2001:172:20:20::3/64 |
| 4 | dc01-junos-ip-fabric-host3   | 5a6e6f5ed85f | alpine:latest           | linux | running | 172.20.20.6/24  | 2001:172:20:20::6/64 |
| 5 | dc01-junos-ip-fabric-lf1     | f97470c7ad11 | crpd:21.3R1.9           | crpd  | running | 172.20.20.10/24 | 2001:172:20:20::a/64 |
| 6 | dc01-junos-ip-fabric-lf2     | 13619033ad53 | crpd:21.3R1.9           | crpd  | running | 172.20.20.9/24  | 2001:172:20:20::9/64 |
| 7 | dc01-junos-ip-fabric-lf3     | 58032fb3e1fd | crpd:21.3R1.9           | crpd  | running | 172.20.20.4/24  | 2001:172:20:20::4/64 |
| 8 | dc01-junos-ip-fabric-sp1     | 74e720302203 | crpd:21.3R1.9           | crpd  | running | 172.20.20.8/24  | 2001:172:20:20::8/64 |
| 9 | dc01-junos-ip-fabric-sp2     | 132647a4b5e2 | crpd:21.3R1.9           | crpd  | running | 172.20.20.7/24  | 2001:172:20:20::7/64 |
+---+------------------------------+--------------+-------------------------+-------+---------+-----------------+----------------------+

cRPD nodes with startup config (prior to config push):

> ssh dc01-junos-ip-fabric-sp1
labuser@sp1> show configuration 
## Last commit: 2021-12-30 17:18:28 UTC by root
version 20210915.070446_builder.r1211817;
system {
    root-authentication {
        encrypted-password "$6$lB5c6$Zeud8c6IhCTE6hnZxXBl3ZMZTC2hOx9pxxYUWTHKW1oC32SATWLMH2EXarxWS5k685qMggUfFur1lq.o4p4cg1"; ## SECRET-DATA
    }
    login {
        user labuser {
            uid 2000;
            class super-user;
            authentication {
                ssh-ed25519 "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIEPYUrbObdyGUbfbdR6xkLarAY+EYiEBQTxMmiHT+rKj lab"; ## SECRET-DATA
            }
        }
    }
    services {
        ssh;
        netconf {
            ssh;
        }
    }
}

And after pushing the config:

> sudo ~/test/containerlab/bin/containerlab config -t junos-routed-fabric.clab.yml -p ./templates/ -l net-cfg
INFO[0000] Parsing & checking topology file: junos-routed-fabric.clab.yml 
WARN[0000] No template found for host3; skipping..      
WARN[0000] No template found for host1; skipping..      
WARN[0000] No template found for batfish; skipping..    
WARN[0000] No template found for host2; skipping..

cRPD node sp1 config afterward:

labuser@sp1> show configuration    
## Last commit: 2021-12-30 17:18:46 UTC by labuser
version 20210915.070446_builder.r1211817;
system {
    root-authentication {
        encrypted-password "$6$lB5c6$Zeud8c6IhCTE6hnZxXBl3ZMZTC2hOx9pxxYUWTHKW1oC32SATWLMH2EXarxWS5k685qMggUfFur1lq.o4p4cg1"; ## SECRET-DATA
    }
    login {
        user labuser {
            uid 2000;
            class super-user;
            authentication {
                ssh-ed25519 "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIEPYUrbObdyGUbfbdR6xkLarAY+EYiEBQTxMmiHT+rKj lab"; ## SECRET-DATA
            }
        }
    }
    services {
        ssh;
        netconf {
            ssh;
        }
    }
}
interfaces {
    eth1 {
        description to_lf1;
        unit 0 {
            family inet {
                address 100.64.10.1/30;
            }
        }
    }
    eth2 {
        description to_lf2;
        unit 0 {
            family inet {
                address 100.64.11.1/30;
            }
        }
    }
    eth3 {
        description to_lf3;
        unit 0 {
            family inet {
                address 100.64.12.1/30;
            }
        }
    }
    lo0 {
        unit 0 {
            family inet {
                address 10.10.10.1/32;
            }
        }
    }
}
policy-options {
    policy-statement fabric-in {
        then accept;                    
    }
    policy-statement fabric-out {
        term 1 {
            from protocol direct;
            then accept;
        }
        term 2 {
            then reject;
        }
    }
}
routing-options {
    router-id 10.10.10.1;
    autonomous-system 65001;
}
protocols {
    bgp {
        group v4_fabric {
            import fabric-in;
            export fabric-out;
            bfd-liveness-detection {
                minimum-interval 1000;
                multiplier 3;
            }
            neighbor 100.64.11.2 {
                description to_lf2;
                peer-as 65012;
            }
            neighbor 100.64.12.2 {
                description to_lf3;
                peer-as 65013;
            }
            neighbor 100.64.10.2 {
                description to_lf1;
                peer-as 65011;
            }
        }
        mtu-discovery;
        log-updown;
    }
}

hellt · 2022-01-12T15:49:27Z

Hi @iprouteget
a few things that I would like to see if you want to address

1 transport interface

The idea behind containerab' support of multiple transports lies within Go interfaces. There is a Transport interface defined here that we need to use.
This interface has three methods that define it, Connect, Write and Close. These methods should be implemented by any transport that containerlab is intended to support.

This requirement also stands true for the the scrapligo transport. Hence, we need to write define those methods for Scrapli transport and use them inside the Send function.

You may check how this was done for the embedded transport - https://github.com/iprouteget/containerlab/blob/scrapligo-as-transport/clab/config/transport/ssh.go#L234

2 Transport configuration

Currently you implemented the fallthrough logic of transport configuration. Namely, if the scrapli transport is defined on a node level it is returned. Otherwise the kind transport is used, and last resort is the default transport.
What would be cool is to merge the configuration of the transport structs from defaults all the way to the kind level.

For example, a user may want to define some kind specific params of the scrapli transport on the kind level and still want to fine tune it on the node level. To accomplish that use case, we need to merge default+kind+node parameters

iprouteget · 2022-01-13T23:04:48Z

Hi @hellt!

This makes sense, thank you for the feedback. I should be able to get some updates in over the weekend. I appreciate your patience!

kellerza · 2022-01-18T06:45:58Z

I'm not convinced the Transport Interface is the correct solution here, or put differently, it might not be complete in its current form:

While we can adapt the transport mechanism to connect to the router, we lack any form of transactions (send, commit, compare), and these transactions can be implemented completely differently based on the transport provider.

Taking the existing embedded ssh implementation as an example. It follows Connect, Session_start, Write, Session_end and then Close. Maybe session_start/end can use Transport's Write method (but a generic write is not flexible enough), and in some implementations there is already some transactional functionality built in (at least when I looked at Scrapligo in the past). The transaction and write according to me is too closely coupled to try separate them. I also had a use case to use custom ports, which required influencing the connect.

For all these cases I simply started using a method to send the config

const (
	ActionSend    = "send"
	ActionCompare = "compare"
	ActionCommit  = "commit"
)

func ConfigSend(c *config.NodeConfig, action string) ([]*transport.SSHReply, error) {

hellt · 2022-01-18T08:06:11Z

@kellerza
I think the transactionality (if that is offered by the transport) will be configured through the transport options.

Let's take the scrapligo example and the use cases our users might need

1 Configuring the device by sending commands

The most common use case, the goal is to get a device to a certain configuration state by sending and applying config commands as if they were entered over the CLI.

Scrapligo offers the following method - SendConfigs - which takes a slice of strings representing commands. This method will (in the background) acquire the configuration level privilege that is relevant to the specified NOS (like edit-config exclusive for SR OS) and will deliver the commands over the SSH channel.

This method does not imply any particular commit behaviour, as such users will have to add to the end of their configuration snippet something like commit now or similar, depending on what kind of commit sequence is relevant.

2 Sending commands which do not require configuration context. Aka `show` commands

In scrapligo this would be carried out with SendCommans which also takes a slice of commands to send over to the device, but this time the execution level privilege is acquired if needed to send show commands (think of enable on cisco platforms).

Now back to scrapligo transport interface implementation. In my view the default mode for scrapligo transport should be SendConfigs. If a user wants to send show commands than it will set the relevant transport option. I think what you did was to deduce if a file contains show commands by looking at the file template name.

kellerza · 2022-01-18T08:30:32Z

Now back to scrapligo transport interface implementation. In my view the default mode for scrapligo transport should be SendConfigs.

This sound more like we need multiple Transport implementations, one for SendConfigs and one for SendCommands.
Otherwise the action (send/commit/compare) is part of the transport options, which seems a bit counter intuitive, since its options for a higher layer, the transaction, rather than the transport.

If a user wants to send show commands than it will set the relevant transport option. I think what you did was to deduce if a file contains show commands by looking at the file template name

Indeed in the very first iteration, but we have CLI to select the action since a while already (see below)

$ clab config --help

configure a lab based on templates and variables from the topology definition file
reference: https://containerlab.srlinux.dev/cmd/config/

Usage:
  containerlab config [flags]
  containerlab config [command]

Aliases:
  config, conf

Available Commands:
  compare     compare configuration to a running lab
  send        send raw configuration to a lab
  template    render a template

Flags:
  -p, --template-path strings   comma separated list of paths to search for templates
  -l, --template-list strings   comma separated list of template names to render
  -f, --filter strings          comma separated list of nodes to include
  -h, --help                    help for config

btw, in my view even commit/SendConfigs should not be default, but rather through an explicit commit on the CLI. Once you work with compare & commit during template development, this is a big usability enhancement and prevents you from accidentally comitting.

iprouteget · 2022-01-18T16:23:28Z

Hi all, just catching up here.

I'm still working on merging the transport structs for default/kind/node, but I wanted to submit the first item regarding the move to the Transport interface.

This cleaned up send.go quite a bit. Based on these more recent notes, perhaps I need to look at adding some transport options to incorporate a broader range of functions instead of only committing the configuration. Currently, I'm just using scrapligo's cfg driver, which seems to work for pushing the configurations.

…igo-as-transport

iprouteget · 2022-04-25T02:14:25Z

Phew, it's been a hot minute since I've been able to look at this again. From the notes provided earlier, I tried moving the scrapli code to use the existing transport interface, which seems to be working so far. @hellt, regarding the merging of structs, I just copied the example you provided in discord since it was lightweight and straightforward. I figured once there are more complex types, we'd start testing more mergo?

I still see some things in my code that probably need refactoring, but I want to make sure I'm on the correct path regarding the two main items mentioned.

hellt · 2022-04-25T08:28:49Z

Hey @iprouteget
thanks for getting back to it
your hot minute was just in time, as we had different things to squash in the meantime.
I will review that in coming weeks!

iprouteget · 2022-04-25T14:22:40Z

Awesome! Much appreciated. I'll add some tests this week as well.

hellt · 2022-05-04T07:04:47Z

@iprouteget do you mind checking if you can port some cleanup changes I started to do in #734 with regards to config engine cleanup?

Then I could retire that PR

iprouteget · 2022-05-04T19:20:28Z

Will do!

iprouteget · 2022-05-05T17:16:01Z

Okay I believe I have the cleanup items from #734, and I also added a test for the MergeStructConfigs function.

hellt · 2022-07-07T14:58:35Z

Hi @iprouteget
sorry this still drags... There was some house keeping work done that is related to that PR as well

scrapilgo was updated to v1, and this has to be addressed here. Here is a PR that updated other parts of containerlab with scrapligo update update to accomodate changes in scrapligo v1.0.0 #925

iprouteget · 2022-07-07T22:36:58Z

Hi @hellt, all good! I'll update this accordingly.

iprouteget · 2022-08-02T18:03:56Z

@hellt with the amount of time that has passed and the updates to scrapligo, I feel like it would make more sense to close this out and start fresh :) It might allow for breaking up some parts into smaller PRs. Let me know if this works best. Thanks!

hellt · 2022-08-02T18:18:14Z

That makes a alot of sense to me indeed

…

On Tue, 2 Aug 2022 at 20:04, Alex Kirby ***@***.***> wrote: @hellt <https://github.com/hellt> with the amount of time that has passed and the updates to scrapligo, I feel like it would make more sense to close this out and start fresh :) It might allow for breaking up some parts into smaller PRs. Let me know if this works best. Thanks! — Reply to this email directly, view it on GitHub <#737 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABLKV5MAGZYS7KIEZNXXB6TVXFPJNANCNFSM5K43O5CA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Alex Kirby added 3 commits December 28, 2021 22:01

Adding new driver type for config

11fd2fb

Adding driver struct and updating schema

b536d15

Go modules

85e7c00

Moving scrapli transport to config engine

0ae5cfc

hellt added 4 commits January 12, 2022 16:01

bring back transport interface

335d45a

renamed name to host for scrapli factory func

007ec56

change empty string test

6bf7352

rename scraplitransport struct and removed default tags

f433444

Converting scrapli into Transport interface

fcbaa7d

Renaming scrapli struct

6b05a13

Alex Kirby and others added 4 commits April 24, 2022 17:48

Merge branch 'main' of github.com:iprouteget/containerlab into scrapl…

be484c1

…igo-as-transport

Added struct merge function

f094a34

Merge branch 'main' into scrapligo-as-transport

62236df

Cleaning up

d884b78

Shore up draft srl-labs#734

ec72d0e

Adding test for merge struct function

2eb9c32

iprouteget closed this Aug 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scrapligo as transport #737

Scrapligo as transport #737

iprouteget commented Dec 28, 2021

hellt commented Dec 28, 2021

iprouteget commented Dec 28, 2021

hellt commented Dec 28, 2021 via email

iprouteget commented Dec 30, 2021 •

edited

Loading

hellt commented Jan 12, 2022

iprouteget commented Jan 13, 2022

kellerza commented Jan 18, 2022

hellt commented Jan 18, 2022

kellerza commented Jan 18, 2022

iprouteget commented Jan 18, 2022

iprouteget commented Apr 25, 2022

hellt commented Apr 25, 2022

iprouteget commented Apr 25, 2022

hellt commented May 4, 2022

iprouteget commented May 4, 2022

iprouteget commented May 5, 2022

hellt commented Jul 7, 2022

iprouteget commented Jul 7, 2022

iprouteget commented Aug 2, 2022

hellt commented Aug 2, 2022 via email

Scrapligo as transport #737

Scrapligo as transport #737

Conversation

iprouteget commented Dec 28, 2021

hellt commented Dec 28, 2021

iprouteget commented Dec 28, 2021

hellt commented Dec 28, 2021 via email

iprouteget commented Dec 30, 2021 • edited Loading

hellt commented Jan 12, 2022

1 transport interface

2 Transport configuration

iprouteget commented Jan 13, 2022

kellerza commented Jan 18, 2022

hellt commented Jan 18, 2022

1 Configuring the device by sending commands

2 Sending commands which do not require configuration context. Aka show commands

kellerza commented Jan 18, 2022

iprouteget commented Jan 18, 2022

iprouteget commented Apr 25, 2022

hellt commented Apr 25, 2022

iprouteget commented Apr 25, 2022

hellt commented May 4, 2022

iprouteget commented May 4, 2022

iprouteget commented May 5, 2022

hellt commented Jul 7, 2022

iprouteget commented Jul 7, 2022

iprouteget commented Aug 2, 2022

hellt commented Aug 2, 2022 via email

iprouteget commented Dec 30, 2021 •

edited

Loading

2 Sending commands which do not require configuration context. Aka `show` commands