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

Create repository to store util packages within NPWG #187

Closed
MichalGuzieniuk opened this issue Jul 5, 2021 · 10 comments
Closed

Create repository to store util packages within NPWG #187

MichalGuzieniuk opened this issue Jul 5, 2021 · 10 comments

Comments

@MichalGuzieniuk
Copy link
Contributor

What issue would you like to bring attention to?

Currently for repositories like SRIOV CNI / SRIOV CNI Operator / Network Resources Injector and Device Plugin there is or will be a set of E2E tests that are using utils from util package. The set of packages inside allows for instance to create / delete K8s objects, read interface details. Those packages are almost similar in implementation between repositories and most probably could be implemented in one place and imported by each component. Beside tests such utils can be used in code if needed.

Some examples:
https://github.com/k8snetworkplumbingwg/network-resources-injector/tree/master/test/util
https://github.com/k8snetworkplumbingwg/sriov-network-operator/tree/master/test/util
https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin/tree/master/pkg/utils
https://github.com/k8snetworkplumbingwg/sriov-cni/tree/master/pkg/utils

What is the impact of this issue?

As a result of storing utils in once place, I see

  • less effort to maintain code
  • clear information about available functions inside utils packages
  • avoid code copy when not imported

Do you have a proposed response or remediation for the issue?

Create repository within NPWG to store utils.

@zshi-redhat
Copy link
Collaborator

/cc @pliurh @martinkennelly @adrianchiris @amorenoz

This was raised in resource management meeting (July 5) and we thought it would be better to create an issue for continued discussion.

I recognize this has been an issue since we have multiple sriov related projects with each needs to perform similar function calls.
This issue was not obvious before, I think partially because it is still manageable to do the import of code from another project, but as we are adding more features (e.g. switchdev), there will be more common functions introduced.

I knew we have several other repos implementing functions as a lib that's easy to share across projects, such as the Mellanox/sriovnet and go-vdpa etc. Would it make sense to consider merging them together under a separate npwg project?
I presume this would benefit us in the long term.

@pliurh
Copy link

pliurh commented Jul 6, 2021

Big +1.

@amorenoz
Copy link
Contributor

amorenoz commented Jul 6, 2021

+1 for merging go-vdpa into a npwg repo

@adrianchiris
Copy link
Contributor

adrianchiris commented Jul 6, 2021

+1 for reducing code duplication, however we need to be careful about what we put in the newly propsed util repo, its structure
and versioning and guidelines on API changes.

RE go-vdpa, do you see it under this util package or a separate repo ?

@pperiyasamy
Copy link

+1 for the proposal.

ovs-cni also have code duplication in w.r.t using caching netConf, we could try to generalise those APIs and make it consumable from both sriov-cni and ovs-cni.

@zshi-redhat
Copy link
Collaborator

+1 for reducing code duplication, however we need to be careful about what we put in the newly propsed util repo, its structure
and versioning and guidelines on API changes.

@adrianchiris do you have something in mind which we may not want to put in the newly proposed repo?

Re repo structure, will the flat rep structure in netlink library be a good model for sriov library or do we have the need to create separate folders or packages for various sriov features such as switchdev, vdpa, general sriov etc?

@adrianchiris
Copy link
Contributor

adrianchiris commented Jul 7, 2021

do you have something in mind which we may not want to put in the newly proposed repo

I dont understand what should end up in this "utils" repo, is it everything that is used in more than one project ?

Re repo structure, will the flat rep structure in netlink library be a good model for sriov library or do we have the need to create separate folders or packages for various sriov features such as switchdev, vdpa, general sriov etc?

if others plan to use the packages/libraries, the convention is to put it into a pkg directory within the project. see[1]
so i dont think we need to do the same as netlink.

[1] https://github.com/golang-standards/project-layout#go-directories

@adrianchiris
Copy link
Contributor

adrianchiris commented Jul 14, 2021

This has been discussed in yesterday's community meeting.

agreement that having a project for common functionality which will be consumed by other projects is desirable.

next steps would be:

  1. decide on a name (proposed names: sriov-utils, resource-mgmt-utils, common-utils, feel free to propose more :))
  2. present idea to NPWG, get approval and create the project
  3. decide on project layout (positive approach in having stable/unstable folder to mark APIs as stable/unstable)
  4. move basic common functionality

@adrianchiris
Copy link
Contributor

I have created the first issue in the new project.

we should continue discussion there.
closing this issue so discussion may continue in the new repository

@adrianchiris
Copy link
Contributor

adrianchiris commented Aug 2, 2021

Inviting everyone to continue discussions in the new repository!

k8snetworkplumbingwg/resource-mgmt-utils#1

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

No branches or pull requests

6 participants