DAOS-4988 control: Provide client with same class network device

[jbrosenz]

    daos_server now examines the network device class that the
    daos_io_server is using and shares that with the
    daos_agent via GetAttachInfo.

    daos_agent now filters the network devices available for
    automatic assignment to clients to avoid assigning clients
    to networks that may not be compatible with the
    daos_io_server.

    In practice, this means that if the daos_io_server is using
    network device ib0, an Infiniband device, the agent will
    assign an Infiniband device.  If the daos_io_server is
    using eth0, an Ethernet device, the agent will assign an
    Ethernet device.

Signed-off-by: Joel Rosenzweig <joel.b.rosenzweig@intel.com>

[mjmac]

The "meat" of the change is overall OK, some small adjustments needed. It appears that you need to fix your versions of protobuf and friends, though.

For future reference, the fact that there are so many changes to generated protobuf files that have nothing to do with your changes is a strong indicator that something is amiss and that you probably need to fix something before committing/pushing.

[jbrosenz]

I forgot to run through gofmt. I'll fix that and update.

[jbrosenz]

Yeah, so I was too trigger happy. I broke unit tests with the latest changes and merrily forgot all about them. I'll fix that and then push again. Sorry for the spam.

[daosbuild1]

Test stage run_test.sh completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2894/2/execution/node/391/log

[mjmac]

Have you also verified that your version of protoc-gen-go is correct?

[daosbuild1]

Test stage run_test.sh completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-2894/6/display/redirect

[mjmac]

LGTM, though I remain concerned that the lack of automated test coverage in this area is likely to result in continued defect reports.

[jbrosenz]

LGTM, though I remain concerned that the lack of automated test coverage in this area is likely to result in continued defect reports.

We shouldn't land it if you do not like it. So let's discuss adding in an appropriate amount of automated test coverage. These are the highlights that are occurring in this PR. 1) detecting the network device class, 2) sharing that with the agent via the GetAttachInfo gRPC, 3) having the agent filter the devices added to the attach info cache based on the network device class provided, and 4) validating that each IO server instance is configured with, and starts with the same device class.

Do you have concerns with all 4 areas, or something in particular that would benefit from some unit testing?

[daosbuild1]

Test stage Functional completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-2894/7/display/redirect

[mjmac]

We shouldn't land it if you do not like it.

It's not that I don't like it. It's that I would be happier about landing it if there were a lot more tests demonstrating that it works now and will continue to work after future refactoring and other changes. Tests are also a good way to programmatically express requirements and explore possibilities before they're encountered in the real world.

So let's discuss adding in an appropriate amount of automated test coverage. These are the highlights that are occurring in this PR. 1) detecting the network device class, 2) sharing that with the agent via the GetAttachInfo gRPC, 3) having the agent filter the devices added to the attach info cache based on the network device class provided, and 4) validating that each IO server instance is configured with, and starts with the same device class.

Do you have concerns with all 4 areas, or something in particular that would benefit from some unit testing?

1. Is tricky, given that it requires interaction with the system unless you're using stubs, and then you're just testing the stubs. The best way to handle this kind of thing is to keep the untestable stuff as separate from the "business logic" as possible. In the case of the netdetect library, this might mean changing things around a bit to pass things like device affinity into createFabricScanEntry() so that a set of table-driven unit tests can explore possibilities with a variety of synthetic inputs. This might arguably be outside the scope of this PR and I'm not insisting on it. I would suggest opening another low-priority ticket to address it as a background task, though.

2. can be accomplished with a new TestServer_MgmtSvc_GetAttachInfo set of tests. I was going to suggest that you could just add on to what's already there, but I just looked and realized that no one ever added a set of test cases for that RPC handler. It's an ancient one that predates my time here. It could go alongside of TestServer_MgmtSvc_LeaderQuery and draw inspiration from it.

3. looks pretty straightforward, unless I'm missing something... You already have some test coverage for the attachInfoCache, so adding a scenario to verify that the filtering works as expected should be pretty easy.

For 4), you could add a test to server/config_test.go to validate that a config with two different interface types fails validation. You added the ability to stub out the real GetDeviceClass(), so this should be trivial. I don't think we need to go further than that.

[kjacque]

In general, I agree with Mike's testing comments. If you don't want to address it now, I think it should be done as a follow-on.

[daosbuild1]

Test stage Functional_Hardware_Large completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2894/10/execution/node/805/log

[kjacque]

Thanks for adding the testing, Joel. Overall it looks good, I still have a few comments about naming but they are minor, and could be addressed in a follow-on PR.

[kjacque]

Looks good!

[daosbuild1]

Test stage Functional completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-2894/11/testReport/(root)/

[daosbuild1]

Test stage Functional_Hardware_Large completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2894/11/execution/node/805/log

[jbrosenz]

The two failures are https://jira.hpdd.intel.com/browse/CART-900 and https://jira.hpdd.intel.com/browse/DAOS-5186