add Commit() and CommitCancel() operations

[nemith]

Adds a Session.Commit() and Session.CommitCancel() operations to issue <commit> and <cancel-commit> RPC calls.

Fixes #23

[rumenvasilev]

Commit(), WithConfirmed(), WithConfirmedTimeout(), WithPersist() and WithPersistID() later also works on cSRX.
CancelCommit() only works when persist ID is used. If no arguments are passed, it does not work. Shouldn't that also work, e.g. within the limits of the current session? Example - I commit a configuration, it kicks in, but at some point I realise it's a mistake, so I want to immediately revoke it. If I don't have the ID, I have to wait for the timeout, which might be too long.

Commit() with WithPersist() (and subsequent confirmation commit) and destroyed session in the middle don't work. I destroy the session with KillSession(), which terminates the SSH connection as well. Then after reconnecting, it doesn't work.

[nemith]

Sounds like Junos bugs to be honest. There is nothing in the library to control that behavior.

[nemith]

Actually on the cancel commit side the XML parser in Junos is absolute trash and the sometimes treat <cancel-commit/> and <cancel-commit></cancel-commit> differently.

However the encoding/xml library that this library uses cannot do self-closing tags. See golang/go#21399

It's funny that NETCONF was spearheaded buy Junosconf before it and NETCONF wouldn't exist without Juniper however Junper's support for the actual RFC is completely lacking (they don't even support RFC6242 ssh encoding)

[rumenvasilev]

What's your take on the second issue? Is it supposed to work?

Commit() with WithPersist() (and subsequent confirmation commit) and destroyed session in the middle don't work. I destroy the session with KillSession(), which terminates the SSH connection as well. Then after reconnecting, it doesn't work.

[nemith]

Commit() with WithPersist() (and subsequent confirmation commit) and destroyed session in the middle don't work. I destroy the session with KillSession(), which terminates the SSH connection as well. Then after reconnecting, it doesn't work.

The spec says it should work. Can you confirm it works with other netconf implementations (ncclient?) You are using WithPersistID() on the second one, right?

My guess is juniper just doesn't support this.

[rumenvasilev]

The spec says it should work. Can you confirm it works with other netconf implementations (ncclient?) You are using WithPersistID() on the second one, right?

My guess is juniper just doesn't support this.

Here's the operation sequence:

// get ssh client and netconf session, omitted
ctx := context.Background()
session.Lock(ctx, netconf.Candidate)
eco := []netconf.EditConfigOption{
		netconf.WithDefaultMergeStrategy(operation),
		netconf.WithErrorStrategy(netconf.StopOnError),
		netconf.WithTestStrategy(netconf.TestThenSet)}
session.EditConfig(ctx, netconf.Candidate, patchString, eco...)
session.Validate(ctx, netconf.Candidate)
session.Commit(ctx, netconf.WithPersist("123456"))
session.KillSession(ctx, uint32(session.SessionID()))
// ssh + netconf reconnect, omitted
session.Commit(ctx, netconf.WithPersistID("123456"))
session.Unlock(ctx, netconf.Candidate)

WithPersist() works if I commit within the same session, e.g.

session.Commit(ctx, netconf.WithPersist("123456"))
session.Commit(ctx, netconf.WithPersistID("123456"))

On a unrelated note:
session.KillSession() expects uint32, but session.SessionID() returns uint64. Perhaps these two should align?

Also, session closing operation.. nevermind, it's in session.go.

[nemith]

Try Close vs Kill. You really aren’t supposed to kill your own session. That being said I believe it is implement as stated in the RFC. When I get to doing proper integration testing we can see what is supported on what platforms.

…

On Sat, Feb 18, 2023 at 13:03, Rumba ***@***.***> wrote: > The spec says it should work. Can you confirm it works with other netconf implementations (ncclient?) You are using WithPersistID() on the second one, right? > > My guess is juniper just doesn't support this. Here's the operation sequence: // get ssh client and netconf session, omitted ctx := context . Background () session . Lock ( ctx , netconf . Candidate ) eco := []netconf. EditConfigOption { netconf . WithDefaultMergeStrategy ( operation ), netconf . WithErrorStrategy ( netconf . StopOnError ), netconf . WithTestStrategy ( netconf . TestThenSet )} session . EditConfig ( ctx , netconf . Candidate , patchString , eco ... ) session . Validate ( ctx , netconf . Candidate ) session . Commit ( ctx , netconf . WithPersist ( "123456" )) session . KillSession ( ctx , uint32 ( session . SessionID ())) // ssh + netconf reconnect, omitted session . Commit ( ctx , netconf . WithPersistID ( "123456" )) session . Unlock ( ctx , netconf . Candidate ) WithPersist() works if I commit within the same session, e.g. session . Commit ( ctx , netconf . WithPersist ( "123456" )) session . Commit ( ctx , netconf . WithPersistID ( "123456" )) — Reply to this email directly, [view it on GitHub](#26 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AACVJMCPIB5K3JGU3AVZFQDWYETKHANCNFSM6AAAAAAU7SXXYA). You are receiving this because you were assigned.Message ID: ***@***.***>

[rumenvasilev]

I've been searching for persist in JunOS netconf configuration and I couldn't find it. I don't think persist is even implemented. I guess you can merge your PR as is, as it implements the RFC.

// I did try with session.Close() too, same outcome.