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

components/ipam-ext: Remove primary-udn-kubevirt-binding NetworkAttachmentDefinition #1949

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RamLavi
Copy link
Collaborator

@RamLavi RamLavi commented Dec 16, 2024

What this PR does / why we need it:
passt is no longer the chosen kubevirt binding for primary UDN. Removing the deployment of the primary-udn-kubevirt-binding NetworkAttachmentDefinition accordingly.

Special notes for your reviewer:

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Dec 16, 2024
@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from ramlavi. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

ClusterRole: "kubevirt-ipam-controller-manager-role",
ClusterRoleBinding: "kubevirt-ipam-controller-manager-rolebinding",
Deployments: []string{"kubevirt-ipam-controller-manager"},
DaemonSets: []string{"passt-binding-cni"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about removing the ds please ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That should be done on a separate PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

not mandatory imho, but ok if more comfortable for you
as long as the tests use managedTap before that, else they will fail

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So perhaps it is better to do it on the PR that bumps ipam-ext?

Copy link
Collaborator

Choose a reason for hiding this comment

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

nope, orthogonal
passt can be removed after the tests are removed, this way we also each focus on orthogonal parts
as far as possible

flow should be adapt to managedTap,
green to remove passt all over

@oshoval
Copy link
Collaborator

oshoval commented Dec 16, 2024

lets please wait for ipam release and then add #1919
and just then remove passt
guess we are on the same page?

@RamLavi
Copy link
Collaborator Author

RamLavi commented Dec 16, 2024

lets please wait for ipam release and then add #1919 and just then remove passt guess we are on the same page?

why is it on draft though?

@oshoval
Copy link
Collaborator

oshoval commented Dec 16, 2024

lets please wait for ipam release and then add #1919 and just then remove passt guess we are on the same page?

why is it on draft though?

just because didnt test it, need first the release

@kubevirt-bot
Copy link
Collaborator

@RamLavi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-e2e-cluster-network-addons-operator-workflow-k8s 5fbf07a link true /test pull-e2e-cluster-network-addons-operator-workflow-k8s

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note-none Denotes a PR that doesn't merit a release note. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants