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

Clean-up around adding linux bridges to OVS bridges #449

Merged
merged 16 commits into from
Jul 22, 2020
Merged

Clean-up around adding linux bridges to OVS bridges #449

merged 16 commits into from
Jul 22, 2020

Conversation

lourot
Copy link
Contributor

@lourot lourot commented Apr 10, 2020

Cleans things up around ifup for adding a linux bridge to an OVS bridge and handle the fact that this isn't supported anymore.

Also useful for marking the corresponding ports as managed by us.

lourot added 3 commits April 10, 2020 14:05
Before we used to mark only OVS interfaces
1. Fix missing 'ifup' on bionic.
2. Mark the OVS port also when upgrading the charm.
Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

Looking good. Just a couple of minor-ish comments.

@lourot lourot marked this pull request as draft May 4, 2020 14:36
@lourot lourot marked this pull request as ready for review June 30, 2020 13:58
@lourot lourot requested a review from ajkavanagh June 30, 2020 13:58
Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

Please see comments. Looking good, but I'm a bit worried about the "doesn't work on 18.04+ in LXD issue", but don't really know if it's a big deal?

ajkavanagh
ajkavanagh previously approved these changes Jul 9, 2020
Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

I'm generally okay, but would definitely appreciate another reviewer.

@lourot
Copy link
Contributor Author

lourot commented Jul 9, 2020

As discussed with @thedac we want to be more opinionated here and neither create /etc/network/interfaces* nor call ifup on focal and newer. I'll rework this PR.

Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

LGMT -- +1 from another reviewer please.

@lourot lourot changed the title OVS port fixes Clean-up around adding linux bridges to OVS bridges Jul 22, 2020
Copy link
Contributor

@fnordahl fnordahl left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for your work on this

@fnordahl fnordahl merged commit 02bc7dc into juju:master Jul 22, 2020
@lourot lourot deleted the ovs-port-fixes branch July 22, 2020 09:20
openstack-mirroring pushed a commit to openstack/charm-neutron-gateway that referenced this pull request Aug 27, 2020
This patchset updates the configure_ovs() function in
hooks/neutron_utils.py such that ports and bridges in OVS are marked as
being managed by this charm.  This will allow us to clean up obsolete
managed bridges and ports in a later patchset.  (On configuration change
new ports and bridges might be created and former ones might become
obsolete.)

This patchset also fully deprecates the 'ext-port' config option such
that if both 'data-port' and 'ext-port' config options are set, the unit
is blocked.  The README and config.yaml are updated to reflect this
change.

This patchset also fixes and removes a few dead links.

Relies on a charm-helpers version containing these patchsets:
juju/charm-helpers#443
juju/charm-helpers#447
juju/charm-helpers#449

Related documentation:
* Deployment guide / Upgrades / Known issues: https://review.opendev.org/630290
* Release notes: https://review.opendev.org/742660

Change-Id: I8b459135d131e16865de40ff3eae16ea3bc7195e
Partial-Bug: #1809190
openstack-mirroring pushed a commit to openstack/openstack that referenced this pull request Aug 27, 2020
* Update charm-neutron-gateway from branch 'master'
  - Mark OVS bridges and ports as managed by charm-neutron-gateway
    
    This patchset updates the configure_ovs() function in
    hooks/neutron_utils.py such that ports and bridges in OVS are marked as
    being managed by this charm.  This will allow us to clean up obsolete
    managed bridges and ports in a later patchset.  (On configuration change
    new ports and bridges might be created and former ones might become
    obsolete.)
    
    This patchset also fully deprecates the 'ext-port' config option such
    that if both 'data-port' and 'ext-port' config options are set, the unit
    is blocked.  The README and config.yaml are updated to reflect this
    change.
    
    This patchset also fixes and removes a few dead links.
    
    Relies on a charm-helpers version containing these patchsets:
    juju/charm-helpers#443
    juju/charm-helpers#447
    juju/charm-helpers#449
    
    Related documentation:
    * Deployment guide / Upgrades / Known issues: https://review.opendev.org/630290
    * Release notes: https://review.opendev.org/742660
    
    Change-Id: I8b459135d131e16865de40ff3eae16ea3bc7195e
    Partial-Bug: #1809190
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants