Skip to content

Commit

Permalink
lbipam: Remove cidrs field from CiliumLoadBalancerIPPool
Browse files Browse the repository at this point in the history
Originally the CiliumLoadBalancerIPPool only had CIDRs, and thus a field
called `cidrs` containing a list of CIDRs. In v1.15 it became possible
to specify ranges of IPs with a start and end IP. A new field was added
called `blocks` which was treated as an alias of `cidrs`. This was done
to allow for a smooth transition. This commit removes the `cidrs` field
to complete the transition.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
  • Loading branch information
dylandreimerink committed Jun 19, 2024
1 parent f27470f commit 6aa4fa8
Show file tree
Hide file tree
Showing 13 changed files with 16 additions and 491 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/conformance-gateway-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ jobs:
metadata:
name: "pool"
spec:
cidrs:
blocks:
- cidr: "$LB_CIDR"
EOF
cat pool.yaml
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/conformance-ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ jobs:
metadata:
name: "pool"
spec:
cidrs:
blocks:
- cidr: "$LB_CIDR"
EOF
cat pool.yaml
Expand Down
4 changes: 4 additions & 0 deletions Documentation/operations/upgrade.rst
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,10 @@ Annotations:
This means that unless explicitly configured otherwise, the first and last IP addresses of the IP pool
are available for allocation. If you rely on the previous behavior, you should explicitly set
``allowFirstLastIPs: no`` in your IP pool configuration before the upgrade.
* The ``CiliumLoadBalancerIPPool.spec.cidrs`` field has been deprecated in v1.15 favor of
``CiliumLoadBalancerIPPool.spec.blocks``. As of v1.15 both fields have the same behavior. The
``cidrs`` field will be removed in v1.16. Please update your IP pool configurations to use
``blocks`` instead of ``cidrs`` before upgrading.

Removed Options
~~~~~~~~~~~~~~~
Expand Down
6 changes: 2 additions & 4 deletions operator/pkg/lbipam/lbipam.go
Original file line number Diff line number Diff line change
Expand Up @@ -1292,8 +1292,7 @@ func (ipam *LBIPAM) handleNewPool(ctx context.Context, pool *cilium_api_v2alpha1
}

ipam.pools[pool.GetName()] = pool
blocks := append(pool.Spec.Cidrs, pool.Spec.Blocks...)
for _, ipBlock := range blocks {
for _, ipBlock := range pool.Spec.Blocks {
from, to, fromCidr, err := ipRangeFromBlock(ipBlock)
if err != nil {
return fmt.Errorf("Error parsing ip block: %w", err)
Expand Down Expand Up @@ -1366,8 +1365,7 @@ func (ipam *LBIPAM) handlePoolModified(ctx context.Context, pool *cilium_api_v2a
fromCidr bool
}
var newRanges []rng
blocks := append(pool.Spec.Cidrs, pool.Spec.Blocks...)
for _, newBlock := range blocks {
for _, newBlock := range pool.Spec.Blocks {
from, to, fromCidr, err := ipRangeFromBlock(newBlock)
if err != nil {
return fmt.Errorf("Error parsing ip block: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion operator/pkg/lbipam/lbipam_fixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func mkPool(uid types.UID, name string, cidrs []string) *cilium_api_v2alpha1.Cil
CreationTimestamp: metav1.Date(2022, 10, 16, 12, 00, 00, 0, time.UTC),
},
Spec: cilium_api_v2alpha1.CiliumLoadBalancerIPPoolSpec{
Cidrs: blocks,
Blocks: blocks,
},
}
}
10 changes: 5 additions & 5 deletions operator/pkg/lbipam/lbipam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestConflictResolution(t *testing.T) {
// Phase 2, resolving the conflict

// Remove the conflicting range
poolB.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
poolB.Spec.Blocks = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
{
Cidr: cilium_api_v2alpha1.IPv4orIPv6CIDR("FF::0/48"),
},
Expand All @@ -74,7 +74,7 @@ func TestPoolInternalConflict(t *testing.T) {
t.Fatal("Pool A should be conflicting")
}

poolA.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
poolA.Spec.Blocks = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
{
Cidr: "10.0.10.0/24",
},
Expand Down Expand Up @@ -1361,7 +1361,7 @@ func TestAddRange(t *testing.T) {
}

poolA = fixture.GetPool("pool-a")
poolA.Spec.Cidrs = append(poolA.Spec.Cidrs, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
poolA.Spec.Blocks = append(poolA.Spec.Blocks, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
Cidr: "10.0.20.0/24",
})
fixture.UpsertPool(t, poolA)
Expand Down Expand Up @@ -1523,7 +1523,7 @@ func TestRangeDelete(t *testing.T) {

poolA = fixture.GetPool("pool-a")
// Add a new CIDR, this should not have any effect on the existing service.
poolA.Spec.Cidrs = append(poolA.Spec.Cidrs, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
poolA.Spec.Blocks = append(poolA.Spec.Blocks, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
Cidr: "10.0.20.0/24",
})
fixture.UpsertPool(t, poolA)
Expand All @@ -1540,7 +1540,7 @@ func TestRangeDelete(t *testing.T) {

poolA = fixture.GetPool("pool-a")
// Remove the existing range, this should trigger the re-allocation of the existing service
poolA.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
poolA.Spec.Blocks = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
{
Cidr: "10.0.20.0/24",
},
Expand Down
18 changes: 0 additions & 18 deletions pkg/bgpv1/manager/reconciler/route_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,24 +251,6 @@ func (r *RoutePolicyReconciler) pathAttributesToPolicy(attrs v2alpha1api.CiliumB
}
prefixesSeen.Insert(cidr)
}
// Note: CiliumLoadBalancerIPPool.Spec.Cidrs was deprecated as of
// https://github.com/cilium/cilium/commit/27322f3959c3fa05b9b1c4f9827527b4a3642687
// It was replaced by CiliumLoadBalancerIPPool.Spec.Blocks.
for _, cidrBlock := range pool.Spec.Cidrs {
cidr, err := netip.ParsePrefix(string(cidrBlock.Cidr))
if err != nil {
return nil, fmt.Errorf("failed to parse IPAM pool CIDR %s: %w", cidrBlock.Cidr, err)
}
// If the same prefix was specified in Spec.Blocks and Spec.Cidrs, ignore the duplicate.
if prefixesSeen.Has(cidr) {
continue
}
if cidr.Addr().Is4() {
v4Prefixes = append(v4Prefixes, &types.RoutePolicyPrefixMatch{CIDR: cidr, PrefixLenMin: maxPrefixLenIPv4, PrefixLenMax: maxPrefixLenIPv4})
} else {
v6Prefixes = append(v6Prefixes, &types.RoutePolicyPrefixMatch{CIDR: cidr, PrefixLenMin: maxPrefixLenIPv6, PrefixLenMax: maxPrefixLenIPv6})
}
}
}
case v2alpha1api.PodCIDRSelectorName:
if attrs.Selector != nil && !labelSelector.Matches(labels.Set(params.CiliumNode.Labels)) {
Expand Down
Loading

0 comments on commit 6aa4fa8

Please sign in to comment.