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

Vdpa introduction #377

Merged
merged 6 commits into from
Mar 1, 2023
Merged

Vdpa introduction #377

merged 6 commits into from
Mar 1, 2023

Conversation

lmilleri
Copy link
Contributor

This PR is the first step on integrating vDPA in kubernetes containers.

API changes: a new attribute has been added to the policy.
The vdpaType can assume two values: virtio or vhost.
This PR implements the virtio mode only, vhost mode will be implemented in the future.

Supported card: NVIDIA Connect6-DX in switchdev mode (OVS HW offloading enabled)

The NIC is configured via the existing systemd services: switchdev-configuration-before-nm.service and switchdev-configuration-after-nm.service.

Webhook validation has been extended to check vDPA specific static configuration.

Note: this PR is depending on k8snetworkplumbingwg/sriov-network-device-plugin#450, so the go.mod needs to be amended after this PR is merged to master.
Please also consider that this vDPA solution depends on PR ovn-kubernetes/ovn-kubernetes#2664.

@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@coveralls
Copy link

coveralls commented Nov 22, 2022

Pull Request Test Coverage Report for Build 4253702965

  • 24 of 36 (66.67%) changed or added relevant lines in 5 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.7%) to 25.767%

Changes Missing Coverage Covered Lines Changed/Added Lines %
api/v1/helper.go 0 1 0.0%
pkg/utils/switchdev.go 0 1 0.0%
pkg/plugins/generic/generic_plugin.go 13 23 56.52%
Files with Coverage Reduction New Missed Lines %
api/v1/helper.go 1 42.51%
Totals Coverage Status
Change from base Build 4241950765: 0.7%
Covered Lines: 1949
Relevant Lines: 7564

💛 - Coveralls

@lmilleri
Copy link
Contributor Author

/cc @adrianchiris @SchSeba

@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@wizhaoredhat
Copy link
Contributor

Changes to the machine config, needs my changes here:
#381
Otherwise your changes won't apply to the node upon upgrade.

@wizhaoredhat
Copy link
Contributor

/cc @zshi-redhat (for visibility)

@github-actions github-actions bot requested a review from zshi-redhat December 2, 2022 16:14
@SchSeba
Copy link
Collaborator

SchSeba commented Dec 5, 2022

/cc @bn222 @adrianchiris

@github-actions
Copy link

github-actions bot commented Jan 9, 2023

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@wizhaoredhat
Copy link
Contributor

/lgtm

Copy link
Collaborator

@SchSeba SchSeba left a comment

Choose a reason for hiding this comment

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

overall looks great!

I just add some small comments related to the webhook validation we do

api/v1/helper_test.go Show resolved Hide resolved
pkg/webhook/validate.go Outdated Show resolved Hide resolved
pkg/webhook/validate.go Outdated Show resolved Hide resolved
@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

The attribute vdpaType has been added to the policy
It can assume the following values:
- virtio
- vhost (not implemented)

Signed-off-by: Leonardo Milleri <[email protected]>
Signed-off-by: Leonardo Milleri <[email protected]>
@adrianchiris
Copy link
Collaborator

@lmilleri would you like adding a vdpa.md under doc ? :)
just to provide some general overview, requirements (kernel, hardware) plus example configuration of SNO

Signed-off-by: Leonardo Milleri <[email protected]>
@lmilleri
Copy link
Contributor Author

@lmilleri would you like adding a vdpa.md under doc ? :) just to provide some general overview, requirements (kernel, hardware) plus example configuration of SNO

Good idea, will do

@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@lmilleri
Copy link
Contributor Author

@SchSeba @adrianchiris I have just pushed all the requested changes. Please have a look to it again

@adrianchiris
Copy link
Collaborator

/test-all

Copy link
Collaborator

@adrianchiris adrianchiris left a comment

Choose a reason for hiding this comment

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

Thx for addressing my comments @lmilleri LGTM!

lets wait for CI to pass

@lmilleri
Copy link
Contributor Author

/test-all

@lmilleri
Copy link
Contributor Author

@adrianchiris Nvidia e2e test CI is failing. I can't check for details, apparently 13.74.249.42 is not reachable

@adrianchiris
Copy link
Collaborator

/test-all

there is some issue with CI log server (should be solved during next week only).
also the CI machine had outdated go version, this was fixed. retriggering.

Copy link
Collaborator

@SchSeba SchSeba left a comment

Choose a reason for hiding this comment

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

only one small comment

pkg/webhook/validate.go Outdated Show resolved Hide resolved
Signed-off-by: Leonardo Milleri <[email protected]>
Signed-off-by: Leonardo Milleri <[email protected]>
Signed-off-by: Leonardo Milleri <[email protected]>
@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@bn222
Copy link
Collaborator

bn222 commented Mar 1, 2023

/lgtm

@github-actions github-actions bot added the lgtm label Mar 1, 2023
@bn222 bn222 merged commit 13405c5 into k8snetworkplumbingwg:master Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants