From 40d4804ad166f01b4c2e51b0f56bf3df62b387c4 Mon Sep 17 00:00:00 2001 From: Matt McEuen Date: Fri, 22 Jan 2021 10:50:40 -0600 Subject: [PATCH] Add IPPool CR This converts the ipammer to use an IPPool CR-based to persist IPAM state. This followon patchset adds the actual persistence to the API Server: https://review.opendev.org/c/airship/vino/+/774184 Change-Id: Ib81a0bc6be1c74d85e2adc2dcadd09590a65b292 --- PROJECT | 3 + .../bases/airship.airshipit.org_ippools.yaml | 79 +++++++ config/crd/kustomization.yaml | 3 + .../crd/patches/cainjection_in_ippools.yaml | 8 + config/crd/patches/webhook_in_ippools.yaml | 17 ++ config/rbac/ippool_editor_role.yaml | 24 +++ config/rbac/ippool_viewer_role.yaml | 20 ++ config/samples/ippool.yaml | 15 ++ docs/api/vino.md | 195 ++++++++++++++++++ pkg/api/v1/ippool_types.go | 68 ++++++ pkg/api/v1/zz_generated.deepcopy.go | 114 ++++++++++ pkg/ipam/errors.go | 18 +- pkg/ipam/ipam.go | 51 ++--- pkg/ipam/ipam_test.go | 111 +++++++--- tools/deployment/test-cr.sh | 1 + 15 files changed, 670 insertions(+), 57 deletions(-) create mode 100644 config/crd/bases/airship.airshipit.org_ippools.yaml create mode 100644 config/crd/patches/cainjection_in_ippools.yaml create mode 100644 config/crd/patches/webhook_in_ippools.yaml create mode 100644 config/rbac/ippool_editor_role.yaml create mode 100644 config/rbac/ippool_viewer_role.yaml create mode 100644 config/samples/ippool.yaml create mode 100644 pkg/api/v1/ippool_types.go diff --git a/PROJECT b/PROJECT index 18c03d4..55d04bc 100644 --- a/PROJECT +++ b/PROJECT @@ -4,4 +4,7 @@ resources: - group: airship kind: Vino version: v1 +- group: airship + kind: IPPool + version: v1 version: "2" diff --git a/config/crd/bases/airship.airshipit.org_ippools.yaml b/config/crd/bases/airship.airshipit.org_ippools.yaml new file mode 100644 index 0000000..ecbe830 --- /dev/null +++ b/config/crd/bases/airship.airshipit.org_ippools.yaml @@ -0,0 +1,79 @@ + +--- +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.3.0 + creationTimestamp: null + name: ippools.airship.airshipit.org +spec: + group: airship.airshipit.org + names: + kind: IPPool + listKind: IPPoolList + plural: ippools + singular: ippool + scope: Namespaced + validation: + openAPIV3Schema: + description: IPPool is the Schema for the ippools API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: IPPool tracks allocation ranges and statuses within a specific + subnet IPv4 or IPv6 subnet. It has a set of ranges of IPs within the + subnet from which IPs can be allocated by IPAM, and a set of IPs that + are currently allocated already. + properties: + allocatedIPs: + items: + type: string + type: array + ranges: + items: + description: Range has (inclusive) bounds within a subnet from which + IPs can be allocated + properties: + start: + type: string + stop: + type: string + required: + - start + - stop + type: object + type: array + subnet: + type: string + required: + - allocatedIPs + - ranges + - subnet + type: object + status: + description: IPPoolStatus defines the observed state of IPPool + type: object + type: object + version: v1 + versions: + - name: v1 + served: true + storage: true +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index fda049b..a3b35aa 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -3,6 +3,7 @@ # It should be run by config/default resources: - bases/airship.airshipit.org_vinoes.yaml +- bases/airship.airshipit.org_ippools.yaml - bases/bmh.yaml # +kubebuilder:scaffold:crdkustomizeresource @@ -10,11 +11,13 @@ patchesStrategicMerge: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD #- patches/webhook_in_vinoes.yaml +#- patches/webhook_in_ippools.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD #- patches/cainjection_in_vinoes.yaml +#- patches/cainjection_in_ippools.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch # the following config is for teaching kustomize how to do kustomization for CRDs. diff --git a/config/crd/patches/cainjection_in_ippools.yaml b/config/crd/patches/cainjection_in_ippools.yaml new file mode 100644 index 0000000..e467169 --- /dev/null +++ b/config/crd/patches/cainjection_in_ippools.yaml @@ -0,0 +1,8 @@ +# The following patch adds a directive for certmanager to inject CA into the CRD +# CRD conversion requires k8s 1.13 or later. +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) + name: ippools.airship.airshipit.org diff --git a/config/crd/patches/webhook_in_ippools.yaml b/config/crd/patches/webhook_in_ippools.yaml new file mode 100644 index 0000000..6698840 --- /dev/null +++ b/config/crd/patches/webhook_in_ippools.yaml @@ -0,0 +1,17 @@ +# The following patch enables conversion webhook for CRD +# CRD conversion requires k8s 1.13 or later. +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: ippools.airship.airshipit.org +spec: + conversion: + strategy: Webhook + webhookClientConfig: + # this is "\n" used as a placeholder, otherwise it will be rejected by the apiserver for being blank, + # but we're going to set it later using the cert-manager (or potentially a patch if not using cert-manager) + caBundle: Cg== + service: + namespace: system + name: webhook-service + path: /convert diff --git a/config/rbac/ippool_editor_role.yaml b/config/rbac/ippool_editor_role.yaml new file mode 100644 index 0000000..8a142cb --- /dev/null +++ b/config/rbac/ippool_editor_role.yaml @@ -0,0 +1,24 @@ +# permissions for end users to edit ippools. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: ippool-editor-role +rules: +- apiGroups: + - airship.airshipit.org + resources: + - ippools + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - airship.airshipit.org + resources: + - ippools/status + verbs: + - get diff --git a/config/rbac/ippool_viewer_role.yaml b/config/rbac/ippool_viewer_role.yaml new file mode 100644 index 0000000..36cfb99 --- /dev/null +++ b/config/rbac/ippool_viewer_role.yaml @@ -0,0 +1,20 @@ +# permissions for end users to view ippools. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: ippool-viewer-role +rules: +- apiGroups: + - airship.airshipit.org + resources: + - ippools + verbs: + - get + - list + - watch +- apiGroups: + - airship.airshipit.org + resources: + - ippools/status + verbs: + - get diff --git a/config/samples/ippool.yaml b/config/samples/ippool.yaml new file mode 100644 index 0000000..742bac0 --- /dev/null +++ b/config/samples/ippool.yaml @@ -0,0 +1,15 @@ +apiVersion: airship.airshipit.org/v1 +kind: IPPool +metadata: + name: ippool-sample +spec: + subnet: 10.0.0.0/16 + ranges: + - start: 10.0.0.1 + stop: 10.0.0.9 + - start: 10.0.1.1 + stop: 10.0.1.9 + allocatedIPs: + - 10.0.0.1 + - 10.0.0.2 + - 10.0.1.1 diff --git a/docs/api/vino.md b/docs/api/vino.md index f6e79da..99397f4 100644 --- a/docs/api/vino.md +++ b/docs/api/vino.md @@ -218,6 +218,160 @@ bool +

IPPool +

+

IPPool is the Schema for the ippools API

+
+
+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+metadata
+ + +Kubernetes meta/v1.ObjectMeta + + +		 +Refer to the Kubernetes API documentation for the fields of the +metadata field. +
+spec
+ + +IPPoolSpec + + +		 +
+
+ + + + + + + + + + + + + +
+subnet
+ +string + +		 +
+ranges
+ + +[]Range + + +		 +
+allocatedIPs
+ +[]string + +		 +
+
+status
+ + +IPPoolStatus + + +		 +
+
+
+

IPPoolSpec +

+

+(Appears on: +IPPool) +

+

IPPool tracks allocation ranges and statuses within a specific +subnet IPv4 or IPv6 subnet. It has a set of ranges of IPs +within the subnet from which IPs can be allocated by IPAM, +and a set of IPs that are currently allocated already.

+
+
+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+subnet
+ +string + +		 +
+ranges
+ + +[]Range + + +		 +
+allocatedIPs
+ +[]string + +		 +
+
+
+

IPPoolStatus +

+

+(Appears on: +IPPool) +

+

IPPoolStatus defines the observed state of IPPool

NamespacedName

@@ -549,6 +703,47 @@ DiskDrivesTemplate +

Range +

+

+(Appears on: +IPPoolSpec) +

+

Range has (inclusive) bounds within a subnet from which IPs can be allocated

+
+
+ + + + + + + + + + + + + + + + + +
FieldDescription
+start
+ +string + +		 +
+stop
+ +string + +		 +
+
+

VMNodeFlavor

diff --git a/pkg/api/v1/ippool_types.go b/pkg/api/v1/ippool_types.go new file mode 100644 index 0000000..628bede --- /dev/null +++ b/pkg/api/v1/ippool_types.go @@ -0,0 +1,68 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// IPPool tracks allocation ranges and statuses within a specific +// subnet IPv4 or IPv6 subnet. It has a set of ranges of IPs +// within the subnet from which IPs can be allocated by IPAM, +// and a set of IPs that are currently allocated already. +type IPPoolSpec struct { + Subnet string `json:"subnet"` + Ranges []Range `json:"ranges"` + AllocatedIPs []string `json:"allocatedIPs"` +} + +// Range has (inclusive) bounds within a subnet from which IPs can be allocated +type Range struct { + Start string `json:"start"` + Stop string `json:"stop"` +} + +// IPPoolStatus defines the observed state of IPPool +type IPPoolStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file +} + +// +kubebuilder:object:root=true + +// IPPool is the Schema for the ippools API +type IPPool struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec IPPoolSpec `json:"spec,omitempty"` + Status IPPoolStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// IPPoolList contains a list of IPPool +type IPPoolList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []IPPool `json:"items"` +} + +func init() { + SchemeBuilder.Register(&IPPool{}, &IPPoolList{}) +} diff --git a/pkg/api/v1/zz_generated.deepcopy.go b/pkg/api/v1/zz_generated.deepcopy.go index eb3c595..b835d4b 100644 --- a/pkg/api/v1/zz_generated.deepcopy.go +++ b/pkg/api/v1/zz_generated.deepcopy.go @@ -91,6 +91,105 @@ func (in *DiskOptions) DeepCopy() *DiskOptions { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IPPool) DeepCopyInto(out *IPPool) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPPool. +func (in *IPPool) DeepCopy() *IPPool { + if in == nil { + return nil + } + out := new(IPPool) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *IPPool) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IPPoolList) DeepCopyInto(out *IPPoolList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]IPPool, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPPoolList. +func (in *IPPoolList) DeepCopy() *IPPoolList { + if in == nil { + return nil + } + out := new(IPPoolList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *IPPoolList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IPPoolSpec) DeepCopyInto(out *IPPoolSpec) { + *out = *in + if in.Ranges != nil { + in, out := &in.Ranges, &out.Ranges + *out = make([]Range, len(*in)) + copy(*out, *in) + } + if in.AllocatedIPs != nil { + in, out := &in.AllocatedIPs, &out.AllocatedIPs + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPPoolSpec. +func (in *IPPoolSpec) DeepCopy() *IPPoolSpec { + if in == nil { + return nil + } + out := new(IPPoolSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IPPoolStatus) DeepCopyInto(out *IPPoolStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPPoolStatus. +func (in *IPPoolStatus) DeepCopy() *IPPoolStatus { + if in == nil { + return nil + } + out := new(IPPoolStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NamespacedName) DeepCopyInto(out *NamespacedName) { *out = *in @@ -206,6 +305,21 @@ func (in *NodeSet) DeepCopy() *NodeSet { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Range) DeepCopyInto(out *Range) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Range. +func (in *Range) DeepCopy() *Range { + if in == nil { + return nil + } + out := new(Range) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VMNodeFlavor) DeepCopyInto(out *VMNodeFlavor) { *out = *in diff --git a/pkg/ipam/errors.go b/pkg/ipam/errors.go index 4d2c47c..fc9c0a1 100644 --- a/pkg/ipam/errors.go +++ b/pkg/ipam/errors.go @@ -16,6 +16,8 @@ package ipam import ( "fmt" + + vinov1 "vino/pkg/api/v1" ) // ErrSubnetNotAllocated returned if the subnet is not registered in IPAM @@ -23,23 +25,28 @@ type ErrSubnetNotAllocated struct { Subnet string } +// ErrSubnetRangeInvalid returned if a requested subnet's range is not valid +type ErrSubnetRangeInvalid struct { + SubnetRange vinov1.Range +} + // ErrSubnetRangeOverlapsWithExistingRange returned if the subnet's range // overlaps (partially or completely) with an already added range in that subnet type ErrSubnetRangeOverlapsWithExistingRange struct { Subnet string - SubnetRange Range + SubnetRange vinov1.Range } // ErrSubnetRangeNotAllocated returned if the subnet's range is not registered in IPAM type ErrSubnetRangeNotAllocated struct { Subnet string - SubnetRange Range + SubnetRange vinov1.Range } // ErrSubnetRangeExhausted returned if the subnet's range has no unallocated IPs type ErrSubnetRangeExhausted struct { Subnet string - SubnetRange Range + SubnetRange vinov1.Range } // ErrInvalidIPAddress returned if an IP address string is malformed @@ -56,6 +63,11 @@ func (e ErrSubnetNotAllocated) Error() string { return fmt.Sprintf("IPAM subnet %s not allocated", e.Subnet) } +func (e ErrSubnetRangeInvalid) Error() string { + return fmt.Sprintf("IPAM range [%s,%s] is invalid", + e.SubnetRange.Start, e.SubnetRange.Stop) +} + func (e ErrSubnetRangeOverlapsWithExistingRange) Error() string { return fmt.Sprintf("IPAM range [%s,%s] in subnet %s overlaps with an existing range", e.SubnetRange.Start, e.SubnetRange.Stop, e.Subnet) diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go index 13dc9b0..adcf594 100644 --- a/pkg/ipam/ipam.go +++ b/pkg/ipam/ipam.go @@ -19,6 +19,8 @@ import ( "strings" "unsafe" + vinov1 "vino/pkg/api/v1" + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -30,45 +32,46 @@ type Ipam struct { Scheme *runtime.Scheme Client client.Client - ippools map[string]*IPPool -} - -// IPPool tracks allocation ranges and statuses within a specific -// subnet IPv4 or IPv6 subnet. It has a set of ranges of IPs -// within the subnet from which IPs can be allocated by IPAM, -// and a set of IPs that are currently allocated already. -type IPPool struct { - Subnet string - Ranges []Range - AllocatedIPs []string -} - -// Range has (inclusive) bounds within a subnet from which IPs can be allocated -type Range struct { - Start string - Stop string + ippools map[string]*vinov1.IPPoolSpec } // NewIpam initializes an empty IPAM configuration. // TODO: persist and refresh state from the API server // TODO: add ability to remove IP addresses and ranges func NewIpam() *Ipam { - ippools := make(map[string]*IPPool) + ippools := make(map[string]*vinov1.IPPoolSpec) return &Ipam{ ippools: ippools, } } +// Create a new Range, validating its input +func NewRange(start string, stop string) (vinov1.Range, error) { + r := vinov1.Range{Start: start, Stop: stop} + a, e := ipStringToInt(start) + if e != nil { + return vinov1.Range{}, e + } + b, e := ipStringToInt(stop) + if e != nil { + return vinov1.Range{}, e + } + if b < a { + return vinov1.Range{}, ErrSubnetRangeInvalid{r} + } + return r, nil +} + // AddSubnetRange adds a range within a subnet for IP allocation -// TODO error: invalid range for subnet // TODO error: range overlaps with existing range or subnet overlaps with existing subnet -func (i *Ipam) AddSubnetRange(subnet string, subnetRange Range) error { +// TODO error: invalid range for subnet +func (i *Ipam) AddSubnetRange(subnet string, subnetRange vinov1.Range) error { // Does the subnet already exist? (this is fine) ippool, exists := i.ippools[subnet] if !exists { - ippool = &IPPool{ + ippool = &vinov1.IPPoolSpec{ Subnet: subnet, - Ranges: []Range{subnetRange}, // TODO DeepCopy() + Ranges: []vinov1.Range{subnetRange}, AllocatedIPs: []string{}, } i.ippools[subnet] = ippool @@ -91,7 +94,7 @@ func (i *Ipam) AddSubnetRange(subnet string, subnetRange Range) error { } // AllocateIP allocates an IP from a range and return it -func (i *Ipam) AllocateIP(subnet string, subnetRange Range) (string, error) { +func (i *Ipam) AllocateIP(subnet string, subnetRange vinov1.Range) (string, error) { // NOTE/TODO: this is not threadsafe, which is fine because // the final impl will use the api server as the backend, not local. ippool, exists := i.ippools[subnet] @@ -122,7 +125,7 @@ func (i *Ipam) AllocateIP(subnet string, subnetRange Range) (string, error) { // steps through them looking for one that that is not already // in use, converts it back to a string and returns it. // It does not itself add it to the list of assigned IPs. -func findFreeIPInRange(ippool *IPPool, subnetRange Range) (string, error) { +func findFreeIPInRange(ippool *vinov1.IPPoolSpec, subnetRange vinov1.Range) (string, error) { allocatedIPSet := sliceToMap(ippool.AllocatedIPs) intToString := intToIPv4String if strings.Contains(ippool.Subnet, ":") { diff --git a/pkg/ipam/ipam_test.go b/pkg/ipam/ipam_test.go index f9b2450..3496aeb 100644 --- a/pkg/ipam/ipam_test.go +++ b/pkg/ipam/ipam_test.go @@ -18,6 +18,8 @@ import ( "math" "testing" + vinov1 "vino/pkg/api/v1" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -25,53 +27,53 @@ import ( func TestAllocateIP(t *testing.T) { tests := []struct { name, subnet, expectedErr string - subnetRange Range + subnetRange vinov1.Range }{ { name: "success ipv4", subnet: "10.0.0.0/16", - subnetRange: Range{"10.0.1.0", "10.0.1.9"}, + subnetRange: vinov1.Range{Start: "10.0.1.0", Stop: "10.0.1.9"}, }, { name: "success ipv6", subnet: "2600:1700:b030:0000::/72", - subnetRange: Range{"2600:1700:b030:0000::", "2600:1700:b030:0009::"}, + subnetRange: vinov1.Range{Start: "2600:1700:b030:0000::", Stop: "2600:1700:b030:0009::"}, }, { name: "error subnet not allocated ipv4", subnet: "10.0.0.0/20", - subnetRange: Range{"10.0.1.0", "10.0.1.9"}, + subnetRange: vinov1.Range{Start: "10.0.1.0", Stop: "10.0.1.9"}, expectedErr: "IPAM subnet 10.0.0.0/20 not allocated", }, { name: "error subnet not allocated ipv6", subnet: "2600:1700:b030:0000::/80", - subnetRange: Range{"2600:1700:b030:0000::", "2600:1700:b030:0009::"}, + subnetRange: vinov1.Range{Start: "2600:1700:b030:0000::", Stop: "2600:1700:b030:0009::"}, expectedErr: "IPAM subnet 2600:1700:b030:0000::/80 not allocated", }, { name: "error range not allocated ipv4", subnet: "10.0.0.0/16", - subnetRange: Range{"10.0.2.0", "10.0.2.9"}, + subnetRange: vinov1.Range{Start: "10.0.2.0", Stop: "10.0.2.9"}, expectedErr: "IPAM range [10.0.2.0,10.0.2.9] in subnet 10.0.0.0/16 is not allocated", }, { name: "error range not allocated ipv6", subnet: "2600:1700:b030:0000::/72", - subnetRange: Range{"2600:1700:b030:0000::", "2600:1700:b030:1111::"}, + subnetRange: vinov1.Range{Start: "2600:1700:b030:0000::", Stop: "2600:1700:b030:1111::"}, expectedErr: "IPAM range [2600:1700:b030:0000::,2600:1700:b030:1111::] " + "in subnet 2600:1700:b030:0000::/72 is not allocated", }, { name: "error range exhausted ipv4", subnet: "192.168.0.0/1", - subnetRange: Range{"192.168.0.0", "192.168.0.0"}, + subnetRange: vinov1.Range{Start: "192.168.0.0", Stop: "192.168.0.0"}, expectedErr: "IPAM range [192.168.0.0,192.168.0.0] in subnet 192.168.0.0/1 is exhausted", }, { name: "error range exhausted ipv6", subnet: "2600:1700:b031:0000::/64", - subnetRange: Range{"2600:1700:b031:0000::", "2600:1700:b031:0000::"}, + subnetRange: vinov1.Range{Start: "2600:1700:b031:0000::", Stop: "2600:1700:b031:0000::"}, expectedErr: "IPAM range [2600:1700:b031:0000::,2600:1700:b031:0000::] " + "in subnet 2600:1700:b031:0000::/64 is exhausted", }, @@ -83,17 +85,21 @@ func TestAllocateIP(t *testing.T) { ipammer := NewIpam() // Pre-populate IPAM with some precondition test data - err := ipammer.AddSubnetRange("10.0.0.0/16", Range{"10.0.1.0", "10.0.1.9"}) + err := ipammer.AddSubnetRange("10.0.0.0/16", vinov1.Range{Start: "10.0.1.0", Stop: "10.0.1.9"}) require.NoError(t, err) - err = ipammer.AddSubnetRange("2600:1700:b030:0000::/72", Range{"2600:1700:b030:0000::", "2600:1700:b030:0009::"}) + err = ipammer.AddSubnetRange("2600:1700:b030:0000::/72", + vinov1.Range{Start: "2600:1700:b030:0000::", Stop: "2600:1700:b030:0009::"}) require.NoError(t, err) - err = ipammer.AddSubnetRange("192.168.0.0/1", Range{"192.168.0.0", "192.168.0.0"}) + err = ipammer.AddSubnetRange("192.168.0.0/1", + vinov1.Range{Start: "192.168.0.0", Stop: "192.168.0.0"}) require.NoError(t, err) - err = ipammer.AddSubnetRange("2600:1700:b031:0000::/64", Range{"2600:1700:b031:0000::", "2600:1700:b031:0000::"}) + err = ipammer.AddSubnetRange("2600:1700:b031:0000::/64", + vinov1.Range{Start: "2600:1700:b031:0000::", Stop: "2600:1700:b031:0000::"}) require.NoError(t, err) - _, err = ipammer.AllocateIP("192.168.0.0/1", Range{"192.168.0.0", "192.168.0.0"}) + _, err = ipammer.AllocateIP("192.168.0.0/1", vinov1.Range{Start: "192.168.0.0", Stop: "192.168.0.0"}) require.NoError(t, err) - _, err = ipammer.AllocateIP("2600:1700:b031:0000::/64", Range{"2600:1700:b031:0000::", "2600:1700:b031:0000::"}) + _, err = ipammer.AllocateIP("2600:1700:b031:0000::/64", + vinov1.Range{Start: "2600:1700:b031:0000::", Stop: "2600:1700:b031:0000::"}) require.NoError(t, err) ip, err := ipammer.AllocateIP(tt.subnet, tt.subnetRange) if tt.expectedErr != "" { @@ -108,22 +114,67 @@ func TestAllocateIP(t *testing.T) { } } +func TestNewRange(t *testing.T) { + tests := []struct { + name, start, stop, expectedErr string + }{ + { + name: "success", + start: "10.0.0.1", + stop: "10.0.0.2", + expectedErr: "", + }, + { + name: "error stop less than start", + start: "10.0.0.2", + stop: "10.0.0.1", + expectedErr: "is invalid", + }, + { + name: "error bad start", + start: "10.0.0.2.x", + stop: "10.0.0.1", + expectedErr: "is invalid", + }, + { + name: "error bad stop", + start: "10.0.0.2", + stop: "10.0.0.1.x", + expectedErr: "is invalid", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + r, err := NewRange(tt.start, tt.stop) + if tt.expectedErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedErr) + } else { + require.NoError(t, err) + assert.Equal(t, r.Start, tt.start) + assert.Equal(t, r.Stop, tt.stop) + } + }) + } +} + // Test some error handling that is not captured by TestAllocateIP func TestAddSubnetRange(t *testing.T) { tests := []struct { name, subnet, expectedErr string - subnetRange Range + subnetRange vinov1.Range }{ { name: "success", subnet: "10.0.0.0/16", - subnetRange: Range{"10.0.2.0", "10.0.2.9"}, + subnetRange: vinov1.Range{Start: "10.0.2.0", Stop: "10.0.2.9"}, expectedErr: "", }, { name: "error range already exists", subnet: "10.0.0.0/16", - subnetRange: Range{"10.0.1.0", "10.0.1.9"}, + subnetRange: vinov1.Range{Start: "10.0.1.0", Stop: "10.0.1.9"}, expectedErr: "IPAM range [10.0.1.0,10.0.1.9] in subnet 10.0.0.0/16 overlaps", }, // TODO: check for partially overlapping ranges and subnets @@ -135,7 +186,7 @@ func TestAddSubnetRange(t *testing.T) { ipammer := NewIpam() // Pre-populate IPAM with some precondition test data - err := ipammer.AddSubnetRange("10.0.0.0/16", Range{"10.0.1.0", "10.0.1.9"}) + err := ipammer.AddSubnetRange("10.0.0.0/16", vinov1.Range{Start: "10.0.1.0", Stop: "10.0.1.9"}) require.NoError(t, err) err = ipammer.AddSubnetRange(tt.subnet, tt.subnetRange) if tt.expectedErr != "" { @@ -151,33 +202,33 @@ func TestFindFreeIPInRange(t *testing.T) { tests := []struct { name string subnet string - subnetRange Range + subnetRange vinov1.Range out string expectedErr string }{ { name: "ip available IPv4", subnet: "10.0.0.0/16", - subnetRange: Range{"10.0.1.0", "10.0.1.10"}, + subnetRange: vinov1.Range{Start: "10.0.1.0", Stop: "10.0.1.10"}, out: "10.0.1.0", }, { name: "ip unavailable IPv4", subnet: "10.0.0.0/16", - subnetRange: Range{"10.0.2.0", "10.0.2.0"}, + subnetRange: vinov1.Range{Start: "10.0.2.0", Stop: "10.0.2.0"}, out: "", expectedErr: "IPAM range [10.0.2.0,10.0.2.0] in subnet 10.0.0.0/16 is exhausted", }, { name: "ip available IPv6", subnet: "2600:1700:b030:0000::/64", - subnetRange: Range{"2600:1700:b030:1001::", "2600:1700:b030:1009::"}, + subnetRange: vinov1.Range{Start: "2600:1700:b030:1001::", Stop: "2600:1700:b030:1009::"}, out: "2600:1700:b030:1001::", }, { name: "ip unavailable IPv6", subnet: "2600:1700:b031::/64", - subnetRange: Range{"2600:1700:b031::", "2600:1700:b031::"}, + subnetRange: vinov1.Range{Start: "2600:1700:b031::", Stop: "2600:1700:b031::"}, expectedErr: "IPAM range [2600:1700:b031::,2600:1700:b031::] " + "in subnet 2600:1700:b031::/64 is exhausted", }, @@ -186,14 +237,14 @@ func TestFindFreeIPInRange(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - ippool := IPPool{ + ippool := vinov1.IPPoolSpec{ Subnet: tt.subnet, // One available and one unavailable range each for ipv4/6 - Ranges: []Range{ - {"10.0.1.0", "10.0.1.10"}, - {"10.0.2.0", "10.0.2.0"}, - {"2600:1700:b030:1001::", "2600:1700:b030:1009::"}, - {"2600:1700:b031::", "2600:1700:b031::"}, + Ranges: []vinov1.Range{ + {Start: "10.0.1.0", Stop: "10.0.1.10"}, + {Start: "10.0.2.0", Stop: "10.0.2.0"}, + {Start: "2600:1700:b030:1001::", Stop: "2600:1700:b030:1009::"}, + {Start: "2600:1700:b031::", Stop: "2600:1700:b031::"}, }, AllocatedIPs: []string{"10.0.2.0", "2600:1700:b031::"}, } diff --git a/tools/deployment/test-cr.sh b/tools/deployment/test-cr.sh index 620df63..2759fc5 100755 --- a/tools/deployment/test-cr.sh +++ b/tools/deployment/test-cr.sh @@ -13,6 +13,7 @@ function vinoDebugInfo () { } kubectl apply -f config/samples/vino_cr.yaml +kubectl apply -f config/samples/ippool.yaml # Remove logs collection from here, when we will have zuul collect logs job until [[ $(kubectl get vino vino-test-cr 2>/dev/null) ]]; do