Add PodUID to the OVSDB #311

jiayoukun · 2024-05-21T10:28:34Z

What this PR does / why we need it:

By adding the UID of a Pod to the external_ids field of the corresponding veth interface in the OVSDB, it becomes convenient for the Pod object to query the OVSDB using the UID to find the vethName of the Pod on the host machine, and then deliver OVS flow tables accordingly.

Currently, the host machine's veth interface in the OVSDB only stores the Netns field, which is useful, but for Pod objects, their netns is not publicly accessible. This change addresses this limitation by allowing Pod objects to query the OVSDB directly using their UID.

Special notes for your reviewer:

Release note:

Added the Pod UID to the external_ids field of the corresponding veth interface in the OVSDB.
This change allows querying the vethName of a Pod on the host machine using its UID, improving the ability to deliver OVS flow tables for Pod objects.

kubevirt-bot · 2024-05-21T10:28:45Z

Hi @jiayoukun. 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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

jiayoukun · 2024-05-21T10:37:21Z

fix #309

adrianchiris · 2024-05-22T11:02:11Z

pkg/plugin/plugin.go

-	OvnPort cnitypes.UnmarshallableString `json:"ovnPort,omitempty"`
+	MAC         cnitypes.UnmarshallableString `json:"mac,omitempty"`
+	OvnPort     cnitypes.UnmarshallableString `json:"ovnPort,omitempty"`
+	K8S_POD_UID cnitypes.UnmarshallableString


there is no assurance runtime will provide this value (altohugh for k8s we do get this information passed down by the runtime)

so in case this arg is not provided should we store an empty value ? or just avoid adding this key to external-ids ?

While I do not believe it is critical, I would prefer not to have an empty value inside the external_id section of the port. Ideally, the field should be added conditionally only if the runtime provides a value for it.

ykulazhenkov · 2024-05-22T11:04:43Z

.idea/.gitignore

Can you remove .idea folder from the commit?

phoracek · 2024-05-26T12:59:49Z

CC @SchSeba

jiayoukun · 2024-08-08T09:14:53Z

@SchSeba Can you confirm the code for me?

filanov · 2024-12-05T16:44:15Z

Looks like this PR can help me as well, any progress on this one?

jiayoukun · 2024-12-06T02:29:48Z

Looks like this PR can help me as well, any progress on this one?

I achieved my goal by modifying the code through PR, but it seems that no one in the current community maintains it.

jiayoukun · 2024-12-06T02:33:00Z

@phoracek Hope PR can help everyone :)

filanov · 2024-12-06T07:57:34Z

@ykulazhenkov maybe you can help with that?

@jiayoukun the only thing that i can suggest is to add a test, you can get a reference from the PR that i closed, #339 should be simple enough.

ykulazhenkov · 2024-12-11T08:23:14Z

@jiayoukun Thanks for the PR.
Could you please squash all commits into a single one with a meaningful name and description?

Please share your thoughts about #311 (comment).

To make CI happy, you need to change the description of the PR according to the PR template https://github.com/k8snetworkplumbingwg/ovs-cni/blob/main/.github/PULL_REQUEST_TEMPLATE.md.
Your PR is missing the Release note section, which is mandatory.

To make the DCO check pass for your PR, you need to sign off your commit with git commit --signoff.

jiayoukun · 2024-12-12T10:36:41Z

@jiayoukun Thanks for the PR. Could you please squash all commits into a single one with a meaningful name and description?

Please share your thoughts about #311 (comment).

To make CI happy, you need to change the description of the PR according to the PR template https://github.com/k8snetworkplumbingwg/ovs-cni/blob/main/.github/PULL_REQUEST_TEMPLATE.md. Your PR is missing the Release note section, which is mandatory.

To make the DCO check pass for your PR, you need to sign off your commit with git commit --signoff.

The modification has been completed, thank you for your guidance. ：)

phoracek · 2024-12-12T22:04:54Z

Thanks for the PR and the reviews too.

@ykulazhenkov wanted to ask you, would you please own the review of this PR? If you trust and like the code, I'd be happy to merge this

phoracek · 2024-12-12T22:05:10Z

/retest

ykulazhenkov

I tested the PR and it works as expected. LGTM

@jiayoukun can you updated PR description to include release note? (check the PR template for details). I think this change should be reflected in the changelog.

jiayoukun · 2024-12-13T08:50:39Z

I tested the PR and it works as expected. LGTM

@jiayoukun can you updated PR description to include release note? (check the PR template for details). I think this change should be reflected in the changelog.

@AlonaKaplan Is that right? I have made the changes, please help me confirm.

ykulazhenkov · 2024-12-13T09:46:37Z

@jiayoukun

copy content of the block below as is to you PR description:

**What this PR does / why we need it**:

By adding the UID of a Pod to the external_ids field of the corresponding veth interface in the OVSDB, it becomes convenient for the Pod object to query the OVSDB using the UID to find the vethName of the Pod on the host machine, and then deliver OVS flow tables accordingly.

Currently, the host machine's veth interface in the OVSDB only stores the Netns field, which is useful, but for Pod objects, their netns is not publicly accessible. This change addresses this limitation by allowing Pod objects to query the OVSDB directly using their UID.

**Special notes for your reviewer**:

**Release note**:


```release-note
Added the Pod UID to the external_ids field of the corresponding veth interface in the OVSDB.
This change allows querying the vethName of a Pod on the host machine using its UID, improving the ability to deliver OVS flow tables for Pod objects.
```

**What this PR does / why we need it**: By adding the UID of a Pod to the external_ids field of the corresponding veth interface in the OVSDB, it becomes convenient for the Pod object to query the OVSDB using the UID to find the vethName of the Pod on the host machine, and then deliver OVS flow tables accordingly. Currently, the host machine's veth interface in the OVSDB only stores the Netns field, which is useful, but for Pod objects, their netns is not publicly accessible. This change addresses this limitation by allowing Pod objects to query the OVSDB directly using their UID. **Special notes for your reviewer**: **Release note**: ```release-note Added the Pod UID to the external_ids field of the corresponding veth interface in the OVSDB. This change allows querying the vethName of a Pod on the host machine using its UID, improving the ability to deliver OVS flow tables for Pod objects. ``` Signed-off-by: jiayoukun <[email protected]>

jiayoukun · 2024-12-13T09:52:03Z

@jiayoukun

copy content of the block below as is to you PR description:

**What this PR does / why we need it**:

By adding the UID of a Pod to the external_ids field of the corresponding veth interface in the OVSDB, it becomes convenient for the Pod object to query the OVSDB using the UID to find the vethName of the Pod on the host machine, and then deliver OVS flow tables accordingly.

Currently, the host machine's veth interface in the OVSDB only stores the Netns field, which is useful, but for Pod objects, their netns is not publicly accessible. This change addresses this limitation by allowing Pod objects to query the OVSDB directly using their UID.

**Special notes for your reviewer**:

**Release note**:


```release-note
Added the Pod UID to the external_ids field of the corresponding veth interface in the OVSDB.
This change allows querying the vethName of a Pod on the host machine using its UID, improving the ability to deliver OVS flow tables for Pod objects.

Thanks! : )

phoracek · 2024-12-13T10:23:57Z

/retest

phoracek · 2024-12-13T12:29:29Z

/approve
/lgtm

kubevirt-bot · 2024-12-13T12:29:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiayoukun, phoracek, ykulazhenkov

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~~ [phoracek]

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

adrianchiris reviewed May 22, 2024

View reviewed changes

ykulazhenkov reviewed May 22, 2024

View reviewed changes

.idea/.gitignore Outdated

Copy link

Contributor

ykulazhenkov May 22, 2024

Choose a reason for hiding this comment

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

Can you remove .idea folder from the commit?

filanov mentioned this pull request Dec 5, 2024

Add port pod namespace name to port creation #339

Closed

jiayoukun force-pushed the add-feature-branch branch from 902fdf1 to dcdf98f Compare December 12, 2024 10:32

kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Dec 12, 2024

ykulazhenkov approved these changes Dec 13, 2024

View reviewed changes

jiayoukun force-pushed the add-feature-branch branch from dcdf98f to 59f9ad5 Compare December 13, 2024 09:50

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 13, 2024

kubevirt-bot assigned phoracek Dec 13, 2024

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2024

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2024

kubevirt-bot merged commit dbf2164 into k8snetworkplumbingwg:main Dec 13, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PodUID to the OVSDB #311

Add PodUID to the OVSDB #311

jiayoukun commented May 21, 2024 •

edited by phoracek

Loading

kubevirt-bot commented May 21, 2024

jiayoukun commented May 21, 2024

adrianchiris May 22, 2024

ykulazhenkov Dec 11, 2024

ykulazhenkov May 22, 2024

phoracek commented May 26, 2024

jiayoukun commented Aug 8, 2024

filanov commented Dec 5, 2024

jiayoukun commented Dec 6, 2024

jiayoukun commented Dec 6, 2024

filanov commented Dec 6, 2024

ykulazhenkov commented Dec 11, 2024

jiayoukun commented Dec 12, 2024

phoracek commented Dec 12, 2024

phoracek commented Dec 12, 2024

ykulazhenkov left a comment

jiayoukun commented Dec 13, 2024

ykulazhenkov commented Dec 13, 2024

jiayoukun commented Dec 13, 2024

phoracek commented Dec 13, 2024

phoracek commented Dec 13, 2024

kubevirt-bot commented Dec 13, 2024

Add PodUID to the OVSDB #311

Add PodUID to the OVSDB #311

Conversation

jiayoukun commented May 21, 2024 • edited by phoracek Loading

kubevirt-bot commented May 21, 2024

jiayoukun commented May 21, 2024

adrianchiris May 22, 2024

Choose a reason for hiding this comment

ykulazhenkov Dec 11, 2024

Choose a reason for hiding this comment

ykulazhenkov May 22, 2024

Choose a reason for hiding this comment

phoracek commented May 26, 2024

jiayoukun commented Aug 8, 2024

filanov commented Dec 5, 2024

jiayoukun commented Dec 6, 2024

jiayoukun commented Dec 6, 2024

filanov commented Dec 6, 2024

ykulazhenkov commented Dec 11, 2024

jiayoukun commented Dec 12, 2024

phoracek commented Dec 12, 2024

phoracek commented Dec 12, 2024

ykulazhenkov left a comment

Choose a reason for hiding this comment

jiayoukun commented Dec 13, 2024

ykulazhenkov commented Dec 13, 2024

jiayoukun commented Dec 13, 2024

phoracek commented Dec 13, 2024

phoracek commented Dec 13, 2024

kubevirt-bot commented Dec 13, 2024

jiayoukun commented May 21, 2024 •

edited by phoracek

Loading