Skip to content

Conversation

danwinship
Copy link
Contributor

When deleting a NetNamespace, we weren't cleaning up all of the state correctly because we tried to make use of a struct field after its value had been cleared, meaning that complicated sequences of egress IP operations could result in junk rules getting added to OVS.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1543786

@danwinship danwinship added kind/bug Categorizes issue or PR as related to a bug. component/networking sig/networking labels Feb 9, 2018
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 9, 2018
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 9, 2018
Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

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

/lgtm

I'm surprised this couldn't be caught bu Golang.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 9, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, knobunc

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@danwinship
Copy link
Contributor Author

I'm surprised this couldn't be caught bu Golang.

The code isn't a priori wrong. delete(eip.namespacesByEgressIP, "") is a valid statement, it's just not the one we wanted.

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 18498, 18544, 18479, 18547).

@openshift-merge-robot openshift-merge-robot merged commit 8581458 into openshift:master Feb 9, 2018
openshift-merge-robot added a commit that referenced this pull request Feb 23, 2018
Automatic merge from submit-queue.

 Fix reassignment of egress IP after removal

When dropping an egress IP from eth0, we weren't updating our internal state to reflect that we had done that, so if you added it back again it wouldn't do everything it needed to do. (Introduced in #18121 but not discoverable until after #18547 was fixed.)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1547899
@danwinship danwinship deleted the egressip-add-delete-add-fix branch March 7, 2018 14:25
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. component/networking kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. sig/networking size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants