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 2d45fb5
Show file tree
Hide file tree
Showing 3 changed files with 276 additions and 22 deletions.
32 changes: 21 additions & 11 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)
address, err = r.allocateAddress(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 := ipamutil.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 @@ -202,7 +218,7 @@ func (r *IPAddressClaimReconciler) reconcileDelete(ctx context.Context, claim *i
return ctrl.Result{}, nil
}

func (r *IPAddressClaimReconciler) allocateAddress(ctx context.Context, claim *ipamv1.IPAddressClaim, pool genericInClusterPool, addressesInUse []ipamv1.IPAddress) (*ipamv1.IPAddress, error) {
func (r *IPAddressClaimReconciler) allocateAddress(claim *ipamv1.IPAddressClaim, pool genericInClusterPool, addressesInUse []ipamv1.IPAddress) (*ipamv1.IPAddress, error) {
poolSpec := pool.PoolSpec()
inUseIPSet, err := poolutil.IPAddressListToSet(addressesInUse, poolSpec.Gateway)
if err != nil {
Expand All @@ -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
43 changes: 32 additions & 11 deletions pkg/ipamutil/address.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
package ipamutil

import (
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"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"
)

// NewIPAddress creates a new ipamv1.IPAddress with references to a pool and claim.
Expand All @@ -17,17 +20,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 All @@ -41,3 +33,32 @@ func NewIPAddress(claim *ipamv1.IPAddressClaim, pool client.Object) ipamv1.IPAdd
},
}
}

// 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
}

0 comments on commit 2d45fb5

Please sign in to comment.