Skip to content

Commit

Permalink
fix: remove owner references on internal nodes
Browse files Browse the repository at this point in the history
This patch removes the owner reference of the physical node from the
internal node, as this caused some issues with the tear down with
ArgoCD. As the physical node does not belong to the application, the
InternalNode (owned by it) and all the resources owned by it were not
removed. The garbage collection of the InternalNode resources is
performed by a controller instead.
  • Loading branch information
claudiolor authored and adamjensenbot committed Dec 17, 2024
1 parent ddafea0 commit 971bdf5
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 16 deletions.
6 changes: 4 additions & 2 deletions cmd/liqo-controller-manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func main() {
ipamClient = ipam.NewIpamClient(connection)
}

if err := modules.SetupNetworkingModule(ctx, mgr, &modules.NetworkingOption{
opts := &modules.NetworkingOption{
DynClient: dynClient,
Factory: factory,

Expand All @@ -291,7 +291,9 @@ func main() {
GwmasqbypassEnabled: *gwmasqbypassEnabled,

GenevePort: *genevePort,
}); err != nil {
}

if err := modules.SetupNetworkingModule(ctx, mgr, uncachedClient, opts); err != nil {
klog.Fatalf("Unable to setup the networking module: %v", err)
}
}
Expand Down
9 changes: 8 additions & 1 deletion cmd/liqo-controller-manager/modules/networking.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"k8s.io/client-go/dynamic"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager"

"github.com/liqotech/liqo/pkg/ipam"
Expand Down Expand Up @@ -61,7 +62,7 @@ type NetworkingOption struct {
}

// SetupNetworkingModule setup the networking module and initializes its controllers .
func SetupNetworkingModule(ctx context.Context, mgr manager.Manager, opts *NetworkingOption) error {
func SetupNetworkingModule(ctx context.Context, mgr manager.Manager, uncachedClient client.Client, opts *NetworkingOption) error {
networkReconciler := networkctrl.NewNetworkReconciler(mgr.GetClient(), mgr.GetScheme(), opts.IpamClient)
if err := networkReconciler.SetupWithManager(mgr, opts.NetworkWorkers); err != nil {
klog.Errorf("Unable to start the networkReconciler: %v", err)
Expand Down Expand Up @@ -156,6 +157,12 @@ func SetupNetworkingModule(ctx context.Context, mgr manager.Manager, opts *Netwo
return err
}

// Before starting the Node reconciler, make sure that there are no "orphan" InternalNode resources.
if err := nodecontroller.SyncInternalNodes(ctx, uncachedClient); err != nil {
klog.Errorf("Unable to perform InternalNode synchronization: %v", err)
return err
}

nodeReconciler := nodecontroller.NewNodeReconciler(mgr.GetClient(), mgr.GetScheme(), opts.LiqoNamespace)
if err := nodeReconciler.SetupWithManager(mgr); err != nil {
klog.Errorf("Unable to start the nodeReconciler: %v", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ import (
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"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"

networkingv1beta1 "github.com/liqotech/liqo/apis/networking/v1beta1"
Expand Down Expand Up @@ -63,13 +63,24 @@ func NewNodeReconciler(cl client.Client, s *runtime.Scheme, liqoNamespace string
// Reconcile manage Node lifecycle.
func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, err error) {
node := &corev1.Node{}
if err = r.Get(ctx, req.NamespacedName, node); err != nil {
if apierrors.IsNotFound(err) {
klog.Infof("Node %q not found", req.Name)
return ctrl.Result{}, nil
}
internalNode := &networkingv1beta1.InternalNode{
ObjectMeta: metav1.ObjectMeta{
Name: req.Name,
},
}

if err = r.Get(ctx, req.NamespacedName, node); client.IgnoreNotFound(err) != nil {
klog.Errorf("Unable to get the Node %q: %s", req.Name, err)
return ctrl.Result{}, err
} else if apierrors.IsNotFound(err) || !node.DeletionTimestamp.IsZero() {
// If node has been deleted we need to remove the InternalNode resource
klog.Infof("Deleting InternalNode %v as there is no corresponding Node resource", req.Name)

if err := r.Client.Delete(ctx, internalNode); err != nil {
return ctrl.Result{}, fmt.Errorf("unable to delete InternalNode %v: %w", req.Name, err)
}

return ctrl.Result{}, nil
}

cmDep, err := getters.GetControllerManagerDeployment(ctx, r.Client, r.liqoNamespace)
Expand All @@ -87,11 +98,6 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res c
return ctrl.Result{}, fmt.Errorf("unable to initialize the IPAM: %w", err)
}

internalNode := &networkingv1beta1.InternalNode{
ObjectMeta: metav1.ObjectMeta{
Name: node.Name,
},
}
if _, err = resource.CreateOrUpdate(ctx, r.Client, internalNode, func() error {
if internalNode.Spec.Interface.Gateway.Name, err = internalnetwork.FindFreeInterfaceName(ctx, r.Client, internalNode); err != nil {
return err
Expand All @@ -103,7 +109,7 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res c
}
internalNode.Spec.Interface.Node.IP = networkingv1beta1.IP(ip.String())

return controllerutil.SetControllerReference(node, internalNode, r.Scheme)
return nil
}); err != nil {
klog.Errorf("Unable to create or update InternalNode %q: %s", internalNode.Name, err)
return ctrl.Result{}, err
Expand All @@ -121,8 +127,39 @@ func (r *NodeReconciler) SetupWithManager(mgr ctrl.Manager) error {
if err != nil {
return err
}

return ctrl.NewControllerManagedBy(mgr).Named(consts.CtrlNode).
Owns(&networkingv1beta1.InternalNode{}).
// We need to reconcile only physical Nodes as we need to apply the networking rules for each of them.
For(&corev1.Node{}, builder.WithPredicates(predicate.Not(filterByLabelsPredicate))).
Complete(r)
}

// SyncInternalNodes makes sure that at controller startup there are no "orphans" InternalNode, so without corresponding Node.
func SyncInternalNodes(ctx context.Context, c client.Client) error {
// Check whether there is the corresponding Node for the given InternalNode
var internalNodeList networkingv1beta1.InternalNodeList
if err := c.List(ctx, &internalNodeList, &client.ListOptions{}); err != nil {
return fmt.Errorf("unable to list InternalNode resources: %w", err)
}

for i := range internalNodeList.Items {
internalNode := &internalNodeList.Items[i]
var ownerNode corev1.Node

internalNodeName := internalNode.GetName()
err := c.Get(ctx, types.NamespacedName{Name: internalNodeName}, &ownerNode)
switch {
case apierrors.IsNotFound(err):
// Delete the internal node as there is no corresponding node
klog.Infof("Deleting InternalNode %v as there is no corresponding Node resource", internalNodeName)
if err := c.Delete(ctx, internalNode); err != nil {
return fmt.Errorf("unable to delete InternalNode %v: %w", internalNodeName, err)
}
case err != nil:
return fmt.Errorf("unable to get corresponding Node for InternalNode %v: %w", internalNodeName, err)
}
}

return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,13 @@ func (r *InternalNodeReconciler) genericEnqueuerfunc(ctx context.Context, _ clie
klog.Error(err)
return nil
}

var requests []reconcile.Request
for i := range internalNodes.Items {
iNode := &internalNodes.Items[i]

requests = append(requests, reconcile.Request{
NamespacedName: client.ObjectKeyFromObject(&internalNodes.Items[i]),
NamespacedName: client.ObjectKeyFromObject(iNode),
})
}
return requests
Expand Down

0 comments on commit 971bdf5

Please sign in to comment.