Add prefix and negate_prefix to container

[matejv]

Implementation of discussion in #66 Just a POC for now, more changes to be done.

[matejv]

For a container without key_value you'll get an empty <command/> element in the XML document. In that case element.text is None and the next line crashes.

I'll see if I can avoid creating the element in such cases.

[dbarrosop]

It's probably cleaner to do:

if element.tag == "command":
    text += element.text  or ""

But it doesn't matter, both do the same.

[dbarrosop]

Overall it looks great :)

I see CI is broken but it might be because you actually fixed it so feel free to update the test cases if needed.

[matejv]

Yeah, no. Some test cases will indeed need to be updated. But there's still a bunch of things broken ATM, namely XMLTranslator and interaction of negate_prefix with continue_negating: True.

Trying to fix it now.

[matejv]

I'm a bit stumped.

I have the changes behave the way I wanted - mostly. The only thing not working is candidate.translate_config(..., replace=running)

It will keep negating route - first by static leaf and then each next-hop as well instead of only doing static and then stop negating.

So if you have a minimal openconfig structure with a single static route and use it for candidate and running config:

d = {'network_instances': {'network_instance': {u'global': {'config': {'enabled': True,
     'type': u'DEFAULT_INSTANCE'},
    'name': u'global',
    'protocols': {'protocol': {u'static static': {'identifier': u'static',
       'name': u'static',
       'static_routes': {'static': {u'2.2.2.2/32': {'config': {'prefix': u'2.2.2.2/32'},
          'next_hops': {'next_hop': {u'10.1.1.9': {'config': {'next_hop': u'10.1.1.9'},
             'index': u'10.1.1.9'}}},
          'prefix': u'2.2.2.2/32'}}}}}}}}}}
running.load_dict(d)
candidate.load_dict(d)

So I'd expect it to do this:

no ip route 2.2.2.2/32
ip route 2.2.2.2/32 10.1.1.9

But it does this:

no ip route 2.2.2.2/32
no ip route 2.2.2.2/32 10.1.1.9
ip route 2.2.2.2/32 10.1.1.9

And on the second no statement at least IOS will yell at you:

%No matching route to delete

Which is not ideal if you want to deploy this config.

I'll try to take continue_negating into account on replace operation. I think it's not currently.

[dbarrosop]

Ok, if you don't manage to get it fixed don't worry. Push everything you have into a branch, build a test case that fails so I have the same scenario as you and I can try to help you looking into it. Maybe we could expose via some variables if we are "replacing", "merging" or even "negating". For example, we could stop the execution of the tree after the no ip route by just saying when: "{{ not negating }}" instead of adding even more variables to the actions.

[matejv]

Sorry for a late reply, other things got in the way.

Just pushed an updated test case that fails. This is what we'd have to fix.

[matejv]

OK, I've followed you suggestion and passed additional variables to resolve_rule. And with that I can make my mappings work as expected!

But I'm afraid, I am making a right mess of the code :)

[dbarrosop]

Great job! I will try to test and provide feedback later this week :)

[dbarrosop]

Ok, this is great. Code wise is a bit of a mess but I don't think there is much we can do right now. At some point when the framework is more feature complete we might want to revisit/refactor and improve some bits but right now I mostly care about the DSL and your additions certainly improved both the DSL itself and the output.

Awesome job! Will be rebasing and doing tiny changes to your code before merging. Will link the commit here for reference when done..