load_merge_candidate is not additive

[mirceaulinic]

Description of Issue/Question

Executing a series of load merge operations, it preserves only the latest config:

>>> i.load_merge_candidate(config='ntp server 1.2.3.7')
>>> i.load_merge_candidate(config='ntp server 1.2.3.8')
>>> i.load_merge_candidate(config='ntp server 1.2.3.9')
>>> i.load_merge_candidate(config='ntp server 1.2.3.10')

I think it would require reading the merge_config.txt file and append the latest config addition before writing the file on the flash.

[mirceaulinic]

To read the content of the merge_config.txt, we could probably do:

prev_merge = self.device.send_command('more merge_config.txt')

In case the file does not exist yet, it will throw:

u'%Error opening bootflash:fake.txt (No such file or directory)'

Then we don't need to append, but simply load.

After commit, I think we can delete that file.

[ktbyers]

@mirceaulinic

People are going to shoot themselves in the foot with this on Cisco IOS...so is this really the operation we want?

If we are going to make this change, I think we need to clearly let people know (as it will probably cause some people some major pain).

I think it is probably the right move (for consistency with our configuration operations in NAPALM), but we definitely need to communicate this.

cc: @dbarrosop

[ktbyers]

@mirceaulinic FYI, I don't think we should delete the file, I think we should just always zero it out (like we do on rollback operation). We will need to make sure this is done on commit.

I think this will run into fewer errors than deleting the file.

[mirceaulinic]

Yes @ktbyers - IMO we should document this properly. However, if I recall correctly the merge feature itself has been introduced relatively soon.

Anyway, if we want the API to work consistent and have the behaviour of loading changes into the candidate candidate config and visible only after commit, I think we have to do it. It is like that already, but the candidate config should support multiple changes.

I don't think we should delete the file, I think we should just always zero it out (like we do on rollback operation). We will need to make sure this is done on commit.

I agree on this, wasn't aware that it's possible to zero it.

[ktbyers]

@mirceaulinic Inline merge is relatively recent, but what would be the behavior of non-inline merge (i.e. of a file merge is that additive as well)?

We should do a design spec of how this is going to work (as part of this issue) as there are a lot of different scenarios.

[mirceaulinic]

I would need some more background on this @ktbyers, please.

I understand there are two possibilities to load merge:

• using TCL (is this the inline one?)
• using SCP

For the SCP I was thinking we could read the merge_config.txt file from the flash, append the config the user wants, then save it (on flash, on merge_config.txt). Regarding the TCL way I haven't look closer and I assume I'm missing many details, so yes, probably it would be a big challenge on this one. Anyway, I don't see any immediate need or rush, so please let keep it as discussion till I completely understand what it requires.

[ktbyers]

@mirceaulinic Yes, there are four combinations for merge:

inline (using TCL) using config argument inline (using TCL) using file SCP using config argument SCP using file

But, yes ultimately the two use cases are inline (TCL) and SCP (and then it is just a matter of do we read or write a file beforehand).

The main question I have is...do we also do additive when not using the config argument? I would think the answer would need to be yes?

If that is the case, then I think we would need to do the following:

1. Read the current contents of merge_config.txt file.
2. Create a new merge_config.txt file locally in the program (and written to the file system if using SCP).
3. Transfer the entirely new file using Inline or SCP.

I think that should work :-)

[mirceaulinic]

That sounds good to me too. I can try a PoC for this next week. Thanks @ktbyers

[dbarrosop]

I think it makes sense that the merge operate works as @mirceaulinic described initially so it should be additive as if a user doesn't want that behavior it's as simple as discarding the candidate. This is how other drivers work.

I will leave the technical details up to you, you don't need me there ;)

@ktbyers how viable and useful is to default to the inline transfer? Seems as powerful as the scp method without the contraints, or will it be a problem with large operations? I was just guessing because if there is going to be a change that requires notification, we might as well include this one and notify 2 in 1.

[ktbyers]

@mirceaulinic This should be rolled into the unit tests for real devices (i.e. we should have this as part of the unit tests). Keep this issue open and I will possibly see if I can work on that.

[ktbyers]

@dbarrosop I think the Secure Copy is the more reliable mechanism. I think there are more errors/problems we will run into with the inline transfer.

We should probably think about doing things to remove errors (like auto-enable archive; better error messages, etc.)

[mirceaulinic]

Moved to https://github.com/napalm-automation/napalm/issues/454