diff --git a/internal/controllers/ipaddressclaim.go b/internal/controllers/ipaddressclaim.go index 8b7dfa6..711a8b9 100644 --- a/internal/controllers/ipaddressclaim.go +++ b/internal/controllers/ipaddressclaim.go @@ -5,6 +5,10 @@ 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" @@ -12,6 +16,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" @@ -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 ( @@ -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 { @@ -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 diff --git a/internal/controllers/ipaddressclaim_test.go b/internal/controllers/ipaddressclaim_test.go index a271303..439c201 100644 --- a/internal/controllers/ipaddressclaim_test.go +++ b/internal/controllers/ipaddressclaim_test.go @@ -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", } @@ -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 {