Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ibKubernetesEnabled config #38

Merged
merged 1 commit into from
May 29, 2020

Conversation

zshi-redhat
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented May 28, 2020

Pull Request Test Coverage Report for Build 117

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 42.715%

Totals Coverage Status
Change from base Build 86: 0.0%
Covered Lines: 214
Relevant Lines: 501

💛 - Coveralls

README.md Outdated
* `ipam` (dictionary, optional): IPAM configuration to be used for this network, `dhcp` is not supported.
* `link_state` (string, optional): Enforces link state for the VF. Allowed values: auto, enable, disable.
* `rdmaIsolation` (boolean, optional): Enable RDMA network namespace isolation for RDMA workloads. More information
about the system requirements to support this mode of operation can be found [here](https://github.com/Mellanox/rdma-cni)
* `cniArgGUID` (bool, optional): Enforces ib-sriov-cni to get VF GUID from CNI-Args["mellanox.infiniband.app"], default to false. CNI-Args["mellanox.infiniband.app"] is configured by [ib-kubernetes](https://www.github.com/Mellanox/ib-kubernetes).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

working with ib-kubernetes we still allow to get GUID from CNI args. I think we need to change the name to IBKubernetesEnabled or something similar

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -39,14 +39,18 @@ func init() {
}

func getGUIDFromConf(netConf *localtypes.NetConf) (string, error) {
// Take from CNI_ARGS if available
if netConf.CniArgGUID {
if guid, ok := netConf.Args.CNI["guid"]; ok {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you need to move the RuntimeConfig.InfinibandGUID to here as well (Line 50). We will use this once InfinibandGUID will be approved by the network-att-def same as mac see [1]

[1] https://docs.google.com/document/d/1V0R_MEiPapZOD_SuBjzTOdGYqILBFovuy1xacYCqFHg/edit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.
I thought with ib-kubernetes, it only allocates GUID for VF device, but it actually respect what's allocated in CNI_ARGS["guid"].

@zshi-redhat zshi-redhat changed the title add CniArgGUID config add ibKubernetesEnabled config May 29, 2020
LinkState string `json:"link_state,omitempty"` // auto|enable|disable
RdmaIso bool `json:"rdmaIsolation,omitempty"`
IBKubernetesEnabled bool `json:"ibKubernetesEnabled,omitempty"`
RdmaNetState rdmatypes.RdmaNetState
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gofmt error:
pkg/types/types.go:27: File is not gofmt-ed with -s (gofmt)
RdmaNetState rdmatypes.RdmaNetState
Args struct {
Makefile:74: recipe for target 'lint' failed
make: *** [lint] Error 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@moshe010 moshe010 merged commit 8688c4b into k8snetworkplumbingwg:master May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants