Skip to content

Commit 9b470d9

Browse files
schrejschrej
authored
Merge pull request #125 from adobley/pool-downsize-in-use-ips
Webhook Validate updated pool does not remove in use IPs
2 parents 734cfab + 549c3c6 commit 9b470d9

8 files changed

+233
-5
lines changed

‎api/v1alpha1/inclusterippool_types.go

+5
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ type InClusterIPPoolStatusIPAddresses struct {
5959
// Used is the count of allocated IPs in the pool.
6060
// Counts greater than int can contain will report as math.MaxInt.
6161
Used int `json:"used"`
62+
63+
// Out of Range is the count of allocated IPs in the pool that is not
64+
// contained within spec.Addresses.
65+
// Counts greater than int can contain will report as math.MaxInt.
66+
OutOfRange int `json:"outOfRange"`
6267
}
6368

6469
// +kubebuilder:object:root=true

‎config/crd/bases/ipam.cluster.x-k8s.io_globalinclusterippools.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ spec:
103103
description: Free is the count of unallocated IPs in the pool.
104104
Counts greater than int can contain will report as math.MaxInt.
105105
type: integer
106+
outOfRange:
107+
description: Out of Range is the count of allocated IPs in the
108+
pool that is not contained within spec.Addresses. Counts greater
109+
than int can contain will report as math.MaxInt.
110+
type: integer
106111
total:
107112
description: Total is the total number of IPs configured for the
108113
pool. Counts greater than int can contain will report as math.MaxInt.
@@ -113,6 +118,7 @@ spec:
113118
type: integer
114119
required:
115120
- free
121+
- outOfRange
116122
- total
117123
- used
118124
type: object

‎config/crd/bases/ipam.cluster.x-k8s.io_inclusterippools.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ spec:
105105
description: Free is the count of unallocated IPs in the pool.
106106
Counts greater than int can contain will report as math.MaxInt.
107107
type: integer
108+
outOfRange:
109+
description: Out of Range is the count of allocated IPs in the
110+
pool that is not contained within spec.Addresses. Counts greater
111+
than int can contain will report as math.MaxInt.
112+
type: integer
108113
total:
109114
description: Total is the total number of IPs configured for the
110115
pool. Counts greater than int can contain will report as math.MaxInt.
@@ -115,6 +120,7 @@ spec:
115120
type: integer
116121
required:
117122
- free
123+
- outOfRange
118124
- total
119125
- used
120126
type: object

‎internal/controllers/inclusterippool.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,16 @@ func genericReconcile(ctx context.Context, c client.Client, pool pooltypes.Gener
199199
}
200200

201201
free := poolCount - inUseCount
202+
outOfRangeIPSet, err := poolutil.AddressesOutOfRangeIPSet(addressesInUse, poolIPSet)
203+
if err != nil {
204+
return ctrl.Result{}, errors.Wrap(err, "failed to build out of range ip set")
205+
}
202206

203207
pool.PoolStatus().Addresses = &v1alpha1.InClusterIPPoolStatusIPAddresses{
204-
Total: poolCount,
205-
Used: inUseCount,
206-
Free: free,
208+
Total: poolCount,
209+
Used: inUseCount,
210+
Free: free,
211+
OutOfRange: poolutil.IPSetCount(outOfRangeIPSet),
207212
}
208213

209214
log.Info("Updating pool with usage info", "statusAddresses", pool.PoolStatus().Addresses)

‎internal/controllers/inclusterippool_test.go

+61
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,67 @@ var _ = Describe("IP Pool Reconciler", func() {
9999
Entry("When there is 1 claim with gateway outside of range - GlobalInClusterIPPool",
100100
"GlobalInClusterIPPool", []string{"10.0.0.10-10.0.0.20"}, "10.0.0.1", 11, 1, 10),
101101
)
102+
103+
DescribeTable("it shows the out of range ips if any",
104+
func(poolType string, addresses []string, gateway string, updatedAddresses []string, numClaims, expectedOutOfRange int) {
105+
poolSpec := v1alpha1.InClusterIPPoolSpec{
106+
Prefix: 24,
107+
Gateway: gateway,
108+
Addresses: addresses,
109+
}
110+
111+
switch poolType {
112+
case "InClusterIPPool":
113+
genericPool = &v1alpha1.InClusterIPPool{
114+
ObjectMeta: metav1.ObjectMeta{GenerateName: testPool, Namespace: namespace},
115+
Spec: poolSpec,
116+
}
117+
case "GlobalInClusterIPPool":
118+
genericPool = &v1alpha1.GlobalInClusterIPPool{
119+
ObjectMeta: metav1.ObjectMeta{GenerateName: testPool, Namespace: namespace},
120+
Spec: poolSpec,
121+
}
122+
default:
123+
Fail("Unknown pool type")
124+
}
125+
126+
Expect(k8sClient.Create(context.Background(), genericPool)).To(Succeed())
127+
128+
for i := 0; i < numClaims; i++ {
129+
claim := ipamv1.IPAddressClaim{
130+
ObjectMeta: metav1.ObjectMeta{
131+
Name: fmt.Sprintf("test%d", i),
132+
Namespace: namespace,
133+
},
134+
Spec: ipamv1.IPAddressClaimSpec{
135+
PoolRef: corev1.TypedLocalObjectReference{
136+
APIGroup: pointer.String("ipam.cluster.x-k8s.io"),
137+
Kind: poolType,
138+
Name: genericPool.GetName(),
139+
},
140+
},
141+
}
142+
Expect(k8sClient.Create(context.Background(), &claim)).To(Succeed())
143+
createdClaimNames = append(createdClaimNames, claim.Name)
144+
}
145+
146+
Eventually(Object(genericPool)).
147+
WithTimeout(5 * time.Second).WithPolling(100 * time.Millisecond).Should(
148+
HaveField("Status.Addresses.Used", Equal(numClaims)))
149+
150+
genericPool.PoolSpec().Addresses = updatedAddresses
151+
Expect(k8sClient.Update(context.Background(), genericPool)).To(Succeed())
152+
153+
Eventually(Object(genericPool)).
154+
WithTimeout(5 * time.Second).WithPolling(100 * time.Millisecond).Should(
155+
HaveField("Status.Addresses.OutOfRange", Equal(expectedOutOfRange)))
156+
},
157+
158+
Entry("InClusterIPPool",
159+
"InClusterIPPool", []string{"10.0.0.10-10.0.0.20"}, "10.0.0.1", []string{"10.0.0.13-10.0.0.20"}, 5, 3),
160+
Entry("GlobalInClusterIPPool",
161+
"GlobalInClusterIPPool", []string{"10.0.0.10-10.0.0.20"}, "10.0.0.1", []string{"10.0.0.13-10.0.0.20"}, 5, 3),
162+
)
102163
})
103164

104165
Context("when the pool has IPAddresses", func() {

‎internal/poolutil/pool.go

+16
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,22 @@ import (
1919
"github.com/telekom/cluster-api-ipam-provider-in-cluster/internal/index"
2020
)
2121

22+
// AddressesOutOfRangeIPSet returns an IPSet of the inUseAddresses IPs that are
23+
// not in the poolIPSet.
24+
func AddressesOutOfRangeIPSet(inUseAddresses []ipamv1.IPAddress, poolIPSet *netipx.IPSet) (*netipx.IPSet, error) {
25+
outOfRangeBuilder := &netipx.IPSetBuilder{}
26+
for _, address := range inUseAddresses {
27+
ip, err := netip.ParseAddr(address.Spec.Address)
28+
if err != nil {
29+
// if an address we fetch for the pool is unparsable then it isn't in the pool ranges
30+
continue
31+
}
32+
outOfRangeBuilder.Add(ip)
33+
}
34+
outOfRangeBuilder.RemoveSet(poolIPSet)
35+
return outOfRangeBuilder.IPSet()
36+
}
37+
2238
// ListAddressesInUse fetches all IPAddresses belonging to the specified pool.
2339
// Note: requires `index.ipAddressByCombinedPoolRef` to be set up.
2440
func ListAddressesInUse(ctx context.Context, c client.Reader, namespace string, poolRef corev1.TypedLocalObjectReference) ([]ipamv1.IPAddress, error) {

‎internal/webhooks/inclusterippool.go

+44-2
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func (webhook *InClusterIPPool) ValidateCreate(_ context.Context, obj runtime.Ob
110110
}
111111

112112
// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type.
113-
func (webhook *InClusterIPPool) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) error {
113+
func (webhook *InClusterIPPool) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
114114
newPool, ok := newObj.(types.GenericInClusterPool)
115115
if !ok {
116116
return apierrors.NewBadRequest(fmt.Sprintf("expected a InClusterIPPool or an GlobalInClusterIPPool but got a %T", newObj))
@@ -119,7 +119,49 @@ func (webhook *InClusterIPPool) ValidateUpdate(_ context.Context, oldObj, newObj
119119
if !ok {
120120
return apierrors.NewBadRequest(fmt.Sprintf("expected a InClusterIPPool or an GlobalInClusterIPPool but got a %T", oldObj))
121121
}
122-
return webhook.validate(oldPool, newPool)
122+
123+
err := webhook.validate(oldPool, newPool)
124+
if err != nil {
125+
return err
126+
}
127+
128+
oldPoolRef := corev1.TypedLocalObjectReference{
129+
APIGroup: pointer.String(v1alpha1.GroupVersion.Group),
130+
Kind: oldPool.GetObjectKind().GroupVersionKind().Kind,
131+
Name: oldPool.GetName(),
132+
}
133+
inUseAddresses, err := poolutil.ListAddressesInUse(ctx, webhook.Client, oldPool.GetNamespace(), oldPoolRef)
134+
if err != nil {
135+
return apierrors.NewInternalError(err)
136+
}
137+
138+
inUseBuilder := &netipx.IPSetBuilder{}
139+
for _, address := range inUseAddresses {
140+
ip, err := netip.ParseAddr(address.Spec.Address)
141+
if err != nil {
142+
// if an address we fetch for the pool is unparsable then it isn't in the pool ranges
143+
continue
144+
}
145+
inUseBuilder.Add(ip)
146+
}
147+
148+
newPoolIPSet, err := poolutil.AddressesToIPSet(newPool.PoolSpec().Addresses)
149+
if err != nil {
150+
// these addresses are already validated, this shouldn't happen
151+
return apierrors.NewInternalError(err)
152+
}
153+
154+
inUseBuilder.RemoveSet(newPoolIPSet)
155+
outOfRangeIPSet, err := inUseBuilder.IPSet()
156+
if err != nil {
157+
return apierrors.NewInternalError(err)
158+
}
159+
160+
if outOfRange := outOfRangeIPSet.Ranges(); len(outOfRange) > 0 {
161+
return apierrors.NewBadRequest(fmt.Sprintf("pool addresses does not contain allocated addresses: %v", outOfRange))
162+
}
163+
164+
return nil
123165
}
124166

125167
// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type.

‎internal/webhooks/inclusterippool_test.go

+87
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ func TestPoolDeletionWithExistingIPAddresses(t *testing.T) {
6161
Kind: namespacedPool.GetObjectKind().GroupVersionKind().Kind,
6262
Name: namespacedPool.GetName(),
6363
},
64+
Address: "10.0.0.10",
6465
},
6566
},
6667
&ipamv1.IPAddress{
@@ -77,6 +78,7 @@ func TestPoolDeletionWithExistingIPAddresses(t *testing.T) {
7778
Kind: globalPool.GetObjectKind().GroupVersionKind().Kind,
7879
Name: globalPool.GetName(),
7980
},
81+
Address: "10.0.0.10",
8082
},
8183
},
8284
}
@@ -98,6 +100,91 @@ func TestPoolDeletionWithExistingIPAddresses(t *testing.T) {
98100

99101
g.Expect(webhook.ValidateDelete(ctx, namespacedPool)).To(Succeed(), "should allow deletion when no claims exist")
100102
g.Expect(webhook.ValidateDelete(ctx, globalPool)).To(Succeed(), "should allow deletion when no claims exist")
103+
104+
}
105+
106+
func TestUpdatingPoolInUseAddresses(t *testing.T) {
107+
g := NewWithT(t)
108+
109+
scheme := runtime.NewScheme()
110+
g.Expect(ipamv1.AddToScheme(scheme)).To(Succeed())
111+
112+
namespacedPool := &v1alpha1.InClusterIPPool{
113+
ObjectMeta: metav1.ObjectMeta{
114+
Name: "my-pool",
115+
},
116+
Spec: v1alpha1.InClusterIPPoolSpec{
117+
Addresses: []string{"10.0.0.10-10.0.0.20"},
118+
Prefix: 24,
119+
Gateway: "10.0.0.1",
120+
},
121+
}
122+
123+
globalPool := &v1alpha1.InClusterIPPool{
124+
ObjectMeta: metav1.ObjectMeta{
125+
Name: "my-pool",
126+
},
127+
Spec: v1alpha1.InClusterIPPoolSpec{
128+
Addresses: []string{"10.0.0.10-10.0.0.20"},
129+
Prefix: 24,
130+
Gateway: "10.0.0.1",
131+
},
132+
}
133+
134+
ips := []client.Object{
135+
&ipamv1.IPAddress{
136+
TypeMeta: metav1.TypeMeta{
137+
Kind: "IPAddress",
138+
APIVersion: "ipam.cluster.x-k8s.io/v1alpha1",
139+
},
140+
ObjectMeta: metav1.ObjectMeta{
141+
Name: "my-ip",
142+
},
143+
Spec: ipamv1.IPAddressSpec{
144+
PoolRef: corev1.TypedLocalObjectReference{
145+
APIGroup: pointer.String(namespacedPool.GetObjectKind().GroupVersionKind().Group),
146+
Kind: namespacedPool.GetObjectKind().GroupVersionKind().Kind,
147+
Name: namespacedPool.GetName(),
148+
},
149+
Address: "10.0.0.10",
150+
},
151+
},
152+
&ipamv1.IPAddress{
153+
TypeMeta: metav1.TypeMeta{
154+
Kind: "IPAddress",
155+
APIVersion: "ipam.cluster.x-k8s.io/v1alpha1",
156+
},
157+
ObjectMeta: metav1.ObjectMeta{
158+
Name: "my-ip-2",
159+
},
160+
Spec: ipamv1.IPAddressSpec{
161+
PoolRef: corev1.TypedLocalObjectReference{
162+
APIGroup: pointer.String(globalPool.GetObjectKind().GroupVersionKind().Group),
163+
Kind: globalPool.GetObjectKind().GroupVersionKind().Kind,
164+
Name: globalPool.GetName(),
165+
},
166+
Address: "10.0.0.10",
167+
},
168+
},
169+
}
170+
171+
fakeClient := fake.NewClientBuilder().
172+
WithScheme(scheme).
173+
WithObjects(ips...).
174+
WithIndex(&ipamv1.IPAddress{}, index.IPAddressPoolRefCombinedField, index.IPAddressByCombinedPoolRef).
175+
Build()
176+
177+
webhook := InClusterIPPool{
178+
Client: fakeClient,
179+
}
180+
181+
oldNamespacedPool := namespacedPool.DeepCopyObject()
182+
oldGlobalPool := globalPool.DeepCopyObject()
183+
namespacedPool.Spec.Addresses = []string{"10.0.0.15-10.0.0.20"}
184+
globalPool.Spec.Addresses = []string{"10.0.0.15-10.0.0.20"}
185+
186+
g.Expect(webhook.ValidateUpdate(ctx, oldNamespacedPool, namespacedPool)).NotTo(Succeed(), "should not allow removing in use IPs from addresses field in pool")
187+
g.Expect(webhook.ValidateUpdate(ctx, oldGlobalPool, globalPool)).NotTo(Succeed(), "should not allow removing in use IPs from addresses field in pool")
101188
}
102189

103190
func TestInClusterIPPoolDefaulting(t *testing.T) {

0 commit comments

Comments
 (0)