OpenStack Provider: Fix add IPv6 rules #6617

Merged
merged 1 commit into from Nov 28, 2016

Conversation

Projects
None yet
6 participants
Member

fnordahl commented Nov 25, 2016

Fixes breakage introduced in commit 3539524 by specifying
EthernetType for IPv6 rules.

Excplicitly set RemoteGroupId when creating rules with no RemoteIPPrefix.
Neutron defaults to set up CIDR 0.0.0.0/0 or ::/0 in some situations.

Also reverts some changes to rules where RemoteIPPrefix were added
where it should not have been added.

Have proposed improved tests that would have uncovered this to go-goose/goose#34

Contributor

macgreagoir commented Nov 25, 2016

@fnordahl, thanks again.

LGTM, and tests pass when using go-goose/goose#34.

I'd like a go-goose approver to review this when the required goose PR is merged.

LGTM to me too, so long as it has at least been manually verified to fix the issue it is addressing (no QA steps in PR).

Member

fnordahl commented Nov 25, 2016

In addition to testing against the updated go-goose version I injected a jujud executable built from source with this patch into a already bootstrapped controller on a OpenStack Cloud and verified that adding a new model and deploying an application to that model created the same security group rules as a unmodified version of jujud with the exception of the expected addition of IPv6 rules.

(For the manual test to work I had to change the version string in version/vesrion.go to match what I already had on the controller to prevent it from killing itself asking for a upgrade.)

Owner

wallyworld commented Nov 25, 2016

$$merge$$

Contributor

jujubot commented Nov 25, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Nov 25, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9712

Owner

wallyworld commented Nov 25, 2016

$$merge$$

Contributor

jujubot commented Nov 25, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Nov 25, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9713

Member

anastasiamac commented Nov 27, 2016

Please address this failure in github.com/juju/juju/provider/openstack:

FAIL: :70: localLiveSuite.TestSetupGlobalGroupExposesCorrectPorts

uploading fake tool versions: [1.99.0-precise-amd64 1.99.0-trusty-amd64 1.99.0-xenial-amd64]
[LOG] 0:00.000 DEBUG juju.environs.tools reading v1.* tools
[LOG] 0:00.000 INFO juju.environs.testing uploading FAKE tools 1.99.0-precise-amd64
[LOG] 0:00.002 INFO juju.environs.testing uploading FAKE tools 1.99.0-trusty-amd64
[LOG] 0:00.003 INFO juju.environs.testing uploading FAKE tools 1.99.0-xenial-amd64
[LOG] 0:00.011 INFO juju.environs.tools Writing tools/streams/v1/index2.json
[LOG] 0:00.011 INFO juju.environs.tools Writing tools/streams/v1/index.json
[LOG] 0:00.011 INFO juju.environs.tools Writing tools/streams/v1/com.ubuntu.juju-released-tools.json
live_test.go:226:
c.Check(stringRules, gc.DeepEquals, expectedRules)
... obtained []string = []string{"ingress icmp 0 0 9", "ingress icmp 0 0 IPv6 9", "ingress tcp 1 65535 9", "ingress tcp 1 65535 IPv6 9", "ingress tcp 22 22 0.0.0.0/0 9", "ingress tcp 22 22 ::/0 IPv6 9", "ingress tcp 34567 34567 0.0.0.0/0 9", "ingress tcp 34567 34567 ::/0 IPv6 9", "ingress udp 1 65535 9", "ingress udp 1 65535 IPv6 9"}
... expected []string = []string{"ingress icmp 0 0 IPv4 9", "ingress icmp 0 0 IPv6 9", "ingress tcp 1 65535 IPv4 9", "ingress tcp 1 65535 IPv6 9", "ingress tcp 22 22 0.0.0.0/0 IPv4 9", "ingress tcp 22 22 ::/0 IPv6 9", "ingress tcp 34567 34567 0.0.0.0/0 IPv4 9", "ingress tcp 34567 34567 ::/0 IPv6 9", "ingress udp 1 65535 IPv4 9", "ingress udp 1 65535 IPv6 9"}

Member

fnordahl commented Nov 28, 2016

@anastasiamac Waiting for go-goose/goose#34 to merge, it is approved and should not be far away. It applies fixes to the Neutron test service to make it operate more like the real Neutron in this regard. As soon as that happens I can update this commit with correct dependency and the test will pass.

Alternatively / in addition I could update the test to set default value for EthernetType if not returned from server, or change the test to check values of both RemoteIPPrefix and RemoteGroupId instead of EthernetType.

OpenStack Provider: Fix add IPv6 rules
Fixes breakage introduced in commit 3539524 by specifying
EthernetType for IPv6 rules.

Excplicitly set RemoteGroupId when creating rules with no RemoteIPPrefix.
Neutron defaults to set up CIDR 0.0.0.0/0 or ::/0 in some situations.

Also reverts some changes to rules where RemoteIPPrefix were added
where it should not have been added.
Contributor

macgreagoir commented Nov 28, 2016

$$merge$$

Contributor

jujubot commented Nov 28, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 7ce30d4 into juju:develop Nov 28, 2016

@fnordahl fnordahl deleted the fnordahl:fix_add_ipv6_support_to_openstack_firewaller branch Nov 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment