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.

Sign up for GitHub

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

Jump to bottom

Create kind-1.23-sriov provider #897

Merged

kubevirt-bot merged 3 commits into kubevirt:main from orelmisan:kind-1.23-sriov

Nov 14, 2022

Member

orelmisan commented Nov 10, 2022 •

edited

Loading

Create an SR-IOV provider, based on kind running K8s v1.23.

There is a problem using kind with K8s 1.24 and above, so an upgrade to one K8s version should keep us in the latest three K8s versions until K8s 1.26 will be released.


          Create kind-1.23-sriov based on kind-1.22-sriov

e990420

Copy the `kind-1.22-sriov` directory to `kind-1.23-sriov`,
in order for it to use as a base.

Signed-off-by: Orel Misan <[email protected]>

Contributor

kubevirt-bot commented Nov 10, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

kubevirt-bot added do-not-merge/work-in-progress dco-signoff: yes labels

kubevirt-bot requested review from oshoval and rmohr

November 10, 2022 09:52

kubevirt-bot added the size/XXL label

Member Author

orelmisan commented Nov 10, 2022

/uncc @rmohr @oshoval

kubevirt-bot removed request for rmohr and oshoval

November 10, 2022 09:52

orelmisan mentioned this pull request

images_to_mirror: Add kind node image v1.23.13 kubevirt/project-infra#2446

Merged

orelmisan added a commit to orelmisan/project-infra that referenced this pull request


          images_to_mirror: Add kind node image v1.23.13

5b7bfa9

Mirror kind node image v1.23.13 in order to use it in kubevirtci
kind-1.23-sriov provider [1].

[1] kubevirt/kubevirtci#897

Signed-off-by: Orel Misan <[email protected]>

orelmisan added a commit to orelmisan/project-infra that referenced this pull request


          kubevirtci, pre-submit: Clone kind-1.22-sriov job

05a7663

PR kubevirt/kubevirtci#897 adds a `kind-1.23-sriov` provider.

In order to test it, add an optional pre-submit job.

Clone the `kind-1.22-sriov` pre-submit job, in order to serve as
a base for `kind-1.23-sriov` pre-submit job.

Signed-off-by: Orel Misan <[email protected]>

orelmisan mentioned this pull request

kubevirtci, presubmit: Add optional job to check kind-1.23-sriov kubevirt/project-infra#2447

Merged

kubevirt-bot pushed a commit to kubevirt/project-infra that referenced this pull request


          images_to_mirror: Add kind node image v1.23.13 (#2446)

8a7075a

Mirror kind node image v1.23.13 in order to use it in kubevirtci
kind-1.23-sriov provider [1].

[1] kubevirt/kubevirtci#897

Signed-off-by: Orel Misan <[email protected]>

Signed-off-by: Orel Misan <[email protected]>


          kind-1.23-sriov: Use K8s version 1.23.13

41c6f7b

Upgrade kind version to current latest v0.17.0.
Upgrade node image to 1.23.13.

Signed-off-by: Orel Misan <[email protected]>

orelmisan force-pushed the kind-1.23-sriov branch from e1bc112 to 41c6f7b Compare

November 10, 2022 10:19

Member Author

orelmisan commented Nov 10, 2022

Change: Use mirrored node image.


          kind-1.23-sriov: Update REAMDE.md

3fd6029

Signed-off-by: Orel Misan <[email protected]>

kubevirt-bot pushed a commit to kubevirt/project-infra that referenced this pull request


          kubevirtci, presubmit: Add optional job to check kind-1.23-sriov (#2447)

032dcc6

* kubevirtci, pre-submit: Clone kind-1.22-sriov job

PR kubevirt/kubevirtci#897 adds a `kind-1.23-sriov` provider.

In order to test it, add an optional pre-submit job.

Clone the `kind-1.22-sriov` pre-submit job, in order to serve as
a base for `kind-1.23-sriov` pre-submit job.

Signed-off-by: Orel Misan <[email protected]>

* kubevirtci, pre-submit: Add optional job to check kind-1.23-sriov

Make the kind-1.23-sriov pre-submit job optional.

Signed-off-by: Orel Misan <[email protected]>

Signed-off-by: Orel Misan <[email protected]>

Member Author

orelmisan commented Nov 10, 2022

/test check-up-kind-1.23-sriov

4 similar comments

Member Author

orelmisan commented Nov 10, 2022

/test check-up-kind-1.23-sriov

Member Author

orelmisan commented Nov 10, 2022

/test check-up-kind-1.23-sriov

Member Author

orelmisan commented Nov 10, 2022

/test check-up-kind-1.23-sriov

Member Author

orelmisan commented Nov 11, 2022

/test check-up-kind-1.23-sriov

orelmisan changed the title ~~[WIP] Create kind-1.23-sriov provider~~ Create kind-1.23-sriov provider

orelmisan marked this pull request as ready for review

November 13, 2022 06:39

kubevirt-bot removed the do-not-merge/work-in-progress label

kubevirt-bot requested review from jean-edouard and stu-gott

November 13, 2022 06:39

kubevirt-bot requested a review from oshoval

November 13, 2022 06:40

oshoval reviewed

View reviewed changes

cluster-up/cluster/kind-1.23-sriov/certcreator/certlib/selfsign.go

		@@ -0,0 +1,115 @@
		package certlib

Contributor

oshoval Nov 13, 2022 •

edited

Loading

Please remove the whole folder of cert creator, we don't use it for long time already IIRC

Member Author

orelmisan Nov 13, 2022

I see multiple references to this directory:
https://github.com/kubevirt/kubevirtci/pull/897/files#diff-ba18cea87edeee1e4c652aaa05ccd4ac663d4b4545d892c9a1db306c6b5b9bb0R6

https://github.com/kubevirt/kubevirtci/pull/897/files#diff-ba18cea87edeee1e4c652aaa05ccd4ac663d4b4545d892c9a1db306c6b5b9bb0R182-R195

Contributor

oshoval Nov 13, 2022 •

edited

Loading

but we dont deploy sriov_operator.sh anymore, nor maintain it
so it uses a very old version
we can remove that as well please imo

Member Author

orelmisan Nov 13, 2022

I suggest we remove it in a follow-up PR.
I don't think it helps the reviewer to see that removal in this PR.

Contributor

oshoval Nov 13, 2022 •

edited

Loading

Imo an additional commit that removes it can be clean and easy enough to review,
I don't see why to take the files with us again and again.
It will be more self contained.

but whatever you prefer

Member Author

orelmisan Nov 13, 2022

I prefer to handle this change in a follow up PR.
I think there are more leftovers, for example:
https://github.com/kubevirt/kubevirtci/pull/897/files#diff-c5f5c4a7c70b52eb6c6561e5629101c50defa39f231960e5a5d4751f4152c82dR1

Contributor

oshoval Nov 13, 2022 •

edited

Loading

I honestly don't see a reason to postpone it, it will make this PR not clean enough
It should be pretty trivial to clean all needed, as it is not used
maybe worth to at least try first ?

we can also wait to see more opinions

Note that for me personally it will be easier to review it cleaner
but i won't block for it

Member Author

orelmisan Nov 13, 2022

I think there are more leftovers that we are not aware of.
Starting a cleanup operation could be an adventure that I don't want to get into at the moment.
Additionally I think that there could be multiple small improvements that could be made in order to improve the code.
But, they all could be done in follow-up PRs.

Contributor

oshoval Nov 13, 2022 •

edited

Loading

The sriov operator scope should be few files that you just delete and forgot
for example the script / yamls and the cert lib
imo it is right to at least try it, (and if you see it is adventure drop it) but again this is just my 2 cents
rest of the cleaning can be done of course on follow PR

EDIT - from a glance, this is all that need to be removed related to the sriov operator

Contributor

ormergi Nov 13, 2022

I would remove what I can, but I won't insist that it will be part of this PR.

Contributor

oshoval commented Nov 13, 2022

It was tested with all the SRIOV tests and kubevirt right ?

Beside the comments above lgtm

Member Author

orelmisan commented Nov 13, 2022

It was tested with all the SRIOV tests and kubevirt right ?

Beside the comments above lgtm

Thank you.
AFAIU this provider is only used for the SR-IOV e2e tests.
@ormergi Please correct me if I'm wrong.

Contributor

oshoval commented Nov 13, 2022 •

edited

Loading

It was tested with all the SRIOV tests and kubevirt right ?
Beside the comments above lgtm

Thank you. AFAIU this provider is only used for the SR-IOV e2e tests. @ormergi Please correct me if I'm wrong.

It is used for only those, i meant to try the kubevirt SR-IOV e2e tests with it before we merge it
(maybe you already did, just making sure)

Contributor

ormergi commented Nov 13, 2022

It looks like there is a check-up job for this new provider, but did it took this PR changes?

Member Author

orelmisan commented Nov 13, 2022 •

edited

Loading

I ran the SR-IOV e2e tests numerous times on a bare-metal server with an SR-IOV NIC and this provider.
I've added a job to project-infra to test this provider, and ran it five times.

Contributor

oshoval commented Nov 13, 2022 •

edited

Loading

It looks like there is a check-up job for this new provider, but did it took this PR changes?

Yes as it uses the right KUBEVIRT_PROVIDER
https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_kubevirtci/897/check-up-kind-1.23-sriov/1591002280692289536#1:build-log.txt%3A109

Contributor

oshoval commented Nov 13, 2022

I ran the SR-IOV e2e tests numerous times on a bare-metal server with an SR-IOV NIC and this provider. I've added a job to project-infra to test this provider, and ran it five times.

Thanks
just note that the mention job doesn't check kubevirt SR-IOV e2e tests, but those you ran manually so all is ok
(maybe this what you meant)

Member Author

orelmisan commented Nov 13, 2022

I ran the SR-IOV e2e tests numerous times on a bare-metal server with an SR-IOV NIC and this provider. I've added a job to project-infra to test this provider, and ran it five times.

Thanks just note that the mention job doesn't check kubevirt SR-IOV e2e tests, but those you ran manually so all is ok (maybe this what you meant)

Isn't this line indicating the execution of the e2e SR-IOV test suite?
https://github.com/kubevirt/project-infra/pull/2447/files#diff-ab108ff0d13755bd671fe2f0a2397f0c0290dc6655d2a03659b2cee3fa5958f7R135

Contributor

ormergi commented Nov 13, 2022 •

edited

Loading

I ran the SR-IOV e2e tests numerous times on a bare-metal server with an SR-IOV NIC and this provider. I've added a job to project-infra to test this provider, and ran it five times.

Thanks just note that the mention job doesn't check kubevirt SR-IOV e2e tests, but those you ran manually so all is ok (maybe this what you meant)

Isn't this line indicating the execution of the e2e SR-IOV test suite? https://github.com/kubevirt/project-infra/pull/2447/files#diff-ab108ff0d13755bd671fe2f0a2397f0c0290dc6655d2a03659b2cee3fa5958f7R135

It does run SR-IOV tests using sonobouy and kubevirt plugin.

ormergi approved these changes

View reviewed changes

kubevirt-bot assigned ormergi

kubevirt-bot added the lgtm label

Contributor

oshoval commented Nov 13, 2022 •

edited

Loading

Right, two here are skipped, they should be skipped ?
They are not skipped on kubevirt, just on conformance (or at least we do run 13 on kubevirt e2e but 11 here)

<testcase name="[Serial]SRIOV VMI connected to single SRIOV network should have cloud-init meta_data with aligned cpus to sriov interface numa node for VMIs with dedicatedCPUs" classname="Tests Suite" time="5.673313703">
<skipped/>
</testcase>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network should have cloud-init meta_data with tagged sriov nics" classname="Tests Suite" time="58.596065772"/>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network [test_id:1754]should create a virtual machine with sriov interface" classname="Tests Suite" time="33.614782728"/>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network [test_id:1754]should create a virtual machine with sriov interface with all pci devices on the root bus" classname="Tests Suite" time="35.19714611"/>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network [test_id:3959]should create a virtual machine with sriov interface and dedicatedCPUs" classname="Tests Suite" time="5.644881961">
<skipped/>
</testcase>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network [test_id:3985]should create a virtual machine with sriov interface with custom MAC address" classname="Tests Suite" time="36.356837491"/>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network migration should be successful with a running VMI on the target" classname="Tests Suite" time="57.725623362"/>
<testcase name="[Serial]SRIOV VMI connected to two SRIOV networks [test_id:1755]should create a virtual machine with two sriov interfaces referring the same resource" classname="Tests Suite" time="41.812583509"/>
<testcase name="[Serial]SRIOV Connected to multiple SRIOV networks should correctly plug all the interfaces based on the specified MAC and (guest) PCI addresses" classname="Tests Suite" time="55.549018169"/>
<testcase name="[Serial]SRIOV VMI connected to link-enabled SRIOV network [test_id:3956]should connect to another machine with sriov interface over IPv4" classname="Tests Suite" time="48.666218362"/>
<testcase name="[Serial]SRIOV VMI connected to link-enabled SRIOV network [test_id:3957]should connect to another machine with sriov interface over IPv6" classname="Tests Suite" time="48.307891759"/>
<testcase name="[Serial]SRIOV VMI connected to link-enabled SRIOV network With VLAN should be able to ping between two VMIs with the same VLAN over SRIOV network" classname="Tests Suite" time="40.644563216"/>
<testcase name="[Serial]SRIOV VMI connected to link-enabled SRIOV network With VLAN should NOT be able to ping between Vlaned VMI and a non Vlaned VMI" classname="Tests Suite" time="44.26897302"/>

Contributor

ormergi commented Nov 13, 2022 •

edited

Loading

Right, two here are skipped, they should be skipped ?

<testcase name="[Serial]SRIOV VMI connected to single SRIOV network should have cloud-init meta_data with aligned cpus to sriov interface numa node for VMIs with dedicatedCPUs" classname="Tests Suite" time="5.673313703">
<skipped/>
</testcase>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network should have cloud-init meta_data with tagged sriov nics" classname="Tests Suite" time="58.596065772"/>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network [test_id:1754]should create a virtual machine with sriov interface" classname="Tests Suite" time="33.614782728"/>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network [test_id:1754]should create a virtual machine with sriov interface with all pci devices on the root bus" classname="Tests Suite" time="35.19714611"/>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network [test_id:3959]should create a virtual machine with sriov interface and dedicatedCPUs" classname="Tests Suite" time="5.644881961">
<skipped/>
</testcase>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network [test_id:3985]should create a virtual machine with sriov interface with custom MAC address" classname="Tests Suite" time="36.356837491"/>
<testcase name="[Serial]SRIOV VMI connected to single SRIOV network migration should be successful with a running VMI on the target" classname="Tests Suite" time="57.725623362"/>
<testcase name="[Serial]SRIOV VMI connected to two SRIOV networks [test_id:1755]should create a virtual machine with two sriov interfaces referring the same resource" classname="Tests Suite" time="41.812583509"/>
<testcase name="[Serial]SRIOV Connected to multiple SRIOV networks should correctly plug all the interfaces based on the specified MAC and (guest) PCI addresses" classname="Tests Suite" time="55.549018169"/>
<testcase name="[Serial]SRIOV VMI connected to link-enabled SRIOV network [test_id:3956]should connect to another machine with sriov interface over IPv4" classname="Tests Suite" time="48.666218362"/>
<testcase name="[Serial]SRIOV VMI connected to link-enabled SRIOV network [test_id:3957]should connect to another machine with sriov interface over IPv6" classname="Tests Suite" time="48.307891759"/>
<testcase name="[Serial]SRIOV VMI connected to link-enabled SRIOV network With VLAN should be able to ping between two VMIs with the same VLAN over SRIOV network" classname="Tests Suite" time="40.644563216"/>
<testcase name="[Serial]SRIOV VMI connected to link-enabled SRIOV network With VLAN should NOT be able to ping between Vlaned VMI and a non Vlaned VMI" classname="Tests Suite" time="44.26897302"/>

From what I remember they are skipped because Kubevirt tests perform some stuff the plugin won't, lemme check..

EDIT: Maybe it's Kubevirt CPUMannager feature gate that is missing.
It's not related to this PR changes though.

Member Author

orelmisan commented Nov 13, 2022

/assign @qinqon

kubevirt-bot assigned qinqon

Contributor

oshoval commented Nov 13, 2022

Please ref the kind issue to the PR desc

Contributor

oshoval commented Nov 13, 2022

From what I remember they are skipped because Kubevirt tests perform some stuff the plugin won't, lemme check..

EDIT: Maybe it's Kubevirt CPUMannager feature gate that is missing. It's not related to this PR changes though.

Not worth to add the feature gate / solve it if its something that make sense ?
even on a follow PR

Member Author

orelmisan commented Nov 13, 2022

Please ref the kind issue to the PR desc

Done.

Contributor

qinqon commented Nov 14, 2022

/lgtm
/approve

Contributor

kubevirt-bot commented Nov 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [qinqon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kubevirt-bot added the approved label

kubevirt-bot merged commit 2f57cdf into kubevirt:main

orelmisan deleted the kind-1.23-sriov branch

November 14, 2022 09:11

kubevirt-bot mentioned this pull request

Bump kubevirtci kubevirt/kubevirt#8688

Merged

kubevirt-bot added a commit to kubevirt-bot/kubevirt that referenced this pull request


          Bump kubevirtci

63350d0

[021efaa Revert "bump: k8s-1.2[4,5] to cnao v0.81.0](https://github.com/kubevirt/kubevirtci/pull/895)"](https://github.com/kubevirt/kubevirtci/pull/904)
[75a155b vm based providers: Change default dns host port](kubevirt/kubevirtci#903)
[71777d3 Support centos9 provisioner ](kubevirt/kubevirtci#896)
[3ddc5b7 Run bazelisk run //robots/cmd/kubevirtci-bumper:kubevirtci-bumper -- -ensure-last-three-minor-of v1 --k8s-provider-dir /home/prow/go/src/github.com/kubevirt/project-infra/../kubevirtci/cluster-provision/k8s](kubevirt/kubevirtci#898)
[765bf0b kind-1.23-sriov: Remove SR-IOV operator leftovers](kubevirt/kubevirtci#899)
[2f57cdf Create kind-1.23-sriov provider](kubevirt/kubevirtci#897)
[dd31ea6 bump: k8s-1.2[4,5] to cnao v0.81.0](kubevirt/kubevirtci#895)
[fa031f8 Bump default provider version](kubevirt/kubevirtci#894)
[8cca8c0 vm based providers: Expose a UDP port for DNS](kubevirt/kubevirtci#867)
[2fea446 kind-1.22-sriov, provider.sh: Remove unused SRIOV_TESTS_NS variable](kubevirt/kubevirtci#891)

```release-note
NONE
```

Signed-off-by: kubevirt-bot <[email protected]>

ormergi mentioned this pull request

Add K8s-1.23 SR-IOV lane jobs kubevirt/project-infra#2477

Merged

Barakmor1 pushed a commit to Barakmor1/kubevirt that referenced this pull request


          Bump kubevirtci

6af2e31

[021efaa Revert "bump: k8s-1.2[4,5] to cnao v0.81.0](https://github.com/kubevirt/kubevirtci/pull/895)"](https://github.com/kubevirt/kubevirtci/pull/904)
[75a155b vm based providers: Change default dns host port](kubevirt/kubevirtci#903)
[71777d3 Support centos9 provisioner ](kubevirt/kubevirtci#896)
[3ddc5b7 Run bazelisk run //robots/cmd/kubevirtci-bumper:kubevirtci-bumper -- -ensure-last-three-minor-of v1 --k8s-provider-dir /home/prow/go/src/github.com/kubevirt/project-infra/../kubevirtci/cluster-provision/k8s](kubevirt/kubevirtci#898)
[765bf0b kind-1.23-sriov: Remove SR-IOV operator leftovers](kubevirt/kubevirtci#899)
[2f57cdf Create kind-1.23-sriov provider](kubevirt/kubevirtci#897)
[dd31ea6 bump: k8s-1.2[4,5] to cnao v0.81.0](kubevirt/kubevirtci#895)
[fa031f8 Bump default provider version](kubevirt/kubevirtci#894)
[8cca8c0 vm based providers: Expose a UDP port for DNS](kubevirt/kubevirtci#867)
[2fea446 kind-1.22-sriov, provider.sh: Remove unused SRIOV_TESTS_NS variable](kubevirt/kubevirtci#891)

```release-note
NONE
```

Signed-off-by: kubevirt-bot <[email protected]>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-signoff: yes lgtm size/XXL