Junos script newline normalization

[jberger]

While testing eNMS using a Juniper switch, we had to normalize the config script's newlines just before sending the command to napalm. If this wasn't done we kept getting errors about literal "\n" characters in identifiers.

Specifically I changed https://github.com/afourmy/eNMS/blob/master/source/scripts/models.py#L286 from

getattr(napalm_driver, self.action)(config=self.content)

to

getattr(napalm_driver, self.action)(config=self.content.replace('\r\n', '\n').replace('\r', '\n'))

If this is a sufficient fix I can pull it together as a PR, however I am not submitting it as such yet because I suspect there's a more wholistic fix that I'm not seeing.

[afourmy]

Thank you for your feedback ! Newlines in an HTML textarea should be normalized to CR LF so getting \r\n seems fine, but I don't know why Junos complains here (I'm testing eNMS primarily on Cisco). I will run some tests and try to reproduce it (I'd rather not merge this fix right now because I fear it might have unwanted side-effects on other platforms).

[afourmy]

@jberger As it turns out, it was slightly worse than what you spotted: there is a discrepancy in the way Napalm handles closing a connection to a device depending on the OS. It's not the same behavior for IOS and Junos, and it impacts eNMS because of the way Napalm integration is currently implemented. I opened napalm-automation/napalm#746, and I fixed eNMS locally in another branch which contains the next version of eNMS, but I cannot commit because this branch is not stable yet. I will publish a new release this week-end with this fix along with many other improvements, so I would suggest you hold off your tests for a couple of days (until next week), then if it works for you, we can close this issue. I usually test for Cisco only, but for this new version, I'll make sure everything works for Juniper too.

[afourmy]

I had added commit_config before closing the connection and showed that for simple changesets (ie when you wouldn't need complex rollback) that was enough, but clearly that wasn't sufficient for more complex things.

Just to be sure I understand correctly, you are saying that it is not sufficient for your needs to automatically commit once a load merge/replace operation is performed, i.e you would like a napalm connection to be persistent throughout the workflow ? Or is it enough to commit each time you load merge/replace, i.e for each task of a workflow, we can open and close a napalm connection ?

[jberger]

I'm more of a developer than a network engineer so I should leave that comment to my neteng team. I was just trying to make it not rollback and that simple fix caused it to behave as expected. I'm not sure what the right answer would be. Hopefully they'll comment too (I've linked them to this issue on our slack).

[afourmy]

I see. I was asking because I cannot think of a use-case where it would make sense to persist a napalm connection throughout the workflow, and I had the impression that's what you were asking for. We probably fixed it the same way: by commiting right after the load merge/replace operation (in the current version, technically, the changes are not rolled back, they are discarded as a side-effect of closing the napalm connection, and when the next step of the workflow tries to commit... there simply isn't anything to commit, so it commits nothing).
If it behaves as you expect with the fix you did, then it's fine because I did the same fix.

[phil21]

This fix is fine with us, I meant to hop on Slack today to discuss a little bit but I think the confusion on our end had to do with how multi-device workflows will one day function.

After reviewing the logic here, I think this is what we were expecting.

[afourmy]

This fix is fine with us, I meant to hop on Slack today to discuss a little bit but I think the confusion on our end had to do with how multi-device workflows will one day function.

Mutli-device workflows will also be part of this week-end new release. I remember saying it would be a "surgical" update, but it turned out to be a big refactoring of the workflow system.

In the current version, a workflow is a graph of script. You create a script, then you build a workflow with scripts, and you schedule the workflow. In the next version, a workflow will be a graph of tasks instead (the bricks of a workflow will be special "unscheduled" tasks). That changes everything, because a task, by definition, is a set of scripts applied to a set of nodes. That means each node of a workflow will be both multi-script and multi-device. This is a big change in the model, but it is almost transparent for the end-user, because you still create and schedule workflows in the same fashion.

[afourmy]

I have just published a new release that you can find here : https://github.com/afourmy/eNMS/releases/tag/v2.1.0

It includes the fix for Napalm Junos configuration push, multi-target support for workflows as well as the ReST API to remote call a task (i.e a script or a workflow via the task). Note that the logs and editing menu of a task in a workflow can be accessed from the right-click menu in the workflow_editor page.

I created a chapter in the doc for the ReST API (https://enms.readthedocs.io/en/latest/automation/rest.html), but the part of the docs that explains how workflows function is not yet up-to-date (more or less the same, except a workflow is made of tasks instead of script) (future enhancements for the ReST API: #77)

Waiting for your feedback to close this issue.

[afourmy]

@jberger @phil21 We had a discussion a while ago about how to see the results of a job triggered with a REST call, this is now possible. When you trigger a task (with a http://IP_address/rest/execute_task/task_name GET call as described here: https://enms.readthedocs.io/en/latest/automation/rest.html), eNMS will return a dictionnary which contains a key id with the id of the job (which is actually the creation time of the job).
Then, when you retrieve the parameters of the task (via a http://IP_address/rest/object/task/task_name), the output contains the logs of the task, a dictionnary which keys are the id of all the jobs for that task (i.e all runs of the task). I believe this is what you were asking for.

I will close this issue as it was fixed a long time ago, but you can ask on slack if it's not clear.