Skip to content

Commit

Permalink
Ensure existing IPAddresses finalizer and ownerRefs
Browse files Browse the repository at this point in the history 
Ensure IPAddresses have the expected finalizer
and Owner References to their respective InClusterIPPool and
IPAddress Claims. A case where this may happen is when a cluster has
been restored from backup.

Co-authored-by: Edwin Xie <[email protected]>
Co-authored-by: Aidan Obley <[email protected]>
Co-authored-by: Christian Ang <[email protected]>
Signed-off-by: Tyler Schultz <[email protected]>
  • Loading branch information
@tylerschultz @flawedmatrix @christianang
4 people committed Mar 24, 2023
1 parent c857081 commit 5b6be98
Show file tree
Hide file tree
Showing 4 changed files with 275 additions and 22 deletions.
28 changes: 19 additions & 9 deletions internal/controllers/ipaddressclaim.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,33 @@ func (r *IPAddressClaimReconciler) reconcile(ctx context.Context, claim *ipamv1.
return ctrl.Result{}, nil
}

log = log.WithValues("pool name", pool.GetName())
log = log.WithValues(pool.GetObjectKind().GroupVersionKind().Kind, fmt.Sprintf("%s/%s", pool.GetNamespace(), pool.GetName()))

address := poolutil.AddressByName(addressesInUse, claim.Name)
if address == nil {
var err error
address, err = r.allocateAddress(ctx, claim, pool, addressesInUse)
if err != nil {
log.Error(err, "failed to allocate address")
return ctrl.Result{}, err
return ctrl.Result{}, errors.Wrap(err, "failed to allocate address")
}
}

operationResult, err := controllerutil.CreateOrPatch(ctx, r.Client, address, func() error {
if err := poolutil.EnsureIPAddressOwnerReferences(r.Scheme, address, claim, pool); err != nil {
return errors.Wrap(err, "failed to ensure owner references on address")
}

_ = controllerutil.AddFinalizer(address, ProtectAddressFinalizer)

return nil
})
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to create or patch address")
}

log.Info(fmt.Sprintf("IPAddress %s/%s (%s) has been %s", address.Namespace, address.Name, address.Spec.Address, operationResult),
"IPAddressClaim", fmt.Sprintf("%s/%s", claim.Namespace, claim.Name))

if !address.DeletionTimestamp.IsZero() {
// We prevent deleting IPAddresses while their corresponding IPClaim still exists since we cannot guarantee that the IP
// wil remain the same when we recreate it.
Expand Down Expand Up @@ -224,11 +240,5 @@ func (r *IPAddressClaimReconciler) allocateAddress(ctx context.Context, claim *i
address.Spec.Gateway = poolSpec.Gateway
address.Spec.Prefix = poolSpec.Prefix

controllerutil.AddFinalizer(&address, ProtectAddressFinalizer)

if err := r.Client.Create(ctx, &address); err != nil {
return nil, fmt.Errorf("failed to create IPAddress: %w", err)
}

return &address, nil
}
223 changes: 223 additions & 0 deletions internal/controllers/ipaddressclaim_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var IgnoreUIDsOnIPAddress = IgnorePaths{
"TypeMeta",
"ObjectMeta.OwnerReferences[0].UID",
"ObjectMeta.OwnerReferences[1].UID",
"ObjectMeta.OwnerReferences[2].UID",
"Spec.Claim.UID",
"Spec.Pool.UID",
}
Expand Down Expand Up @@ -852,6 +853,228 @@ var _ = Describe("IPAddressClaimReconciler", func() {
})
})
})

Context("When an existing IPAddress with no ownerReferences is missing finalizers and owner references", func() {
const poolName = "test-pool"

BeforeEach(func() {
pool := v1alpha1.InClusterIPPool{
ObjectMeta: metav1.ObjectMeta{
Name: poolName,
Namespace: namespace,
},
Spec: v1alpha1.InClusterIPPoolSpec{
First: "10.0.0.1",
Last: "10.0.0.254",
Prefix: 24,
Gateway: "10.0.0.2",
},
}
Expect(k8sClient.Create(context.Background(), &pool)).To(Succeed())
Eventually(Get(&pool)).Should(Succeed())
})

AfterEach(func() {
deleteNamespacedPool(poolName, namespace)
deleteClaim("test", namespace)
})

It("should add the owner references and finalizer", func() {
claim := clusterv1.IPAddressClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: namespace,
},
Spec: clusterv1.IPAddressClaimSpec{
PoolRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String("ipam.cluster.x-k8s.io"),
Kind: "InClusterIPPool",
Name: poolName,
},
},
}

addressSpec := clusterv1.IPAddressSpec{
ClaimRef: corev1.LocalObjectReference{
Name: "test",
},
PoolRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String("ipam.cluster.x-k8s.io"),
Kind: "InClusterIPPool",
Name: poolName,
},
Address: "10.0.0.1",
Prefix: 24,
Gateway: "10.0.0.2",
}

address := clusterv1.IPAddress{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: namespace,
},
Spec: addressSpec,
}

Expect(k8sClient.Create(context.Background(), &address)).To(Succeed())
Expect(k8sClient.Create(context.Background(), &claim)).To(Succeed())

expectedIPAddress := clusterv1.IPAddress{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: namespace,
Finalizers: []string{ProtectAddressFinalizer},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "ipam.cluster.x-k8s.io/v1alpha1",
BlockOwnerDeletion: pointer.Bool(true),
Controller: pointer.Bool(true),
Kind: "IPAddressClaim",
Name: "test",
},
{
APIVersion: "ipam.cluster.x-k8s.io/v1alpha1",
BlockOwnerDeletion: pointer.Bool(true),
Controller: pointer.Bool(false),
Kind: "InClusterIPPool",
Name: poolName,
},
},
},
Spec: addressSpec,
}

actual := clusterv1.IPAddress{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: namespace,
},
}

Eventually(Object(&actual)).
WithTimeout(time.Second).WithPolling(100 * time.Millisecond).Should(
EqualObject(&expectedIPAddress, IgnoreAutogeneratedMetadata, IgnoreUIDsOnIPAddress),
)
})
})

Context("When an existing IPAddress with an unrelated ownerRef is missing finalizers and IPAM owner references", func() {
const poolName = "test-pool"

BeforeEach(func() {
pool := v1alpha1.InClusterIPPool{
ObjectMeta: metav1.ObjectMeta{
Name: poolName,
Namespace: namespace,
},
Spec: v1alpha1.InClusterIPPoolSpec{
First: "10.0.0.1",
Last: "10.0.0.254",
Prefix: 24,
Gateway: "10.0.0.2",
},
}
Expect(k8sClient.Create(context.Background(), &pool)).To(Succeed())
Eventually(Get(&pool)).Should(Succeed())
})

AfterEach(func() {
deleteNamespacedPool(poolName, namespace)
deleteClaim("test", namespace)
})

It("should add the owner references and finalizer", func() {
claim := clusterv1.IPAddressClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: namespace,
},
Spec: clusterv1.IPAddressClaimSpec{
PoolRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String("ipam.cluster.x-k8s.io"),
Kind: "InClusterIPPool",
Name: poolName,
},
},
}

addressSpec := clusterv1.IPAddressSpec{
ClaimRef: corev1.LocalObjectReference{
Name: "test",
},
PoolRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String("ipam.cluster.x-k8s.io"),
Kind: "InClusterIPPool",
Name: poolName,
},
Address: "10.0.0.1",
Prefix: 24,
Gateway: "10.0.0.2",
}

address := clusterv1.IPAddress{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: namespace,
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "alpha-dummy",
Kind: "dummy-kind",
Name: "dummy-name",
UID: "abc-dummy-123",
},
},
},
Spec: addressSpec,
}

Expect(k8sClient.Create(context.Background(), &address)).To(Succeed())
Expect(k8sClient.Create(context.Background(), &claim)).To(Succeed())

expectedIPAddress := clusterv1.IPAddress{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: namespace,
Finalizers: []string{ProtectAddressFinalizer},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "alpha-dummy",
Kind: "dummy-kind",
Name: "dummy-name",
UID: "abc-dummy-123",
},
{
APIVersion: "ipam.cluster.x-k8s.io/v1alpha1",
BlockOwnerDeletion: pointer.Bool(true),
Controller: pointer.Bool(true),
Kind: "IPAddressClaim",
Name: "test",
},
{
APIVersion: "ipam.cluster.x-k8s.io/v1alpha1",
BlockOwnerDeletion: pointer.Bool(true),
Controller: pointer.Bool(false),
Kind: "InClusterIPPool",
Name: poolName,
},
},
},
Spec: addressSpec,
}

actual := clusterv1.IPAddress{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: namespace,
},
}

Eventually(Object(&actual)).
WithTimeout(time.Second).WithPolling(100 * time.Millisecond).Should(
EqualObject(&expectedIPAddress, IgnoreAutogeneratedMetadata, IgnoreUIDsOnIPAddress),
)
})
})
})

func createNamespace() string {
Expand Down
34 changes: 33 additions & 1 deletion internal/poolutil/pool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@ package poolutil

import (
"context"
"errors"
"fmt"
"strings"

"github.com/pkg/errors"
"inet.af/netaddr"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/utils/pointer"
ipamv1 "sigs.k8s.io/cluster-api/exp/ipam/api/v1alpha1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/telekom/cluster-api-ipam-provider-in-cluster/api/v1alpha1"
"github.com/telekom/cluster-api-ipam-provider-in-cluster/internal/index"
Expand Down Expand Up @@ -167,3 +170,32 @@ func AddressStrParses(addressStr string) bool {
_, err := netaddr.ParseIP(addressStr)
return err == nil
}

// EnsureIPAddressOwnerReferences ensures that an IPAddress has the
// IPAddressClaim and IPPool as an OwnerReference.
func EnsureIPAddressOwnerReferences(scheme *runtime.Scheme, address *ipamv1.IPAddress, claim *ipamv1.IPAddressClaim, pool client.Object) error {
if err := controllerutil.SetControllerReference(claim, address, scheme); err != nil {
if _, ok := err.(*controllerutil.AlreadyOwnedError); !ok {
return errors.Wrap(err, "Failed to update address's claim owner reference")
}
}

if err := controllerutil.SetOwnerReference(pool, address, scheme); err != nil {
return errors.Wrap(err, "Failed to update address's pool owner reference")
}

var poolRefIdx int
poolGVK := pool.GetObjectKind().GroupVersionKind()
for i, ownerRef := range address.GetOwnerReferences() {
if ownerRef.APIVersion == poolGVK.GroupVersion().String() &&
ownerRef.Kind == poolGVK.Kind &&
ownerRef.Name == pool.GetName() {
poolRefIdx = i
}
}

address.OwnerReferences[poolRefIdx].Controller = pointer.Bool(false)
address.OwnerReferences[poolRefIdx].BlockOwnerDeletion = pointer.Bool(true)

return nil
}
12 changes: 0 additions & 12 deletions pkg/ipamutil/address.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package ipamutil
import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
ipamv1 "sigs.k8s.io/cluster-api/exp/ipam/api/v1alpha1"
"sigs.k8s.io/controller-runtime/pkg/client"
)
Expand All @@ -17,17 +16,6 @@ func NewIPAddress(claim *ipamv1.IPAddressClaim, pool client.Object) ipamv1.IPAdd
ObjectMeta: metav1.ObjectMeta{
Name: claim.Name,
Namespace: claim.Namespace,
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(claim, claim.GetObjectKind().GroupVersionKind()),
{
APIVersion: pool.GetObjectKind().GroupVersionKind().GroupVersion().String(),
Kind: pool.GetObjectKind().GroupVersionKind().Kind,
Name: pool.GetName(),
UID: pool.GetUID(),
BlockOwnerDeletion: pointer.Bool(true),
Controller: pointer.Bool(false),
},
},
},
Spec: ipamv1.IPAddressSpec{
ClaimRef: corev1.LocalObjectReference{
Expand Down

0 comments on commit 5b6be98

Please sign in to comment.