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 14, 2023
1 parent 667866d commit 423f12e
Show file tree
Hide file tree
Showing 2 changed files with 180 additions and 11 deletions.
69 changes: 60 additions & 9 deletions internal/controllers/ipaddressclaim.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@ import (
"fmt"

"github.com/pkg/errors"
"github.com/telekom/cluster-api-ipam-provider-in-cluster/api/v1alpha1"
"github.com/telekom/cluster-api-ipam-provider-in-cluster/internal/poolutil"
"github.com/telekom/cluster-api-ipam-provider-in-cluster/pkg/ipamutil"
ipampredicates "github.com/telekom/cluster-api-ipam-provider-in-cluster/pkg/predicates"
"inet.af/netaddr"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"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 All @@ -21,11 +26,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"

"github.com/telekom/cluster-api-ipam-provider-in-cluster/api/v1alpha1"
"github.com/telekom/cluster-api-ipam-provider-in-cluster/internal/poolutil"
"github.com/telekom/cluster-api-ipam-provider-in-cluster/pkg/ipamutil"
ipampredicates "github.com/telekom/cluster-api-ipam-provider-in-cluster/pkg/predicates"
)

const (
Expand Down Expand Up @@ -152,7 +152,7 @@ func (r *IPAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return r.reconcile(ctx, claim, pool, addresses)
}

func (r *IPAddressClaimReconciler) reconcile(ctx context.Context, c *ipamv1.IPAddressClaim, pool genericInClusterPool, addresses []ipamv1.IPAddress) (ctrl.Result, error) {
func (r *IPAddressClaimReconciler) reconcile(ctx context.Context, claim *ipamv1.IPAddressClaim, pool genericInClusterPool, addresses []ipamv1.IPAddress) (ctrl.Result, error) {
log := ctrl.LoggerFrom(ctx)

if pool == nil {
Expand All @@ -163,26 +163,77 @@ func (r *IPAddressClaimReconciler) reconcile(ctx context.Context, c *ipamv1.IPAd

log = log.WithValues("pool name", pool.GetName())

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

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 {
if err := r.ensureOwnerRefsAndFinalizer(ctx, address, claim, pool); err != nil {
log.Error(err, "failed to ensure owner refs and finalizer on address")
return ctrl.Result{}, err
}

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.
log.Info("Address is marked for deletion, but deletion is prevented until the claim is deleted as well.", "address", address.Name)
}

c.Status.AddressRef = corev1.LocalObjectReference{Name: address.Name}
claim.Status.AddressRef = corev1.LocalObjectReference{Name: address.Name}

return ctrl.Result{}, 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)

if !controllerutil.ContainsFinalizer(address, ProtectAddressFinalizer) {
controllerutil.AddFinalizer(address, ProtectAddressFinalizer)
}

if err := r.Update(ctx, address); err != nil {
return errors.Wrap(err, "Failed to update object refs and finalizer on IPAddress")
}

return nil
}

func (r *IPAddressClaimReconciler) reconcileDelete(ctx context.Context, c *ipamv1.IPAddressClaim, address *ipamv1.IPAddress) (ctrl.Result, error) {
if address.Name != "" {
var err error
Expand Down
122 changes: 120 additions & 2 deletions internal/controllers/ipaddressclaim_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/telekom/cluster-api-ipam-provider-in-cluster/api/v1alpha1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
clusterv1 "sigs.k8s.io/cluster-api/exp/ipam/api/v1alpha1"
. "sigs.k8s.io/controller-runtime/pkg/envtest/komega"

"github.com/telekom/cluster-api-ipam-provider-in-cluster/api/v1alpha1"
)

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 @@ -756,6 +756,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

0 comments on commit 423f12e

Please sign in to comment.