Skip to content

Commit

Permalink
Fix #201 by supporting azs key in subnets
Browse files Browse the repository at this point in the history
Previously only `az` was supported. Now when specifying
multiple AZs in a subnet, all IPs from that subnet can be
used in any instance_group/job that is in a zone that the
subnet mentioned.

This can lead to interesting scenarios when using mixes of
multi-az subnets and single-az subnets, where different offsets
can mean the same IP in a different zone, or the same index could
mean different IPs in different zones. Try not to do this, as it will
likely lead to confusion down the road. However, care is made to ensure
that IPs are never re-used, regardless of what subnets/azs they were
allowed to be used by.

This should not affect any existing IP allocations, since previously the
`azs` field wasn't looked at, and the old behaviors remain the same
for `az` and no-azs.
  • Loading branch information
geofffranks committed Jul 11, 2017
1 parent 84c949d commit e3380de
Show file tree
Hide file tree
Showing 10 changed files with 413 additions and 19 deletions.
19 changes: 19 additions & 0 deletions assets/static_ips/multi-azs-multi-underprovision.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

jobs:
- name: static_z1
instances: 1
azs: [z1, z2, z3]
networks:
- name: net1
static_ips: (( static_ips(16) )) # should fail saying pool is only 16 big
# definitely shouldn't say pool is 48 big
# (# azs * size of pool == 48 in this case)
networks:
- name: net1
subnets:
- azs: [ z1, z2, z3 ]
static:
- 10.0.0.1 - 10.0.0.15
- azs: [z1]
static:
- 10.1.1.1
21 changes: 21 additions & 0 deletions assets/static_ips/multi-azs-multi-zone-job.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@

jobs:
- name: static_z1
instances: 2
azs: [z1, z2, z3]
networks:
- name: net1
static_ips: (( static_ips(15, 16) )) # will grab 10.1.1.1 and 10.2.2.2
# from the 2nd/3rd subnet definitions
networks:
- name: net1
subnets:
- azs: [ z1, z2, z3 ]
static:
- 10.0.0.1 - 10.0.0.15
- azs: [z1]
static:
- 10.1.1.1
- azs: [z2]
static:
- 10.2.2.2
25 changes: 25 additions & 0 deletions assets/static_ips/multi-azs-one-zone-job.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
jobs:
- azs:
- z1
instances: 2
name: static_z1
networks:
- name: net1
static_ips: (( static_ips(0, 15) ))
networks:
- name: net1
subnets:
- azs:
- z1
- z2
- z3
static:
- 10.0.0.1 - 10.0.0.15
- azs:
- z1
static:
- 10.1.1.1
- azs:
- z2
static:
- 10.2.2.2
26 changes: 26 additions & 0 deletions assets/static_ips/multi-azs-same-index-different-ip.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@

jobs:
- name: static_z1
instances: 1
azs: [z1]
networks:
- name: net1
static_ips: (( static_ips(15) ))
- name: static_z2
instances: 1
azs: [z2]
networks:
- name: net1
static_ips: (( static_ips(15) ))
networks:
- name: net1
subnets:
- azs: [ z1, z2, z3 ]
static:
- 10.0.0.1 - 10.0.0.15
- azs: [z1]
static:
- 10.1.1.1
- azs: [z2]
static:
- 10.2.2.2
26 changes: 26 additions & 0 deletions assets/static_ips/multi-azs-same-ip-different-index.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@

jobs:
- name: static_z1
instances: 1
azs: [z1, z2, z3]
networks:
- name: net1
static_ips: (( static_ips(16) ))
- name: static_z2
instances: 1
azs: [z2]
networks:
- name: net1
static_ips: (( static_ips(15) ))
networks:
- name: net1
subnets:
- azs: [ z1, z2, z3 ]
static:
- 10.0.0.1 - 10.0.0.15
- azs: [z1]
static:
- 10.1.1.1
- azs: [z2]
static:
- 10.2.2.2
26 changes: 26 additions & 0 deletions assets/static_ips/multi-azs-same-ip-different-zones.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@

jobs:
- name: static_z1
instances: 1
azs: [z1]
networks:
- name: net1
static_ips: (( static_ips(14) ))
- name: static_z2
instances: 1
azs: [z2]
networks:
- name: net1
static_ips: (( static_ips(14) ))
networks:
- name: net1
subnets:
- azs: [ z1, z2, z3 ]
static:
- 10.0.0.1 - 10.0.0.15
- azs: [z1]
static:
- 10.1.1.1
- azs: [z2]
static:
- 10.2.2.2
18 changes: 18 additions & 0 deletions assets/static_ips/multi-azs-z2-underprovision.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@

jobs:
- name: static_z1
instances: 1
azs: [z2]
networks:
- name: net1
static_ips: (( static_ips(15) )) # should fail to grab 10.1.1.1, giving
# an index offset error - z2 only has 15 IPs
networks:
- name: net1
subnets:
- azs: [ z1, z2, z3 ]
static:
- 10.0.0.1 - 10.0.0.15
- azs: [z1]
static:
- 10.1.1.1
20 changes: 20 additions & 0 deletions ci/release_notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Bug Fixes

- Fixes #201 by supporting `azs` key in subnets

Previously only `az` was supported. Now when specifying
multiple AZs in a subnet, all IPs from that subnet can be
used in any instance-group/job that is in a zone that the
subnet mentioned.

This can lead to interesting scenarios when using mixes of
multi-az subnets and single-az subnets, where different offsets
can mean the same IP in a different zone, or the same index could
mean different IPs in different zones. Try not to do this, as it will
likely lead to confusion down the road. However, care is made to ensure
that IPs are never re-used, regardless of what subnets/azs they were
allowed to be used by.

This should not affect any existing IP allocations, since previously the
`azs` field wasn't looked at, and the old behaviors remain the same
for `az` and no-azs.
174 changes: 174 additions & 0 deletions cmd/spruce/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,180 @@ networks:
`)
})
Convey("Issue #201 - using `azs` instead of `az` in subnets", func() {
Convey("jobs in only one zone can see the IPs of all subnets that mentioned that zone", func() {
os.Args = []string{"spruce", "merge", "../../assets/static_ips/multi-azs-one-zone-job.yml"}
stdout = ""
stderr = ""

main()
So(stderr, ShouldEqual, "")
So(stdout, ShouldEqual, `jobs:
- azs:
- z1
instances: 2
name: static_z1
networks:
- name: net1
static_ips:
- 10.0.0.1
- 10.1.1.1
networks:
- name: net1
subnets:
- azs:
- z1
- z2
- z3
static:
- 10.0.0.1 - 10.0.0.15
- azs:
- z1
static:
- 10.1.1.1
- azs:
- z2
static:
- 10.2.2.2
`)
})
Convey("jobs in multiple zones can see the IPs of all subnets mentioning those zones", func() {
os.Args = []string{"spruce", "merge", "../../assets/static_ips/multi-azs-multi-zone-job.yml"}
stdout = ""
stderr = ""

main()
So(stderr, ShouldEqual, "")
So(stdout, ShouldEqual, `jobs:
- azs:
- z1
- z2
- z3
instances: 2
name: static_z1
networks:
- name: net1
static_ips:
- 10.1.1.1
- 10.2.2.2
networks:
- name: net1
subnets:
- azs:
- z1
- z2
- z3
static:
- 10.0.0.1 - 10.0.0.15
- azs:
- z1
static:
- 10.1.1.1
- azs:
- z2
static:
- 10.2.2.2
`)
})
Convey("a z2-only job cannot see z1-only IPs", func() {
os.Args = []string{"spruce", "merge", "../../assets/static_ips/multi-azs-z2-underprovision.yml"}
stdout = ""
stderr = ""

main()
So(stderr, ShouldEqual, `1 error(s) detected:
- $.jobs.static_z1.networks.net1.static_ips: request for static_ip(15) in a pool of only 15 (zero-indexed) static addresses
`)
So(stdout, ShouldEqual, "")
})
Convey("jobs with multiple zones see one copy of available IPs, rather than one copy per zone", func() {
os.Args = []string{"spruce", "merge", "../../assets/static_ips/multi-azs-multi-underprovision.yml"}
stdout = ""
stderr = ""

main()
So(stderr, ShouldEqual, `1 error(s) detected:
- $.jobs.static_z1.networks.net1.static_ips: request for static_ip(16) in a pool of only 16 (zero-indexed) static addresses
`)
So(stdout, ShouldEqual, "")
})
Convey("edge case - same index used for different IPs with multi-az subnets", func() {
os.Args = []string{"spruce", "merge", "../../assets/static_ips/multi-azs-same-index-different-ip.yml"}
stdout = ""
stderr = ""

main()
So(stderr, ShouldEqual, "")
So(stdout, ShouldEqual, `jobs:
- azs:
- z1
instances: 1
name: static_z1
networks:
- name: net1
static_ips:
- 10.1.1.1
- azs:
- z2
instances: 1
name: static_z2
networks:
- name: net1
static_ips:
- 10.2.2.2
networks:
- name: net1
subnets:
- azs:
- z1
- z2
- z3
static:
- 10.0.0.1 - 10.0.0.15
- azs:
- z1
static:
- 10.1.1.1
- azs:
- z2
static:
- 10.2.2.2
`)
})
Convey("edge case - dont give out same IP when specified in jobs with different zones", func() {
os.Args = []string{"spruce", "merge", "../../assets/static_ips/multi-azs-same-ip-different-zones.yml"}
stdout = ""
stderr = ""

main()
So(stderr, ShouldEqual, `1 error(s) detected:
- $.jobs.static_z2.networks.net1.static_ips: tried to use IP '10.0.0.15', but that address is already allocated to static_z1/0
`)
So(stdout, ShouldEqual, "")
})
Convey("edge case - don't give out same IP when using different offsets", func() {
os.Args = []string{"spruce", "merge", "../../assets/static_ips/multi-azs-same-ip-different-index.yml"}
stdout = ""
stderr = ""

main()
So(stderr, ShouldEqual, `1 error(s) detected:
- $.jobs.static_z2.networks.net1.static_ips: tried to use IP '10.2.2.2', but that address is already allocated to static_z1/0
`)
So(stdout, ShouldEqual, "")
})
})

Convey("Empty operator works", func() {

Expand Down
Loading

0 comments on commit e3380de

Please sign in to comment.