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

Explicitly unmanage veth interfaces #701

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

phoracek
Copy link
Member

@phoracek phoracek commented Feb 24, 2021

Is this a BUG FIX or a FEATURE ?:

/kind bug

What this PR does / why we need it:

nmstate 0.3 introduces a bug where all veths attached to nmstatectl
configured bridge turn into "managed" by NetworkManager. This was not
the case in 0.2.

Due to this regression, NetworkManager deliberatelly detaches veth
ifaces from the bridge and by doing that it disconnectecs Pods/VMs from
the network.

With this change, we explicitly set veth interfaces as unmanaged.

Important: With this change, it is no longer possible to change
configuration of bridges that have veths attached.

This should be reverted once https://bugzilla.redhat.com/show_bug.cgi?id=1932247 becomes available.

Special notes for your reviewer:

Release note:

Explicitly set host veths as unmanaged

nmstate 0.3 introduces a bug where all veths attached to nmstatectl
configured bridge turn into "managed" by NetworkManager. This was not
the case in 0.2.

Due to this regression, NetworkManager deliberatelly detaches veth
ifaces from the bridge and by doing that it disconnectecs Pods/VMs from
the network.

With this change, we explicitly set veth interfaces as unmanaged.

Important: With this change, it is no longer possible to change
configuration of bridges that have veths attached.

Signed-off-by: Petr Horáček <[email protected]>
@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Feb 24, 2021
@qinqon
Copy link
Member

qinqon commented Feb 24, 2021

/lgtm
/approve

Ugly hack that will unblock upgrades, this will be removed after fix at nmstate.

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 24, 2021
@kubevirt-bot
Copy link
Collaborator

[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:

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 Feb 24, 2021
@phoracek
Copy link
Member Author

/cherry-pick release-0.37

@kubevirt-bot
Copy link
Collaborator

@phoracek: once the present PR merges, I will cherry-pick it on top of release-0.37 in a new PR and assign it to you.

In response to this:

/cherry-pick release-0.37

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/test-infra repository.

@phoracek
Copy link
Member Author

/retest

@phoracek
Copy link
Member Author

/override pull-kubernetes-nmstate-e2e-handler-k8s-future

@kubevirt-bot
Copy link
Collaborator

@phoracek: Overrode contexts on behalf of phoracek: pull-kubernetes-nmstate-e2e-handler-k8s-future

In response to this:

/override pull-kubernetes-nmstate-e2e-handler-k8s-future

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/test-infra repository.

@kubevirt-bot
Copy link
Collaborator

@phoracek: new pull request created: #702

In response to this:

/cherry-pick release-0.37

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/test-infra repository.

@kubevirt-bot
Copy link
Collaborator

@phoracek: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubernetes-nmstate-e2e-handler-k8s-future 2e4da97 link /test pull-kubernetes-nmstate-e2e-handler-k8s-future

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

phoracek added a commit to phoracek/kubernetes-nmstate that referenced this pull request Feb 25, 2021
phoracek added a commit to phoracek/kubernetes-nmstate that referenced this pull request Feb 25, 2021
phoracek added a commit to phoracek/kubernetes-nmstate that referenced this pull request Feb 25, 2021
This reverts commit f717775.

Signed-off-by: Petr Horáček <[email protected]>
kubevirt-bot pushed a commit that referenced this pull request Feb 26, 2021
This reverts commit f717775.

Signed-off-by: Petr Horáček <[email protected]>
kubevirt-bot pushed a commit to kubevirt-bot/kubernetes-nmstate that referenced this pull request Feb 26, 2021
This reverts commit f717775.

Signed-off-by: Petr Horáček <[email protected]>
kubevirt-bot pushed a commit that referenced this pull request Feb 26, 2021
This reverts commit f717775.

Signed-off-by: Petr Horáček <[email protected]>

Co-authored-by: Petr Horáček <[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. kind/bug lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants