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

[dev] Deduplicate ingress/egress subnets and addresses for NetworkInfo results #11535

Merged
merged 1 commit into from May 5, 2020

Conversation

achilleasa
Copy link
Contributor

Description of change

Due to the way that addresses are being collected and merged together by the machiner and instancepoller workers, we may end up with duplicate entries for the public IP addresses when running on aws substrates (for more information see this related bug).

As a result, network-get may potentially return duplicate entries as shown in https://bugs.launchpad.net/juju/+bug/1864072 which this PR masks by filtering the output of the NetworkInfo API call at the facade level to remove duplicates before returning the result back to the client.

This particular route (deduplication vs fixing the problem at the core) was chosen after considering the fact that there is active ongoing (longer-term) work with networking internals which will eventually fix the problem. The proposed solution ensures that consumers get the right result while still buying us additional time to land a better fix.

QA steps

# Install juju from snap
$ /snap/bin/juju bootstrap aws --credential aws --no-gui
$ /snap/bin/juju deploy percona-cluster
$ /snap/bin/juju run --unit percona-cluster/0 'printenv JUJU_VERSION'
2.7.6

# Notice that network-get returns duplicate entries for bind/ingress addresses
$ /snap/bin/juju run --unit percona-cluster/0 'network-get cluster'
bind-addresses:
- macaddress: 0e:c7:44:73:f3:07
  interfacename: ens3
  addresses:
  - hostname: ""
    address: 172.31.46.122            <--+
    cidr: 172.31.32.0/20                 | Dup
  - hostname: ""                         |
    address: 172.31.46.122            <--+
    cidr: 172.31.32.0/20
- macaddress: ee:19:50:1f:3e:9a
  interfacename: fan-252
  addresses:
  - hostname: ""
    address: 252.46.122.1
    cidr: 252.32.0.0/12
egress-subnets:
- 172.31.46.122/32
ingress-addresses:
- 172.31.46.122                      <--| Dup
- 172.31.46.122                      <--+
- 252.46.122.1

# Upgrade your controller to the version from this PR
$ juju upgrade-controller --build-agent

# Notice that network-get filters duplicates
$ juju run --unit percona-cluster/0 'network-get cluster'
bind-addresses:
- macaddress: 0e:c7:44:73:f3:07
  interfacename: ens3
  addresses:
  - hostname: ""
    address: 172.31.46.122
    cidr: 172.31.32.0/20
- macaddress: ee:19:50:1f:3e:9a
  interfacename: fan-252
  addresses:
  - hostname: ""
    address: 252.46.122.1
    cidr: 252.32.0.0/12
egress-subnets:
- 172.31.46.122/32
ingress-addresses:
- 252.46.122.1
- 172.31.46.122

Bug reference

https://bugs.launchpad.net/juju/+bug/1864072

Copy link
Member

@manadart manadart left a comment

Choose a reason for hiding this comment

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

Code and QA all good here.

// Copyright 2020 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package uniter
Copy link
Member

Choose a reason for hiding this comment

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

Yes.

@achilleasa
Copy link
Contributor Author

!!build!! (BootstrapSuite timeout)

@achilleasa
Copy link
Contributor Author

$$merge$$

@jujubot jujubot merged commit fc41081 into juju:develop May 5, 2020
@achilleasa achilleasa deleted the dev-dedup-network-info-results branch May 5, 2020 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants