[YANGTOOLS-859] Augmenting a container with if-feature Created: 07/Mar/18  Updated: 04/Nov/20  Resolved: 04/Nov/20

Status: Resolved
Project: yangtools
Component/s: parser
Affects Version/s: 1.2.1, 2.0.9
Fix Version/s: 4.0.14, 6.0.1, 5.0.8

Type: Bug Priority: Medium
Reporter: Peter Verthez Assignee: Robert Varga
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Zip Archive minimalmodel.zip     Zip Archive models.zip    
Issue Links:
Blocks
blocks YANGTOOLS-694 Eliminate duplicate DescriptionEffect... Resolved
Relates
relates to YANGTOOLS-403 Add support for tailf:action Confirmed

 Description   

I would like ODL to reconsider the point of YANGTOOLS-811.

I've been researching this a little, and it seems people from the RFC itself acknowledge that the RFC is unclear on this point:

https://www.ietf.org/mail-archive/web/netmod/current/msg17835.html

However, there are standardized YANG modules that are relying on the assumption that no if-feature is needed in the augment, see the following standardized models from BBF:

https://github.com/BroadbandForum/yang/blob/master/standard/interface/bbf-sub-interfaces.yang

https://github.com/BroadbandForum/yang/blob/master/standard/interface/bbf-sub-interface-tagging.yang

The first model contains at the end containers ingress-rewrite and egress-rewrite that are conditional under the feature tag-rewrites.   The second module contains an augment on these containers, without an if-feature statement.

It has also been brought to my attention that ConfD from Tail-F had provided a fix for this problem.    So it seems there is some kind of consensus out there that the if-feature statement is not needed in the augment.

That is why I request that ODL reconsiders this.

 Comments   
Comment by Robert Varga [ 07/Mar/18 ]

This actually works as expected with 2.0.1.

Comment by Robert Varga [ 07/Mar/18 ]

As far as I can tell, this works with 1.2.2, too. What version are you seeing this with?

Comment by Peter Verthez [ 07/Mar/18 ]

OK, we are using 1.2.1 at this moment.

Comment by Robert Varga [ 07/Mar/18 ]

Unless I am missing something, the reproducer is as simple as :

module foo {
    namespace foo;
    prefix foo;

    feature foo;

    container foo {
        if-feature foo;
    }
}

module bar {
    namespace bar;
    prefix bar;

    import foo {
        prefix foo;
    }

    augment "/foo:foo" {
        container bar {
        }
    }
}

 

 and parsing this without feature foo present.

Comment by Peter Verthez [ 07/Mar/18 ]

Yes, looks correct.

Comment by Robert Varga [ 07/Mar/18 ]

Try it out with 1.2.2 and update the issue accordingly, please.

Comment by Peter Verthez [ 07/Mar/18 ]

OK, will do that.

Comment by Peter Verthez [ 07/Mar/18 ]

I can't reproduce it with the above modules in 1.2.1 either.   I'll check what the exact reproducer is then, because we do have the problem with the standard modules I mentioned.

Comment by Peter Verthez [ 08/Mar/18 ]

I've attached the combination of standard models that exhibits the bug, even in 1.2.1.   I'll still work on a minimized version of this if needed.

Comment by Peter Verthez [ 08/Mar/18 ]

I will also still try 1.2.2.

Comment by Peter Verthez [ 08/Mar/18 ]

With the attached models, I have exactly the same error in 1.2.2.

Comment by Peter Verthez [ 08/Mar/18 ]

I've been able to minimize the model needed to reproduce: see attachment minimalmodel.zip.   Apparently the 2-level augment is relevant: without that 2-level augment it works.

Comment by Robert Varga [ 09/Mar/18 ]

2.0.2 assembles both packages without any issue. What is the error you are seeing?

Comment by Robert Varga [ 09/Mar/18 ]

Sorry, I forgot to jiggle the features. 2.0.2 reports:

org.opendaylight.yangtools.yang.parser.spi.meta.SomeModifiersUnresolvedException: Some of EFFECTIVE_MODEL modifiers for statements were not resolved.
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.BuildGlobalContext.addSourceExceptions(BuildGlobalContext.java:354)
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.BuildGlobalContext.completePhaseActions(BuildGlobalContext.java:415)
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.BuildGlobalContext.executePhases(BuildGlobalContext.java:219)
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.BuildGlobalContext.buildEffective(BuildGlobalContext.java:230)
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.CrossSourceStatementReactor$BuildAction.buildEffective(CrossSourceStatementReactor.java:196)
    at org.opendaylight.yangtools.yang.stmt.StmtTestUtils.parseYangSources(StmtTestUtils.java:141)
    at org.opendaylight.yangtools.yang.stmt.StmtTestUtils.parseYangSources(StmtTestUtils.java:158)
    at org.opendaylight.yangtools.yang.stmt.StmtTestUtils.parseYangSources(StmtTestUtils.java:190)
    at org.opendaylight.yangtools.yang.stmt.YT859Test.testFindDataSchemaNode(YT859Test.java:20)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:538)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:760)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:460)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:206)
Caused by: org.opendaylight.yangtools.yang.parser.spi.meta.InferenceException: Yang model processing phase EFFECTIVE_MODEL failed [at /home/nite/odl/yangtools/yang/yang-parser-rfc7950/target-ide/test-classes/bugs/YT859/bbf-sub-interface-tagging.yang:1:0]
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.SourceSpecificContext.failModifiers(SourceSpecificContext.java:360)
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.BuildGlobalContext.addSourceExceptions(BuildGlobalContext.java:319)
    ... 31 more
Caused by: org.opendaylight.yangtools.yang.parser.spi.meta.InferenceException: Augment target 'Absolute{path=[(urn:ietf:params:xml:ns:yang:ietf-interfaces)interfaces, (urn:ietf:params:xml:ns:yang:ietf-interfaces)interface, (urn:bbf:yang:bbf-sub-interfaces)egress-rewrite]}' not found [at /home/nite/odl/yangtools/yang/yang-parser-rfc7950/target-ide/test-classes/bugs/YT859/bbf-sub-interface-tagging.yang:15:2]
    at org.opendaylight.yangtools.yang.parser.rfc7950.stmt.augment.AbstractAugmentStatementSupport$1.prerequisiteFailed(AbstractAugmentStatementSupport.java:161)
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.ModifierImpl.failModifier(ModifierImpl.java:83)
    at org.opendaylight.yangtools.yang.parser.stmt.reactor.SourceSpecificContext.failModifiers(SourceSpecificContext.java:348)
    ... 32 more

Test case at:  https://git.opendaylight.org/gerrit/69298

Comment by Robert Varga [ 09/Mar/18 ]

Looks like a problem in SchemaNodeIdentifierBuildNamespace around subtree matches. The inference action needs to succeed, but report that the target is not supported. Since namespaces currently support only exact matches, we need to teach them about matching subtrees.

This will definitely require an API change to report that 'your prerequisite failed, here is the closest node we have'. Based on that information AbstractAugmentStatementSupport can understand that it is not to take any action, as an unsupported parent exists.

Comment by Robert Varga [ 09/Mar/18 ]

Resolving this equals to solving part 3) of YANGTOOLS-694 problem statement.

Comment by Peter Verthez [ 21/Mar/18 ]

We're seeing a similar problem with deviations (Deviation target not found), also with if-feature involved.   Is that the same issue, or should I create a new ticket for this?

Comment by Peter Verthez [ 28/Jun/18 ]

Hi, what is the target date for fixing this?   This is causing constant issues with the devices that we manage, for which we currently each time have to perform a workaround.

Comment by Robert Varga [ 29/Jun/18 ]

Sorry, no fixed target date yet, as I am busy in downstream projects. If I get enough quiet time it could make it into Oxygen SR3, but I cannot guarantee that.

Comment by Robert Varga [ 31/Jul/18 ]

So I have a prototype patch, 69298,17, but the it does not quite work. The reason for that is that we have two augments at play, the first one introducing a if-feature container which is then being augmented.

As it stands we do not populate if-feature'd statements if that feature is not effective, which is what makes sense. If we populate them, we need to understand that is really going on and account for it – somehow, I am not yet sure how.

Comment by Robert Varga [ 03/Aug/18 ]

So the problem is that I cannot find any text in RFC7950 which would preclude the following nightmare:

feature foo;
feature bar;

container baz {
   if-feature "foo and (not baz)";
}

list baz {
   if-feature "(not foo) and baz";
}

i.e. mutually-exclusive items which conflict on namespace, but are both augmentable. Eventhough such a scenario will thoroughly break MD-SAL, it should be acceptable by YANG parser, unless it can be proven otherwise.

This means, though, that at least ChildSchemaNodeNamespace needs to track a soft "potentially unsupported" state, which is cleared when a statement is fully resolved. If the statement is not fully resolved by end of the namespace mutation, we need to push either an "unsupported" or "invalid" event.

Comment by Robert Varga [ 03/Aug/18 ]

I have raised https://mailarchive.ietf.org/arch/msg/netmod/9JgPZphOdim-wLZA3NrSe3I1un4 will see what the reply is.

Comment by Robert Varga [ 08/Aug/18 ]

Based on the initial reply it would seem the sentiment is to disallow the above construct. That means we can ignore the nightmare.

Comment by Robert Varga [ 08/Aug/18 ]

ChildSchemaNodeNamespace needs to be be updated to handle the case of adding an unsupported statement. Details are a bit sketchy, but I think it would work by squashing unsupported statements into a well-known (private) value and filter them out when queried.

Furthermore augment/deviate etc. copy functions need to let the copy statement get created (so that proper onStatementAdded() namespace-handling logic kicks in) and then not add them to the copy set if they are not supported.

That is not exactly pretty, but should provide the final pieces to the puzzle.

Generated at Wed Feb 07 20:54:30 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.