Subject: [Netmod-ver-dt] Guidelines text comments

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Thu, 28 Feb 2019 12:19:01 +0000

Hi Reshad,

Apologies that it is so late, but some comments on the Guidelines text:

Main comments:

(1)   Section 7.1.1 needs to give more guidance on when to use m or M.  E.g. perhaps in the description of the updating the patch.  One choice would be to split between how modules "SHOULD" be updated (i.e. just MAJOR.MINOR.PATCH without modifier) vs how modules "MAY" be updated (i.e. using PATCH modifier).

(2)   The guidance at the moment is very much that all changes should be backwards compatible, use status, etc.  But I think that it just one approach, not all vendors are following this, OpenConfig doesn't follow this.  Also, if version selection is an option, then that is also another potential solution to the problem.

I've also put comments inline [RW]

If we have time, discussing this section and these comments may be helpful today.  I know that we also have comments from Balazs to go through.

From: Rob Wilton (rwilton) <[email-masked]>
Sent: 28 February 2019 09:59
To: Rob Wilton (rwilton) <[email-masked]>
Subject: Guidelines text

7.  Guidelines

7.1.  Guidelines to YANG model authors

[RW]

I think that we should be should have some overview text here, along the lines of:
- nbc changes cause problems to clients, but sometimes they are required (e.g. fix something that is broken).
- module authors are recommended to try and minimize NBC changes, and keep changes BC where pragmatic.
- using status deprecated allow with status-description helps a client understand how to migrate to alternative data nodes.
- if NBC changes are introduced then consider the impact on clients and try to mitigate that were possible.

7.1.1.  Use of semantic versioning

   Reuse of an already used module-version MUST be avoided.  For
   example, when updating 3.4.0 in a NBC manner the module author must
   verify whether version 4.0.0 is available for use and if that version
   was already used, the updated module must use the version 3.4.1M.

   From a published revision N of a module, the next published revision
   N+1 of the module SHOULD have one of:

   1.  MAJOR version incremented by 1, MINOR and PATCH versions set to
       0.  This indicates that in revision N+1:

       *  There have been one or more non-backwards-compatible changes

       *  There may also be new functionality which have been added in a
          backwards-compatible manner

       *  There may also be bug fixes which have been made in a
         backwards-compatible manner

   2.  MAJOR version unchanged, MINOR version incremented by 1 and PATCH
       version set to 0.  This indicates that in revision N+1:

       *  There are no non-backwards-compatible changes

       *  There have been one or more new functionality which have been
          added in a backwards compatible-manner

       *  There may also be bug fixes which have been made in a
          backwards-compatible manner

Claise, et al.           Expires August 31, 2019               [Page 16]

Internet-Draft           YANG Module Versioning            February 2019

   3.  MAJOR and MINOR versions unchanged, PATCH version incremented by
       1.  This indicates that in revision N+1:

[RW]

This definition isn't quite right, it doesn't follow the semver rules.  Because YANG is an API rather than an implementation then bugfixes to the API are not allowed to increment the patch number.

The PATCH number for regular semver should probably state something like "There are no backwards or non-backwards compatible changes, but there may be editorial changes such as fixing a description statement that doesn't change the semantics of the data node in anyway."

       *  There are no non-backwards-compatible changes

       *  There is no new functionality which has been added in a
          backwards compatible-manner

       *  Ther are 1 or more bug fixes which have been made in a
          backwards-compatible manner

   As explained in Appendix A.2.2, while programatically determining a
   semantic version is possible using the pyang utility, human overisght
   is highly recommended because of some special cases which can not be
   detected by pyang.  Therefore, a model author SHOULD use both means
   to determine a model's semantic version.

[RW]

Rather than binding this to pyang, I would talk by using "tooling (e.g. pyang)".

7.1.2.  Making non-backwards-compatible changes to a YANG module

[RW]

So, it feels like these guidelines are really the RFC 7950 way of making changes, but I don't think that semver necessarily requires this approach.  In particular, I question the usefulness of recommending new nodes are created, existing at the same time as a deprecated version, but then saying that handling the coexistence of those two nodes is outside the scope of this specification. As an aside, I'm not convinced that there is a generic workable solution for this problem.

   There are various valid situations where a YANG module has to be
   modified in a non-backwards-compatible way.  Here are some guidelines
  on how non-backwards compatible changes SHOULD be made:

   1.  The changes should be made incrementally, e.g. a data node's
       status SHOULD NOT be changed directly from "current" to
       "obsolete" (see Section 4.7 of [RFC8407]), instead the status
       SHOULD first be marked "deprecated" and then when support is
       removed its status MUST be changed to "obsolete".  Instead of
       using the "obsolete" status, the data node MAY be removed from
       the model but this has the risk of breaking modules which import
       the modified module.

   2.  A node with status "deprecated" MUST be supported.  Q: if we keep
       this, do we still need the deprecated-nodes-present leaf, see
       Section 5.

[RW]

I would possible say something like: The semver solution described in this document requires that deprecated nodes are implemented to function correctly".

   3.  A node with status "deprecated" SHOULD be available for at least
       one year before its status is changed to "obsolete", see
       Section 4.7 of [RFC8407].

   4.  Support for a node which is "obsolete" is indicated by the node
       "obsolete-nodes-present, see Section 5.

   5.  The new extension "status-description" SHOULD be used for nodes
       which are "obsolete" or "deprecated".

   6.  For status "deprecated", the "status-description" SHOULD also
       indicate until when support for the node is guaranteed.  If there

Claise, et al.           Expires August 31, 2019               [Page 17]

Internet-Draft           YANG Module Versioning            February 2019

       is a replacement data node, rpc, action or notification for the
       deprecated node, this MUST be stated in the "status-description".

[RW]

I think that this should be can rather than SHOULD, and I think that the MUST should be a SHOULD

   7.  When obsoleting or deprecating data nodes, the "deprecated" or
       "obsolete" status SHOULD be applied at the highest possible level
       in the data tree.  For example, when deprecating all data nodes
       in a container, the "deprecated" status SHOULD be applied to the
       container.

[RW]

I think that the status should be specified on every node to make the module more readable.  Although this raises the question as to whether they should be a status-description on every node.

   The following sections have examples on how non-backwards-compatible
   changes can be made.

7.1.2.1.  Removing a data node

   Removing a leaf or container from the data tree, e.g. because support
   for the corresponding feature is being removed:

   1.  The node's status SHOULD be changed to "deprecated" and it MUST
       be supported for at least one year.  This is a backwards-
       compatible change.

[RW]

I think that the MUST should be SHOULD.  I'm not sure whether anything that is guidance or guidelines can really be a MUST because then they are not guidelines so much as rules ;-)

   2.  When the node is not available anymore, its status MUST be
       changed to "obsolete" and the "status-description" updated, this
       is a non-backwards-compatible change.  The "status-description"
       SHOULD be used to explain why the node is not available anymore.

7.1.2.2.  Changing the type of a leaf node

[RW]

I'm not convinced that this is always the right way to handle this change.  If the leaf is config false, then going the deprecation path make sense, for configurable items then the right solution is less obvious.  E.g. if it is a bugfix then I think that it should probably just be fixed as an NBC change.

   Changing the type of a leaf-node. e.g. consider a "vpn-id" node of
   type integer being changed to a string:

   1.  The status of node "vpn-id" SHOULD be changed to "deprecated" and
       the node SHOULD be available for at least one year.  This is a
       backwards-compatible change.

   2.  A new node, e.g. "vpn-name", of type string is added to the same
       location as the existing node "vpn-id".  This new node has status
       "current" and its description SHOULD explain that it is replacing
       node "vpn-id".

   3.  During the period of time where both nodes are available, how the
       server behaves when either node is set is outside the scope of
       this document.  Here are some options:

       1.  A server MAY prevent the new node from being set if the old
           node is already set (and vice-versa).  The new node MAY have
           a when statement to achieve this.  The old node MUST NOT have
           a when statement since this would be a non-backwards-

Claise, et al.           Expires August 31, 2019               [Page 18]

Internet-Draft           YANG Module Versioning            February 2019

           compatible change, but the server MAY reject the old node
           from being set if the new node is already set.

       2.  If the new node is set and a client does a get or get-config
           operation on the old node, the server MAY map the value.  For
           example, if the new node "vpn-name" has value "123" then the
           server MAY return integer value 123 for the old node "vpn-
           id".  However, if the value can not be mapped, we need a way
           of returning "unsupported" TBD.

   4.  When node "vpn-id" is not available anymore, its status MUST be
       changed to "obsolete" and the "status-description" is updated.
       This is a non-backwards-compatible change.

7.1.2.3.  Reducing the range of a leaf node

7.1.2.4.  Changing the key of a list

7.1.2.5.  Renaming a node

7.1.2.6.  Changing a default value

7.2.  Guidelines to YANG model clients

   Here are some guidelines for YANG model clients:

   o  Clients SHOULD be liberal when processing data received from a
      server.  For example, the server may have increased the range of
      an operational node causing the client to receive a value whch is
      outside the range of the YANG model revision it was coded against

   o  Clients SHOULD look for status changes in new revisions of YANG
      models they support.

   o  Clients SHOULD plan to make changes to match published status
      changes.  When a node's status changes from "current" to
      "deprecated", clients SHOULD plan to stop using that node in a
      timely fashion.  When a node's status changes to "obsolete",
      clients MUST stop using that node.

[RW]

I would weaken the last sentence to:

"When a node's status changes to "obsolete" then clients "

Thanks,
Rob

   o  A client SHOULD not use unknown NBC versions automatically

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Thu, 28 Feb 2019 13:42:51 +0000

Hi Rob,

I’ll try to take a look before our meeting today but not sure I’ll be able to.

Regards,
Reshad.

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Thursday, February 28, 2019 at 7:19 AM
To: "Reshad Rahman (rrahman)" <[email-masked]>, "[email-masked]" <[email-masked]>
Subject: Guidelines text comments

Hi Reshad,

Main comments:

  1.  Section 7.1.1 needs to give more guidance on when to use m or M.  E.g. perhaps in the description of the updating the patch.  One choice would be to split between how modules “SHOULD” be updated (i.e. just MAJOR.MINOR.PATCH without modifier) vs how modules “MAY” be updated (i.e. using PATCH modifier).
  2.  The guidance at the moment is very much that all changes should be backwards compatible, use status, etc.  But I think that it just one approach, not all vendors are following this, OpenConfig doesn’t follow this.  Also, if version selection is an option, then that is also another potential solution to the problem.

I’ve also put comments inline [RW]

7.  Guidelines

[RW]

[RW]

This definition isn’t quite right, it doesn’t follow the semver rules.  Because YANG is an API rather than an implementation then bugfixes to the API are not allowed to increment the patch number.

The PATCH number for regular semver should probably state something like “There are no backwards or non-backwards compatible changes, but there may be editorial changes such as fixing a description statement that doesn’t change the semantics of the data node in anyway.”

[RW]

Rather than binding this to pyang, I would talk by using “tooling (e.g. pyang)”.

[RW]

So, it feels like these guidelines are really the RFC 7950 way of making changes, but I don’t think that semver necessarily requires this approach.  In particular, I question the usefulness of recommending new nodes are created, existing at the same time as a deprecated version, but then saying that handling the coexistence of those two nodes is outside the scope of this specification. As an aside, I’m not convinced that there is a generic workable solution for this problem.

[RW]

I would possible say something like: The semver solution described in this document requires that deprecated nodes are implemented to function correctly”.

[RW]

[RW]

[RW]

I think that the MUST should be SHOULD.  I’m not sure whether anything that is guidance or guidelines can really be a MUST because then they are not guidelines so much as rules ;-)

[RW]

I’m not convinced that this is always the right way to handle this change.  If the leaf is config false, then going the deprecation path make sense, for configurable items then the right solution is less obvious.  E.g. if it is a bugfix then I think that it should probably just be fixed as an NBC change.

7.1.2.5.  Renaming a node

[RW]

“When a node’s status changes to “obsolete” then clients ”

Thanks,
Rob