Skip to content
This repository has been archived by the owner on Oct 22, 2019. It is now read-only.

Raise error if IPPool is created with a bad cidr block size #82

Merged
merged 2 commits into from
Feb 6, 2016

Conversation

TrimBiggs
Copy link
Contributor

@TrimBiggs TrimBiggs force-pushed the ipam-block-size-error branch 2 times, most recently from ad04fd9 to fc346e4 Compare February 5, 2016 23:20
@@ -133,9 +134,16 @@ def __init__(self, cidr, ipip=False, masquerade=False, ipam=True):
"""
# Normalize the CIDR (e.g. 1.2.3.4/16 -> 1.2.0.0/16)
self.cidr = IPNetwork(cidr).cidr
self.ipam = bool(ipam)
if self.ipam:
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add a comment to the doc string to indicate the limitation on the CIDR when used with calico IPAM module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@robbrockbank
Copy link
Contributor

Minor comment and then LGTM.

@TrimBiggs TrimBiggs force-pushed the ipam-block-size-error branch from fc346e4 to bf2c7c6 Compare February 6, 2016 00:27
@@ -543,3 +543,9 @@ class AddressNotAssignedError(BlockError):
Tried to query an address that isn't assigned.
"""
pass

class AddressRangeNotAllowedError(BlockError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I like the name of this. It isn't a range. it's a CIDR.

Perhaps just CIDRToSmallError or something along those lines.

@TrimBiggs TrimBiggs force-pushed the ipam-block-size-error branch from 0a296cc to cb043fd Compare February 6, 2016 01:12
@robbrockbank
Copy link
Contributor

LGTM

robbrockbank pushed a commit that referenced this pull request Feb 6, 2016
Raise error if IPPool is created with a bad cidr block size
@robbrockbank robbrockbank merged commit 4702169 into projectcalico:master Feb 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants