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

SRIOV provider: Deploy default CNI #644

Merged
merged 1 commit into from
Aug 11, 2021

Conversation

ormergi
Copy link
Contributor

@ormergi ormergi commented Jul 18, 2021

While back then we decided to deploy Calico CNI on KinD providers
in order to create dual-stack cluster for DEV and CI env's.

As for today we managed to enable dual-stack clusters using our
VM based providers, and eventually removed the IPV6 KinD based provider.
Though we still deploy old version of Calico on the SRIOV lane provider.

We should use the default KinD CNI 'kindnet' instead,
it will reduce complexity of the setup and maintenance.

Signed-off-by: Or Mergi [email protected]

@kubevirt-bot
Copy link
Contributor

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 kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jul 18, 2021
@kubevirt-bot kubevirt-bot requested review from dhiller and rmohr July 18, 2021 10:35
@ormergi
Copy link
Contributor Author

ormergi commented Jul 18, 2021

/test check-up-kind-1.19-sriov

@ormergi ormergi force-pushed the sriov-provider-default-cni branch from 5108c22 to 427c6ce Compare July 18, 2021 10:37
@ormergi
Copy link
Contributor Author

ormergi commented Jul 18, 2021

/test check-up-kind-1.19-sriov

@ormergi
Copy link
Contributor Author

ormergi commented Jul 18, 2021

/test check-up-kind-1.19-sriov

@ormergi ormergi force-pushed the sriov-provider-default-cni branch from 9a9367d to 427c6ce Compare July 18, 2021 16:07
ormergi added a commit to ormergi/project-infra that referenced this pull request Jul 19, 2021
Test SRIOV provider with kindnet CNI instead of Calico
and run conformance tests.

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/project-infra that referenced this pull request Jul 20, 2021
Test SRIOV provider with kindnet CNI instead of Calico
and run conformance tests.

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/project-infra that referenced this pull request Jul 20, 2021
Test SRIOV provider with kindnet CNI instead of Calico
and run conformance tests.

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/project-infra that referenced this pull request Jul 20, 2021
Test kubevirt SRIOV tests when deploying KinD
default CNI plugin, 'kindnet', instead of Calico.

Signed-off-by: Or Mergi <[email protected]>
Copy link
Member

@EdDev EdDev left a comment

Choose a reason for hiding this comment

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

Very nice!

cluster-up/cluster/kind/common.sh Show resolved Hide resolved
@ormergi ormergi force-pushed the sriov-provider-default-cni branch from 427c6ce to 10ff3e6 Compare August 5, 2021 16:21
@kubevirt-bot kubevirt-bot added size/M and removed size/S labels Aug 5, 2021
@ormergi ormergi marked this pull request as ready for review August 5, 2021 16:22
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 5, 2021
@kubevirt-bot kubevirt-bot requested a review from enp0s3 August 5, 2021 16:22
@ormergi ormergi force-pushed the sriov-provider-default-cni branch from 10ff3e6 to 39b4f0e Compare August 5, 2021 16:27
ormergi added a commit to ormergi/project-infra that referenced this pull request Aug 5, 2021
Test SRIOV provider with kindnet CNI instead of Calico
and run conformance tests.

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/project-infra that referenced this pull request Aug 6, 2021
Test SRIOV provider with kindnet CNI instead of Calico
and run conformance tests.

Signed-off-by: Or Mergi <[email protected]>
@ormergi
Copy link
Contributor Author

ormergi commented Aug 10, 2021

@EdDev I addressed you review comment could you please take another look?

Copy link
Member

@EdDev EdDev left a comment

Choose a reason for hiding this comment

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

Does this change mean we do not deploy dual-stack anymore on the SR-IOV lane? I could not understand that part from the commit message.

And if not, does kindnet support it (so we can add it)?

@ormergi
Copy link
Contributor Author

ormergi commented Aug 10, 2021

Does this change mean we do not deploy dual-stack anymore on the SR-IOV lane? I could not understand that part from the commit message.

Yes its single stack now,
The test that checks connectivity between VM's though SRIOV interface with IPv6 is green.

And if not, does kindnet support it (so we can add it)?

In general kindnet supports dual-stack

@EdDev
Copy link
Member

EdDev commented Aug 10, 2021

In general kindnet supports dual-stack

If it is easily addable, then can we do it now? It is not a must, but it will be nicer so we will not need to think about it in the future.

@ormergi
Copy link
Contributor Author

ormergi commented Aug 10, 2021

If it is easily addable, then can we do it now? It is not a must, but it will be nicer so we will not need to think about it in the future.

With the new Kind API its pretty straight forward https://kind.sigs.k8s.io/docs/user/configuration/#networking
the problem is that it supported on k8s-1.20+ clusters.
Due to the fact we still use PodPreset (deprecated on k8s-1.20) we cant upgrade the provider now.
As soon as kubernetes-sigs/kind#2321 will get in we will be able to upgrade the provider,
stop using PodPreset and eventually turn on dual-stack.

Copy link
Member

@EdDev EdDev left a comment

Choose a reason for hiding this comment

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

Thanks!

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2021
Copy link
Member

@EdDev EdDev left a comment

Choose a reason for hiding this comment

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

ohh, what about ./cluster-up/cluster/kind/manifests/kube-calico.yaml.in?

EDIT: Also ./cluster-up/cluster/kind/manifests/kube-calico.diff.in

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2021
While back then we decided to deploy Calico CNI on KinD providers
in order to create dual-stack cluster for DEV and CI env's.

As for today we managed to enable dual-stack clusters using our
VM based providers, and eventually removed the IPV6 KinD based provider.
Though we still deploy old version of Calico on Kind infra providers.

We should use the default KinD CNI 'kindnet' instead,
it will reduce complexity of the setup and maintenance.

Signed-off-by: Or Mergi <[email protected]>
@ormergi ormergi force-pushed the sriov-provider-default-cni branch from 39b4f0e to 9a80eeb Compare August 11, 2021 08:09
@ormergi
Copy link
Contributor Author

ormergi commented Aug 11, 2021

ohh, what about ./cluster-up/cluster/kind/manifests/kube-calico.yaml.in?

EDIT: Also ./cluster-up/cluster/kind/manifests/kube-calico.diff.in

@EdDev Done

@ormergi ormergi requested a review from EdDev August 11, 2021 08:10
Copy link
Member

@EdDev EdDev left a comment

Choose a reason for hiding this comment

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

Thanks!

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2021
Copy link
Contributor

@enp0s3 enp0s3 left a comment

Choose a reason for hiding this comment

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

@ormergi Thank you!
/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enp0s3

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:

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 11, 2021
@kubevirt-bot kubevirt-bot merged commit 9c1261b into kubevirt:main Aug 11, 2021
oshoval added a commit to oshoval/kubevirt that referenced this pull request Aug 30, 2021
```
2c2862b cnao, Bump CNAO to 0.58.0 on all providers (kubevirt/kubevirtci#664)
ba05421 cluster-provision, Prefetch cdi images (kubevirt/kubevirtci#660)
71d8ce5 Bump k8s providers (kubevirt/kubevirtci#661)
0007793 cdi, Bump cdi to 1.36.0 on k8s-1.21 (kubevirt/kubevirtci#659)
9c1261b Kind infra providers: Deploy default CNI (kubevirt/kubevirtci#644)
```

Signed-off-by: Or Shoval <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants