Code review comment for ~yoshikadokawa/charm-nrpe:master-check-lacpdu

> * Partner state - what is considered partner state? Is LACP considered to be
> configured with 2 slaves (max and min number)?
For each underlying interface of an LACP bond, it has an actor state and partner state.
Actor state is the status of the host side, and partner state is the status coming from the switch side. Basically these have to be identical.

> I would like to understand use cases for this change and why the aggregator ID
> mismatch is not enough. On the other hand, there could be more than 2 slaves,
> so I'm not clear on what the "partner" would be in such cases. Or, there are
> deployments that initially use a single interface (no real purpose for bonding
> except for a near future reconfig to add a second slave). Would that mean that
> no "partner" would exist and "WARNING" would be alerted?
In my experience, aggregator ID mismatch is a more critical situation, which is usually one interface is DOWN/Unstable or not configured as LACP, etc.
And of course, the actor port state and partner port state will be in a mismatch as well.
However, there are use cases when aggregator ID are identical for the underlying interfaces, but a mismatch between actor port state and partner port state.
This is often seen with different LACP port rate configuration between host conf and switch conf.
For example, slow(30s) is configured on the switch side and fast(1s) is configured on the host side.

> Once we have use cases (what it could have been spotted with this fix), I'd
> also like to know if these checks could be made optional (ie.
> check_lacp_bond.py --skip-extra-port-state).
Since we already have the aggregator ID mismatch check, I think this can be optional.
I'll update it to make this optional.

> With regard to the code, there are a couple of for loops over the same values
> (list of slaves). Could a single for loop be used?
I did this to separate the output for aggregator ID mismatch and port state mismatch. If aggregator ID mismatch happens, there will be port state mismatch as well, and since the priority for aggregator ID mismatch is higher, I wanted to make sure that the warning message for aggregator ID comes up in Nagios.
However, if I make this port state mismatch check optional, I can put this check into the same for loop, and show the message only if the option is true.

Thank you