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

Can config management really not be implemented? #71

Closed
adam-edwards opened this issue Mar 13, 2019 · 26 comments
Closed

Can config management really not be implemented? #71

adam-edwards opened this issue Mar 13, 2019 · 26 comments
Labels

Comments

@adam-edwards
Copy link

adam-edwards commented Mar 13, 2019

In the README, you make the statement:

You provide a plain text config file and replace running config with that. MikroTik does not have this.

Is this really accurate? You can run /export file=<filename> as well as /import file-name=<filename> to export/import the config and modify it in file format. Is there a technical reason that this is not a workable solution? I know you mention that order matters in some places and not others. Is this the issue? And if so, is there not a way around this, such as generating the file from scratch each time? The OS seems to handle references fairly well in the export by using [find ...] rather than referencing objects by number. So is it just that this is a PITA to implement, or is it somehow not possible because of how the plugin interface works?

@luqasz
Copy link
Collaborator

luqasz commented Mar 13, 2019

/export file=<filename> produces a lot of scripting "macro"

This is a trap. Once I (or any one else) implement this, I will have tons of maintenance with escaping mikrotik scripting language. MikroTik may change without prior information (which they did this in the past breaking backwards compatibility) syntax. It is just a PITA.

Uniqueness in some command paths.

Two options here. User provides tuple with keys which form a unique key, or provide those in library. Again thing may change.

No commit/rollback.

This is self explanatory. It can be emulated in plugin BUT when access is cut off in middle of configuration, no way to roll back. API does not provide safe mode access. Neither you can control timeouts via cli.

With all those points, I can implement a simple yaml / json file parsing with rules and do "config replace". One thing for sure. I need feedback and not everything can be done (see points above)

@adam-edwards
Copy link
Author

Makes sense - thanks for clarifying. So basically, whereas the commands in a Cisco or Juniper config file represent a desired configuration state, the commands in the RouterOS represent actions. Or in other words, there's not necessarily an idempotent config syntax.

@luqasz
Copy link
Collaborator

luqasz commented Mar 13, 2019

Makes sense - thanks for clarifying. So basically, whereas the commands in a Cisco or Juniper config file represent a desired configuration state, the commands in the RouterOS represent actions. Or in other words, there's not necessarily an idempotent config syntax.

Yes. Exactly.

@PatrickstarWritesCode
Copy link

PatrickstarWritesCode commented Sep 18, 2019

Makes sense - thanks for clarifying. So basically, whereas the commands in a Cisco or Juniper config file represent a desired configuration state, the commands in the RouterOS represent actions. Or in other words, there's not necessarily an idempotent config syntax.

Yes. Exactly.

Technically, there is a workaround but it requires a reboot of the device.
/system reset-configuration run-after-reset=new_config.rsc

@luqasz
Copy link
Collaborator

luqasz commented Sep 18, 2019

Technically, there is a workaround but it requires a reboot of the device.
/system reset-configuration run-after-reset=new_config.rsc

Hmm. Didn't know that. Thx for information. IMO this is crude but acceptable.

@PatrickstarWritesCode
Copy link

PatrickstarWritesCode commented Sep 19, 2019

Hmm. Didn't know that. Thx for information. IMO this is crude but acceptable.

You're welcome, I'm glad to help. Another caveat is that you need to upload the rsc file to the device before issuing the command which I don't know if it is possible through the API. As you mentioned this is indeed crude and not justifiable on edge/core devices but acceptable on leaf nodes.

@luqasz
Copy link
Collaborator

luqasz commented Sep 19, 2019

Yes. I agree. Since there is no other way, end user must decide if reboot is ok for his device.

@luqasz luqasz mentioned this issue Apr 19, 2020
@adamcharnock
Copy link

I've just come across this while considering a deployment of Netbox + NAPALM ROS. Did anything ever come of the discussion with @adamharm in #40 regarding creating configuration merge system?

I ask because I already have something tentatively working in this regard, and I'd be happy for it to be used here. I may also be up for submitting the PR myself (I'm not 100% sure I'm going to go ahead with Netbox yet).

The code I have is about 800 lines of fairly well structure Python, plus unit tests. It will take two RouterOS configurations and produce a diff. When the diff is executed, it will move the router config from the old state to the new state. It is smart enough to maintain the order of ordered sections (i.e. firewall config), and can identify configuration expressions by their unique keys (i.e. name for most things, or address for /ip address etc). Where that isn't possible, unique IDs can be placed in the comment field.

That all being said, it isn't perfect. It is the kind of thing that works well within limitations. Certainly if you stick to the output of /export it should be fine. But if you start to do anything fancy it will struggle more.

Examples

Simple:

# Old:
/routing ospf instance
add name=core router-id=100.127.0.1

# New:
/routing ospf instance
add name=core router-id=100.127.0.99

# Diff:
/routing ospf instance
set [ find name=core ] router-id=100.127.0.99

Firewall NAT rules:

# Old:
/ip firewall nat 
add chain=a comment="Example text [ ID:1 ]"
add chain=c comment="[ ID:3 ]"

# New:
/ip firewall nat 
add chain=a comment="Example text [ ID:1 ]"
add chain=b comment="[ ID:2 ]"
add chain=c comment="[ ID:3 ]"

# Diff:
/ip firewall nat 
add chain=b comment="[ ID:2 ]" place-before=[ find where comment~ID:3 ]

If this would be useful let me know.

@dBitech
Copy link

dBitech commented Mar 6, 2021

I've just come across this while considering a deployment of Netbox + NAPALM ROS. Did anything ever come of the discussion with @adamharm in #40 regarding creating configuration merge system?

I ask because I already have something tentatively working in this regard, and I'd be happy for it to be used here. I may also be up for submitting the PR myself (I'm not 100% sure I'm going to go ahead with Netbox yet).

The code I have is about 800 lines of fairly well structure Python, plus unit tests. It will take two RouterOS configurations and produce a diff. When the diff is executed, it will move the router config from the old state to the new state. It is smart enough to maintain the order of ordered sections (i.e. firewall config), and can identify configuration expressions by their unique keys (i.e. name for most things, or address for /ip address etc). Where that isn't possible, unique IDs can be placed in the comment field.

That all being said, it isn't perfect. It is the kind of thing that works well within limitations. Certainly if you stick to the output of /export it should be fine. But if you start to do anything fancy it will struggle more.

Examples

Simple:

# Old:
/routing ospf instance
add name=core router-id=100.127.0.1

# New:
/routing ospf instance
add name=core router-id=100.127.0.99

# Diff:
/routing ospf instance
set [ find name=core ] router-id=100.127.0.99

Firewall NAT rules:

# Old:
/ip firewall nat 
add chain=a comment="Example text [ ID:1 ]"
add chain=c comment="[ ID:3 ]"

# New:
/ip firewall nat 
add chain=a comment="Example text [ ID:1 ]"
add chain=b comment="[ ID:2 ]"
add chain=c comment="[ ID:3 ]"

# Diff:
/ip firewall nat 
add chain=b comment="[ ID:2 ]" place-before=[ find where comment~ID:3 ]

If this would be useful let me know.

Even if it is not useful to anyone else, I have a use case where this would be a perfect solution

@adamcharnock
Copy link

Hey @dBitech, I'm glad to hear it. Ok, I've just spun it up into its own little project:

https://github.com/gardunha/routeros-diff

You'll probably run into some quirks, but feel free to raise issues / send PRs. I took some time to refactor & comment the code before releasing.


Additionally, I'd be happy for napalm-ros to pull in the above project as a dependency (and indeed, I may even do a PR myself as I mentioned). I released it as a separate project as I figured it has use outside of napalm-ros.

@luqasz
Copy link
Collaborator

luqasz commented Mar 7, 2021

Hi. @adamcharnock Thx for your input. Your solution seems good enough. I have couple of questions though.

  1. Is your program able to produce a replace commands ?
  2. How end user can provide NATURAL_KEYS ? Things change in RouterOS (sometimes even without prior notice) so keys also may change.

@adamcharnock
Copy link

That is good to hear @luqasz. Answers below:

Is your program able to produce a replace commands ?

Can you clarify this, I'm not sure I completely understand.

How end user can provide NATURAL_KEYS ? Things change in RouterOS (sometimes even without prior notice) so keys also may change.

An excellent point. I have added settings and released version 0.2.0.

@luqasz
Copy link
Collaborator

luqasz commented Mar 10, 2021

We are interested in replacing and merging config. Napalm provides methods for that.

E.g if you want to add a ip address to a list and you don't care / don't want to touch other entries then, you will use a merge option.

If you want to have exactly what you say in config file (later to be parsed), then you want a replace method.

@luqasz
Copy link
Collaborator

luqasz commented Mar 10, 2021

As for NATURAL_KEYS, we still need some way to add it to parsable config.

@adamcharnock
Copy link

adamcharnock commented Mar 10, 2021

Hi @luqasz,

Ok, I see what you are saying. routeros-diff will currently only provide you with a replace method. An implementation of Napalm's load_replace_candidate() would work as follows:

  • Accept the destination config as a parameter
  • Fetch the current config from the device
  • Run routeros-diff
  • Upload the patch script to the device
  • Execute the patch script

I have not looked into merging yet. I suspect merging could be defined as some combination of the following:

  • Don't touch sections which are missing in the destination config
  • Don't remove any entries unless explicitly removed in the destination config
  • Otherwise just operate as normal

I don't think this would be too hard to implement in routeros-diff


As for NATURAL_KEYS, we still need some way to add it to parsable config.

Ok, this is the parsable config system provided by napalm-ros? I haven't looked into that yet.

@luqasz
Copy link
Collaborator

luqasz commented Mar 15, 2021

Don't touch sections which are missing in the destination config

Destination config is a file that we want to apply ?

Ok, this is the parsable config system provided by napalm-ros? I haven't looked into that yet.

Nope. napalm-ros doesn't parse any external files.

@adamcharnock
Copy link

adamcharnock commented Mar 15, 2021

Destination config is a file that we want to apply ?

Correct. I'm using this terminology:

  • Old / source - the config we are coming from (i.e. the config on the router)
  • New / destination - the config we wish to be on the router

As for NATURAL_KEYS, we still need some way to add it to parsable config.

Ok, this is the parsable config system provided by napalm-ros? I haven't looked into that yet.

Nope. napalm-ros doesn't parse any external files.

Ok, in that case, can you clarify your original meaning?

@adamcharnock
Copy link

It seems we are bouncing back and forth a bit here. The questions on my mind are:

  • Is routeros-diff suitable for use in implementing a solution to this issue? If not, what would you like to see changed?
  • Are there any specific things you are looking for in a PR? Specifically, a PR which makes use of routeros-diff to implement a solution to this issue.

@luqasz
Copy link
Collaborator

luqasz commented Mar 17, 2021

Is routeros-diff suitable for use in implementing a solution to this issue? If not, what would you like to see changed?

For what I see so far, yes. We would have to test it either way to see what issues we will encounter.

Are there any specific things you are looking for in a PR? Specifically, a PR which makes use of routeros-diff to implement a solution to this issue.

I am thinking about merging your code / work into https://github.com/luqasz/librouteros. Those functionalities are pretty much on pair with each other. I know that librouteros provides API only code, however we can expand it and provide extra functionalities. I think it will be easier in long term to maintain and provide a consistent library. What do you think ?

@adamcharnock
Copy link

I've had a think and that sounds like a good idea to me. My thoughts are:

  1. Merging routeros-diff into librouteros definitely makes sense. It seems like a pretty natural fit
  2. I believe that fetching a /export from RouterOS is only possible via SSH. It should be possible via the API, but it has not worked for me and I have seen some forum posts stating as much. Equally, I believe uploading new scripts also requires SSH (SCP) access. I have a working solution which uses Paramiko for SSH. While I find the need for SSH unsatisfying, it does seem to work reliably. I think adding this SSH client into librouteros would make the diffing facility more practically useful, although it does expand the scope of the library. What do you think?
  3. I would like to retain some stewardship of the diffing code. Would you be open to me becoming a collaborator on librouteros?

@adamcharnock
Copy link

FYI @luqasz, I have this working now.

Napalm-ros fork

I've got a fork with the SSH client added and config reading & writing working. However, I realised that early on I also turned the fork into a working Netbox plugin. I think this was primarily because I wanted some way to store each host's SSH keys, so I created a Django model for it.

This fork should still work just fine without Netbox being available. I coded it in such a way that it should be optional. That being said, I've been juggling so many different packages to get this working that I'm willing to accept that I may have drawn the line in the wrong place in this case.

Netbox RouterOS

This is a working Netbox plugin which I have now successfully end-to-end tested. I've successfully used it to push new configuration diffs onto a RouterOS device via the SSH functionality implemented above. See the github repo for list of features.

Screenshot:

Screenshot 2021-03-19 at 19 23 46

Addendum

Just for my own sanity I want to brain dump out the various layers to this:

  • My own deployment and code which contains...
  • Netbox, which uses...
  • the netbox-routeros plugin, which uses
  • the napalm-ros packge, which uses
  • the routeros-diff package, and also
  • the librouteros package

That feels like quite a lot of turtles, and has certainly made my head hurt from time to time. I'm not sure anything needs to be done about it, but I feel better for writing it down!

I'm keen to get all of this released and usable soon. The main thing I need to make this happen is a decision from you on whether you want to merge my napalm-ros fork. If so I will submit a PR.

Secondarily, and non-blocking, is any discussion on merging routeros-diff into librouteros. I think I would prefer to tackle this as a separate question once I have everything released. I can always update dependencies if the merge goes ahead.

@luqasz
Copy link
Collaborator

luqasz commented Mar 23, 2021

Ok. Let's merge your work. Please correct me if I am wrong. Do you want to merge code from routeros-diff into napalm-ros ? If not then I fear we will have possible dependency issues. napalm-ros depending on version x of librouteros while routeros-diff may depend on other version of librouteros

@luqasz
Copy link
Collaborator

luqasz commented Apr 3, 2021

@adamcharnock What do you think ?

@adamcharnock
Copy link

Hey @luqasz - sorry for the delayed response. I'm also starting a WISP and building a house 🙄

Do you want to merge code from routeros-diff into napalm-ros?

Given my experiences in recent weeks, I think it would be best to delay this. I still finding the occasional problem with with routeros-diff, and being able to iterate/release quickly is a big benefit.

Also, I previously asked, "I would like to retain some stewardship of the diffing code. Would you be open to me becoming a collaborator on librouteros?". Do you have any thoughts on this?

If not then I fear we will have possible dependency issues. napalm-ros depending on version x of librouteros while routeros-diff may depend on other version of librouteros

routeros-diff does not actually depend on librouteros, so I don't think that will be a problem.

Ok. Let's merge your work.

Great. Just to confirm, that is the merge of my napalm-ros fork? If so I will see if I can sort out a PR over the weekend.

@luqasz
Copy link
Collaborator

luqasz commented Apr 18, 2021

Also, I previously asked, "I would like to retain some stewardship of the diffing code. Would you be open to me becoming a collaborator on librouteros?". Do you have any thoughts on this?

Sure. Everyone can contribute to librouteros.

routeros-diff does not actually depend on librouteros, so I don't think that will be a problem.

Ok so napalm-ros should produce parsable config string for routeros-diff.

Great. Just to confirm, that is the merge of my napalm-ros fork? If so I will see if I can sort out a PR over the weekend.

Yes. Please merge into config-diff branch

@stale
Copy link

stale bot commented Aug 14, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the Stale label Aug 14, 2022
@stale stale bot closed this as completed Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants