reset_port() and/or reset_nic()

[henn]

We need a function to reset a switch port to be on the VLANs that HaaS thinks they should be on.

This would be useful when:

• someone manually modifies the switch port to be on certain VLANs
• troubleshooting network connectivity problems (admin can use reset_nic() flavor)
• when in environments where machines come in and out of haas on a regular basis (we'll want to reset the port when bringing the machine in so we can ensure it's not connected to anything)
• when a switch is replaced, whereby NICs might be connected to different ports and a reset_nic() could be called to set the NIC properly.

[henn]

This has two components:

1. Use switch-specific commands to reset a port to an empty state
2. Apply haas's views of which VLANs a port should be connected to

[kylehogan]

After reading some of the docs I get the impression that reset is the wrong word here. Reset implies reverting to some default state.

I think what we want is to remove every VLAN from a port.

e.g. the command 'no switchport trunk allowed vlan' is described as 'reset a trunking characteristic to
the default' which would be "its Native VLAN and the port belongs to either all
VLANs or only to the Default VLAN depending on a value of parameter
Trunk Port Default Configuration."

while 'switchport trunk allowed vlan none —Specifies an empty VLAN list The port does not belong to any VLAN'

Is this correct?

[SahilTikale]

I understand what you are saying @kylehogan . I agree that reset_ sounds like resetting to its default state. We are not doing that. What we are doing here is a restore operation.
We should call this calls port_restore or nic_restore

[kylehogan]

I like port_restore and nic_restore better as well. Was I correct in assuming that I would want to remove access for all VLANs and that this would be considered 'restoring' it?

[zenhack]

So in a comment on #621, @henn suggested sending the the actual networking operations to the backend via the usual mechanism, rather than handling them in the api call proper. There's a problem with this, that requires at least a bit of rearchitecting: right now the only thing we can send to the network daemon is "attach this port to this network on this channel" or "detach this port from any network on this channel." This interface is unable to express the operation we want to preform here.

So the question becomes, how do we extend this?

One possibility: have different types of networking actions. We could use a similar inheritance mechanism to have subclasses of NetworkingAction, and differentiate on the other end.

One awkwardness with this is right now we're passing the networking action to the session, so all of the knowledge about how to interpret a networking action needs to be per-switch, which means each driver needs to implement it. Ick.

A possibly better design is to have the NetworkingAction "drive" on the other end; it makes use of a handful of primitives provided by the switch/session, so we can add new api-level operations without having to change every driver in the future.

This is still a little bit more refactoring than I expected to have to do before starting to pick at this, so I'd like others' perspectives before I go ahead and do a bunch of work on it.

//cc @henn, @gsilvis, @knikolla

[henn]

Interesting point - it didn't occur to me how tied to connect/disconnect
the current NetworkingAction API is.

Extending the NetworkingAction table sounds good. Doing something
simpler, like having NetworkingAction contain an action (CONNECT,
DISCONNECT, RESET) then a JSON blob containing the arguments) could also
work, and might lead to trusting the front end less down the road since
we'll have to re-validate the arguments.

[zenhack]

Meta: I worry somewhat about designing our own RPC protocol by accident. I like your suggestion, but also want to point out that it's like 60-70% of the way to just using JSON-rpc; if we go down that road I want to be explicit that our understanding is there's an existant rpc protocol at the end of it.

[zenhack]

I'm going to start grabbing other tasks from the 0.2 milestone, so I can work on stuff in parallel while we figure out the design issues for this one.

[henn]

@zenhack: That's a really good point - we are designing an RPC protocol!

Do you think it'd be worth going all-in, using something like JSON-RPC? Maybe we could re-use our Schema() stuff, though that might be hard since the transfer protocol is SQL rather than HTTP.

[henn]

Also, for the record, there are some review comments in #621 that might help.

[zenhack]

One obvious possible first step is to replace the NetworkingAction table with something that just has an ID used for ordering and the actual JSON for the JSON rpc. We could process calls on the other side by just doing a first() and then running a schema on it. If/when we needed to have multiple "queues" we could just add a column for that and filter on it. We'll have to keep an eye on the situation to decide when it's time to start shelling out to an existant queue solution, but for the interm I don't think it would be a heck of a lot of work.

The awkward thing with this is that there's still no way to yield responses, but that's at least not a step backward.

I do want to think for a bit as to whether there's an acceptably clean shorter term solution, since the revert_port call is something that folks have found themselves actively wanting.

Also, I would really like others besides just the two of us to weigh in.

I'm going to unassign @kylehogan and assign myself.

[gsilvis]

Hm... this doesn't actually need anything new, as far as I can tell?

The API call would just add a normal networking action, whose destination is determined by what's already in the database.

[gsilvis]

In the longer run, we might want a more fully featured RPC mechanism, but I don't think this feature requires it.

[zenhack]

@gsilvis, what we want here is the ability to just clear the port entirely. Right now we can only remove it from specific channels (by setting new_network to None). I don't see a way to encode what we want in the information the table currently contains. For this one it's preferable not to assume the information in the database is correct. I suppose theoretically, in the case of VLANs, we could just queue up 4Ki NetworkingActions, but...

Again, if there's a simpler change that would enable this I'm game.

Also, when I say @henn's intial suggestion, adding an action and just having the args as a JSON blob, is 60-70% of the way to JSON rpc, I really mean that. If we need to change the info in that table, the other option seems to be playing with SQLAlchemy's inheritence goop, which seems more complex for no particular benefit.

[zenhack]

(Also, I'm not really suggesting we integrate a proper RPC protocol right now. Just, if we use @henn's idea, I want an acknowledgment that that's the road we're going down).

[gsilvis]

Oh, thought we were talking about resetting the port to what's in the database. If you want to clear it all at once... yeah, that's a fundamentally different operation. I'd say, for now, just do it in some hacky way, but it might be a good idea to have a real protocol later.

[gsilvis]

(My other comment is that network operations should always specify the full destination of the operation---most likely as a dictionary. If you do it that way, then clearing is not a special operation at all.)

[zenhack]

I'm actually not 100% on this; @henn, which did we want, just clear the port or reset to the expected state?

Either way, the thinking behind this is that the database's state may be wrong, which means to use the existing operator, we'd actually have to queue up an operation for every possible channel; anything that doesn't have an attachment needs to be explicitly cleared.

And yeah, I agree re: setting the full state. But that's a bigger chunk of work and raises more questions re: how do we get there.

Also, I'm pretty insistent on doing this in the network daemon, and given that we need to change how that table works. I expect any "hacky" thing I came up with would be at least as complex as what @henn suggested.

Here's a possible design:

• Do what @henn suggests, and make the table have a column for ordering, a column for the operation/method, and a JSON blob for parameters.
• define an operator "set_channel" or such that does what the NetworkingActions currently do
• define a "clear" operator that does what we want here.

This also provides a nice evolution path, since we can add new operations and remove old ones (possibly implementing the corresponding api calls in terms of the new ones) as needed.

...and if and when we want to formalize this as an rpc protocol, we're basically there already:

http://www.jsonrpc.org/specification

[shuwens]

I agree with the assumption that database might be wrong.

Doing RPC or the other should be the proper way to have this revert/reset functionality. In this case, the question is what effort do we want to spend on it? Based on my background info, BMI team can assume everything in database is right, it might not be the case that this one is vital.

[zenhack]

Observation: I don't think the reset/revert port method needs any new fields (besides the "method" on which to discriminate). We could just add the one field for now, and defer the converting arguments to JSON thing.

The only thing is that by doing this we're deviating from what we've done with e.g. switches and obms where you have the per-type fields in a separate table. I'm fine with that, but only if everyone is on the same page that this is because we're planning on evolving it towards doing RPC. I certainly don't think we need to do the whole switch now, nor should we.

[zenhack]

So here's a concrete proposal, which I'd like people to comment on; if folks think it's a good idea I'll just go ahead and implement it:

1. Add a type or method or such field to NetworkingAction, which would be something resembling an enum.
2. Have the existing api calls that inject NetworkingActions set type to modify_port (or something similar; we can defer quibbling about the name until review if folks actually care).
3. Have the new api call inject a NetworkingAction with type reset_port

This should be all we need to communicate the necessary information; some things will need to change on the backend, but this solves the communication problem. My plan is to submit just (1) and (2) as a single PR, and do the actual reset_port bit in a separate change, to keep things manual.

@henn, @shwsun, @gsilvis, can you way in on this? I'd like to get started.

[zenhack]

Spoke to @henn yesterday, who said it sounded reasonable, so I'm going to get started.

[zenhack]

Closing this, since #683 has been merged.