search

k8snetworkplumbingwg / ovs-cni

Open vSwitch CNI plugin
Apache License 2.0
224 stars 71 forks source link

Add TCP connection support for OVS #330

Closed tabuhariri closed 1 week ago

tabuhariri commented 2 weeks ago

Example usage: -ovs-socket tcp:127.0.0.1:6640

What this PR does / why we need it:

Add Support for ovs tcp connection

Special notes for your reviewer:

Release note:

This feature allows for remote OVS connections, enhancing flexibility in network configurations.
User will be allowed to use tcp connection to OVS by specifying OVS socket endpoint by using -ovs-socket flag with tcp endpoint "tcp:<IP>:<PORT>".
kubevirt-bot commented 2 weeks ago

Hi @tabuhariri. Thanks for your PR.

I'm waiting for a k8snetworkplumbingwg member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
phoracek commented 2 weeks ago

Hello @tabuhariri, thanks for the contribution and making the code a little cleaner with it.

I will need you so sign the commit - you can do that by running git commit -s --amend and pushing the updated commit to this branch.

tabuhariri commented 2 weeks ago

Hello @tabuhariri, thanks for the contribution and making the code a little cleaner with it.

I will need you so sign the commit - you can do that by running git commit -s --amend and pushing the updated commit to this branch.

Hello Petr, Sure no problem, I've signed the commit and added the release label. While implementing remote OVS connection support, I took the opportunity to refactor the existing code slightly without significantly altering the core logic.

phoracek commented 2 weeks ago

/retest

phoracek commented 2 weeks ago

@tabuhariri sorry, it seems that the e2e test suite is broken (unrelated to your PR). I'll try to look into it ASAP

phoracek commented 1 week ago

/retest

tabuhariri commented 1 week ago

/retest

kubevirt-bot commented 1 week ago

@tabuhariri: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to [this](https://github.com/k8snetworkplumbingwg/ovs-cni/pull/330#issuecomment-2468757716): >/retest Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
tabuhariri commented 1 week ago

/retest-required

kubevirt-bot commented 1 week ago

@tabuhariri: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to [this](https://github.com/k8snetworkplumbingwg/ovs-cni/pull/330#issuecomment-2468759837): >/retest-required Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
tabuhariri commented 1 week ago

@tabuhariri sorry, it seems that the e2e test suite is broken (unrelated to your PR). I'll try to look into it ASAP

I have a potential fix for the e2e test suite: https://github.com/k8snetworkplumbingwg/ovs-cni/pull/334

phoracek commented 1 week ago

/retest

@tabuhariri thanks again for the fix. Let's see if the PR gets automatically based on it, or if you'll have to rebase.

tabuhariri commented 1 week ago

/retest

@tabuhariri thanks again for the fix. Let's see if the PR gets automatically based on it, or if you'll have to rebase.

I really appreciate your prompt responses. Thank you for being so attentive! Just to make it easier on the merge process, I have rebased my commit.

phoracek commented 1 week ago

/retest

Thanks! I appreciate you leaving the code in a better state than you've found it.

phoracek commented 1 week ago

/lgtm /approve

kubevirt-bot commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: phoracek, tabuhariri

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](https://github.com/k8snetworkplumbingwg/ovs-cni/blob/main/OWNERS)~~ [phoracek] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment