sriov-network-device-plugin
sriov-network-device-plugin copied to clipboard
Support for acquiring DDP profile using devlink
This PR introduces support for devlink commands to be used to acquire DDP profile name on supported hardware. First, the devlink will be used, and if it is not supported then it will fallback to DDPTool as it was before.
Some of the devlink related code will be possibly moved to vishvananda/netlink library if the PR there will be accepted.
Later this can be used to add support for DDP profiles in k8snetworkplumbingwg/sriov-network-operator.
LGTM
/cc @SchSeba
@ipatrykx Would you mind updating this PR? I assume some of the util functions have been merged in netlink go library. Thank you!
Hi @zshi-redhat , sure, sorry for the delay on that. I am right now on a tight schedule, but will try to do this until Tuesday.
Pull Request Test Coverage Report for Build 8053076208
Warning: This coverage report may be inaccurate.
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
- For more information on this, see Tracking coverage changes with pull request builds.
- To avoid this issue with future PRs, see these Recommended CI Configurations.
- For a quick fix, rebase this PR at GitHub. Your next report should be accurate.
Details
- -21 of 63 (66.67%) changed or added relevant lines in 3 files are covered.
- 133 unchanged lines in 7 files lost coverage.
- Overall coverage decreased (-0.8%) to 74.83%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| pkg/utils/ddp.go | 0 | 8 | 0.0% |
| pkg/netdevice/pciNetDevice.go | 7 | 20 | 35.0% |
| <!-- | Total: | 42 | 63 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| pkg/devices/gen_net.go | 2 | 86.25% |
| pkg/netdevice/netDeviceProvider.go | 3 | 85.84% |
| pkg/utils/sriovnet_provider.go | 9 | 22.22% |
| pkg/factory/factory.go | 11 | 87.92% |
| pkg/resources/server.go | 19 | 80.51% |
| pkg/netdevice/pciNetDevice.go | 29 | 53.13% |
| pkg/utils/utils.go | 60 | 80.7% |
| <!-- | Total: | 133 |
| Totals | |
|---|---|
| Change from base Build 7377056051: | -0.8% |
| Covered Lines: | 2087 |
| Relevant Lines: | 2789 |
💛 - Coveralls
Hi @zshi-redhat , sure, sorry for the delay on that. I am right now on a tight schedule, but will try to do this until Tuesday.
@zshi-redhat I know, I've said I'll deliver it until Tuesday, but Thursday isn't bad as well, isn't it? 🥳
Anyway, I've rebased this PR on top of the current master, fixed some issues and added unit tests. Hope we can get it merged soon.
@Eoghan1232 @ipatrykx is there a plan to re-kindle this work ?
@Eoghan1232 @ipatrykx is there a plan to re-kindle this work ?
Hi @adrianchiris , yes of course. Looks like just to resolve a merge conflict, will get to it this week :) Then let's get it merged!
@SchSeba @adrianchiris PTAL
As discussed in community, a test is needed to verify if the DDP Tool can be removed from the Dockerfile, as this PR should remove the dependency.
will fix lint issues in a moment.
I am currently testing this PR locally and will also provide update soon
@SchSeba @adrianchiris PTAL
I've updated the PR and validated it now works. I've also fixed lint issue.
I can squash the PR at the end if there are no comments/changes needed.
@SchSeba @adrianchiris I've updated this PR. PTAL
at some point we would need to switch to interfaces here like we did with sriov-network-operator. not related to this PR.
@adrianchiris thanks for the feedback! I've updated the PR, PTAL