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]>
Signed-off-by: Tyler Schultz <[email protected]>
  • Loading branch information
@tylerschultz @flawedmatrix
3 people committed Mar 23, 2023
1 parent c857081 commit 278b5c4
Show file tree
Hide file tree
Showing 3 changed files with 200 additions and 21 deletions.
90 changes: 81 additions & 9 deletions internal/controllers/ipaddressclaim.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/utils/pointer"
ipamv1 "sigs.k8s.io/cluster-api/exp/ipam/api/v1alpha1"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/cluster-api/util/predicates"
Expand Down Expand Up @@ -165,12 +166,27 @@ func (r *IPAddressClaimReconciler) reconcile(ctx context.Context, claim *ipamv1.
address := poolutil.AddressByName(addressesInUse, claim.Name)
if address == nil {
var err error
address, err = r.allocateAddress(ctx, claim, pool, addressesInUse)
address, err = r.createAddress(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 create address")
}

log.Info("Allocated IPAddress for IPAddressClaim",
"IPAddressClaim", fmt.Sprintf("%s/%s", claim.Namespace, claim.Name),
"IPAddress", fmt.Sprintf("%s/%s %s", address.Namespace, address.Name, address.Spec.Address),
"InClusterIPPool", pool.GetName())
} else {
err := r.patchAddressOwnerRefsAndFinalizer(ctx, address, claim, pool)
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to patch address ownerRefs and finalizer")
}

log.Info("Found existing IPAddress for IPAddressClaim",
"IPAddressClaim", fmt.Sprintf("%s/%s", claim.Namespace, claim.Name),
"IPAddress", fmt.Sprintf("%s/%s %s", address.Namespace, address.Name, address.Spec.Address),
"InClusterIPPool", pool.GetName())
}

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 All @@ -182,6 +198,68 @@ func (r *IPAddressClaimReconciler) reconcile(ctx context.Context, claim *ipamv1.
return ctrl.Result{}, nil
}

func (r *IPAddressClaimReconciler) patchAddressOwnerRefsAndFinalizer(ctx context.Context, address *ipamv1.IPAddress, claim *ipamv1.IPAddressClaim, pool genericInClusterPool) error {
patchHelper, err := patch.NewHelper(address, r.Client)
if err != nil {
return errors.Wrap(err, "failed to initialize patch helper")
}

if err := r.ensureOwnerRefsAndFinalizer(ctx, address, claim, pool); err != nil {
return errors.Wrap(err, "failed to ensure owner refs and finalizer on address")
}

if err := patchHelper.Patch(ctx, address); err != nil {
return errors.Wrap(err, "failed to patch address")
}
return nil
}

func (r *IPAddressClaimReconciler) createAddress(ctx context.Context, claim *ipamv1.IPAddressClaim, pool genericInClusterPool, addressesInUse []ipamv1.IPAddress) (*ipamv1.IPAddress, error) {
address, err := r.allocateAddress(ctx, claim, pool, addressesInUse)
if err != nil {
return nil, errors.Wrap(err, "failed to allocate address")
}

if err := r.ensureOwnerRefsAndFinalizer(ctx, address, claim, pool); err != nil {
return nil, errors.Wrap(err, "failed to ensure owner refs and finalizer on address")
}

if err := r.Client.Create(ctx, address); err != nil {
return nil, errors.Wrap(err, "failed to create IPAddress")
}

return address, nil
}

func (r *IPAddressClaimReconciler) ensureOwnerRefsAndFinalizer(ctx context.Context, address *ipamv1.IPAddress, claim *ipamv1.IPAddressClaim, pool genericInClusterPool) error {
if err := controllerutil.SetControllerReference(claim, address, r.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, r.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)

_ = controllerutil.AddFinalizer(address, ProtectAddressFinalizer)

return nil
}

func (r *IPAddressClaimReconciler) reconcileDelete(ctx context.Context, claim *ipamv1.IPAddressClaim, address *ipamv1.IPAddress) (ctrl.Result, error) {
if address.Name != "" {
var err error
Expand Down Expand Up @@ -224,11 +302,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
}
119 changes: 119 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,124 @@ var _ = Describe("IPAddressClaimReconciler", func() {
})
})
})

Context("When an existing IPAddress is missing finalizers and owner refs", 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
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 278b5c4

Please sign in to comment.