Merge pull request #64 from srl-labs/fix/tunnel-setup-improvements

Fix/tunnel setup improvements

apis/topology/v1alpha1/common.go

@@ -112,7 +112,7 @@ type LinkEndpoint struct {
 // Tunnel represents a VXLAN tunnel between clabernetes nodes (as configured by containerlab).
 type Tunnel struct {
 	// ID is the VXLAN ID (vnid) for the tunnel.
-	ID int `json:"id"`
+	ID int `json:"id"             yaml:"id"`
 	// LocalNodeName is the name of the local node for this tunnel.
 	LocalNodeName string `json:"localNodeName"  yaml:"localNodeName"`
 	// RemoteName is the name of the service to contact the remote end of the tunnel.

controllers/topology/containerlab/config.go

@@ -6,15 +6,12 @@ import (
 
 	clabernetesconstants "github.com/srl-labs/clabernetes/constants"
 
-	clabernetescontrollerstopology "github.com/srl-labs/clabernetes/controllers/topology"
-
-	clabernetesutilcontainerlab "github.com/srl-labs/clabernetes/util/containerlab"
-
-	"gopkg.in/yaml.v3"
-
 	clabernetesapistopologyv1alpha1 "github.com/srl-labs/clabernetes/apis/topology/v1alpha1"
+	clabernetescontrollerstopologyreconciler "github.com/srl-labs/clabernetes/controllers/topology/reconciler"
 	claberneteserrors "github.com/srl-labs/clabernetes/errors"
 	clabernetesutil "github.com/srl-labs/clabernetes/util"
+	clabernetesutilcontainerlab "github.com/srl-labs/clabernetes/util/containerlab"
+	"gopkg.in/yaml.v3"
 )
 
 func getDefaultPorts() []*clabernetesutilcontainerlab.TypedPort {
@@ -276,8 +273,7 @@ func (c *Controller) processConfigForNode(
 	nodeName string,
 	nodeDefinition *clabernetesutilcontainerlab.NodeDefinition,
 	defaultsYAML []byte,
-	clabernetesConfigs map[string]*clabernetesutilcontainerlab.Config,
-	clabernetesTunnels map[string][]*clabernetesapistopologyv1alpha1.Tunnel,
+	reconcileData *clabernetescontrollerstopologyreconciler.ReconcileData,
 ) error {
 	deepCopiedDefaults := &clabernetesutilcontainerlab.NodeDefinition{}
 
@@ -296,7 +292,7 @@ func (c *Controller) processConfigForNode(
 		nodeDefinition.Ports = nodePorts
 	}
 
-	clabernetesConfigs[nodeName] = &clabernetesutilcontainerlab.Config{
+	reconcileData.PostReconcileConfigs[nodeName] = &clabernetesutilcontainerlab.Config{
 		Name: fmt.Sprintf("clabernetes-%s", nodeName),
 		Topology: &clabernetesutilcontainerlab.Topology{
 			Defaults: deepCopiedDefaults,
@@ -357,8 +353,8 @@ func (c *Controller) processConfigForNode(
 
 		if endpointA.NodeName == nodeName && endpointB.NodeName == nodeName {
 			// link loops back to ourselves, no need to do overlay things just append the link
-			clabernetesConfigs[nodeName].Topology.Links = append(
-				clabernetesConfigs[nodeName].Topology.Links,
+			reconcileData.PostReconcileConfigs[nodeName].Topology.Links = append(
+				reconcileData.PostReconcileConfigs[nodeName].Topology.Links,
 				link,
 			)
 
@@ -373,8 +369,8 @@ func (c *Controller) processConfigForNode(
 			uninterestingEndpoint = endpointA
 		}
 
-		clabernetesConfigs[nodeName].Topology.Links = append(
-			clabernetesConfigs[nodeName].Topology.Links,
+		reconcileData.PostReconcileConfigs[nodeName].Topology.Links = append(
+			reconcileData.PostReconcileConfigs[nodeName].Topology.Links,
 			&clabernetesutilcontainerlab.LinkDefinition{
 				LinkConfig: clabernetesutilcontainerlab.LinkConfig{
 					Endpoints: []string{
@@ -393,8 +389,8 @@ func (c *Controller) processConfigForNode(
 			},
 		)
 
-		clabernetesTunnels[nodeName] = append(
-			clabernetesTunnels[nodeName],
+		reconcileData.PostReconcileTunnels[nodeName] = append(
+			reconcileData.PostReconcileTunnels[nodeName],
 			&clabernetesapistopologyv1alpha1.Tunnel{
 				LocalNodeName:  nodeName,
 				RemoteNodeName: uninterestingEndpoint.NodeName,
@@ -417,21 +413,15 @@ func (c *Controller) processConfigForNode(
 func (c *Controller) processConfig(
 	clab *clabernetesapistopologyv1alpha1.Containerlab,
 	clabTopo *clabernetesutilcontainerlab.Topology,
+	reconcileData *clabernetescontrollerstopologyreconciler.ReconcileData,
 ) (
-	clabernetesConfigs map[string]*clabernetesutilcontainerlab.Config,
-	clabernetesTunnels map[string][]*clabernetesapistopologyv1alpha1.Tunnel,
-	shouldUpdate bool,
 	err error,
 ) {
-	clabernetesConfigs = make(map[string]*clabernetesutilcontainerlab.Config)
-
-	clabernetesTunnels = make(map[string][]*clabernetesapistopologyv1alpha1.Tunnel)
-
 	// we may have *different defaults per "sub-topology" so we do a cheater "deep copy" by just
 	// marshalling/unmarshalling :)
 	defaultsYAML, err := yaml.Marshal(clabTopo.Defaults)
 	if err != nil {
-		return clabernetesConfigs, clabernetesTunnels, false, err
+		return err
 	}
 
 	for nodeName, nodeDefinition := range clabTopo.Nodes {
@@ -441,47 +431,12 @@ func (c *Controller) processConfig(
 			nodeName,
 			nodeDefinition,
 			defaultsYAML,
-			clabernetesConfigs,
-			clabernetesTunnels,
+			reconcileData,
 		)
 		if err != nil {
-			return nil, nil, false, err
+			return err
 		}
 	}
 
-	clabernetesConfigsBytes, err := yaml.Marshal(clabernetesConfigs)
-	if err != nil {
-		return nil, nil, false, err
-	}
-
-	tunnelsBytes, err := yaml.Marshal(clabernetesTunnels)
-	if err != nil {
-		return nil, nil, false, err
-	}
-
-	newConfigsHash := clabernetesutil.HashBytes(clabernetesConfigsBytes)
-
-	newTunnelsHash := clabernetesutil.HashBytes(tunnelsBytes)
-
-	if clab.Status.ConfigsHash == newConfigsHash && clab.Status.TunnelsHash == newTunnelsHash {
-		// the configs hashes match, nothing to do, should reconcile is false, and no error
-		return clabernetesConfigs, clabernetesTunnels, false, nil
-	}
-
-	// if we got here we know we need to re-reconcile as the hash has changed, set the config and
-	// config hash, and then return "true" (yes we should reconcile/update the object). before we
-	// can do that though, we need to handle setting tunnel ids. so first we go over and re-use
-	// all the existing tunnel ids by assigning matching node/interface pairs from the previous
-	// status to the new tunnels... when doing so we record the allocated ids...
-	clabernetescontrollerstopology.AllocateTunnelIDs(
-		clab.Status.TopologyStatus.Tunnels,
-		clabernetesTunnels,
-	)
-
-	clab.Status.Configs = string(clabernetesConfigsBytes)
-	clab.Status.ConfigsHash = newConfigsHash
-	clab.Status.Tunnels = clabernetesTunnels
-	clab.Status.TunnelsHash = newTunnelsHash
-
-	return clabernetesConfigs, clabernetesTunnels, true, nil
+	return nil
 }

controllers/topology/containerlab/get.go

@@ -8,21 +8,21 @@ import (
 	ctrlruntime "sigs.k8s.io/controller-runtime"
 )
 
-// getClabFromReq fetches the reconcile target Containerlab topology from the Request.
-func (c *Controller) getClabFromReq(
+// getContainerlabFromReq fetches the reconcile target Containerlab topology from the Request.
+func (c *Controller) getContainerlabFromReq(
 	ctx context.Context,
 	req ctrlruntime.Request,
 ) (*clabernetesapistopologyv1alpha1.Containerlab, error) {
-	clab := &clabernetesapistopologyv1alpha1.Containerlab{}
+	containerlab := &clabernetesapistopologyv1alpha1.Containerlab{}
 
 	err := c.BaseController.Client.Get(
 		ctx,
 		apimachinerytypes.NamespacedName{
 			Namespace: req.Namespace,
 			Name:      req.Name,
 		},
-		clab,
+		containerlab,
 	)
 
-	return clab, err
+	return containerlab, err
 }

controllers/topology/containerlab/reconcile.go

@@ -3,12 +3,8 @@ package containerlab
 import (
 	"context"
 
-	clabernetesutil "github.com/srl-labs/clabernetes/util"
-
+	clabernetescontrollerstopologyreconciler "github.com/srl-labs/clabernetes/controllers/topology/reconciler"
 	clabernetesutilcontainerlab "github.com/srl-labs/clabernetes/util/containerlab"
-
-	"gopkg.in/yaml.v3"
-
 	apimachineryerrors "k8s.io/apimachinery/pkg/api/errors"
 	ctrlruntime "sigs.k8s.io/controller-runtime"
 )
@@ -20,7 +16,7 @@ func (c *Controller) Reconcile(
 ) (ctrlruntime.Result, error) {
 	c.BaseController.LogReconcileStart(req)
 
-	clab, err := c.getClabFromReq(ctx, req)
+	containerlab, err := c.getContainerlabFromReq(ctx, req)
 	if err != nil {
 		if apimachineryerrors.IsNotFound(err) {
 			// was deleted, nothing to do
@@ -34,33 +30,31 @@ func (c *Controller) Reconcile(
 		return ctrlruntime.Result{}, err
 	}
 
-	if clab.DeletionTimestamp != nil {
+	if containerlab.DeletionTimestamp != nil {
 		// deleting nothing to do, we have no finalizers or anything at this point
 		return ctrlruntime.Result{}, nil
 	}
 
-	preReconcileConfigs := make(map[string]*clabernetesutilcontainerlab.Config)
-
-	if clab.Status.Configs != "" {
-		err = yaml.Unmarshal([]byte(clab.Status.Configs), &preReconcileConfigs)
-		if err != nil {
-			c.BaseController.Log.Criticalf(
-				"failed parsing unmarshalling previously stored config, error: %s", err,
-			)
+	reconcileData, err := clabernetescontrollerstopologyreconciler.NewReconcileData(containerlab)
+	if err != nil {
+		c.BaseController.Log.Criticalf(
+			"failed processing previously stored containerlab resource, error: %s", err,
+		)
 
-			return ctrlruntime.Result{}, err
-		}
+		return ctrlruntime.Result{}, err
 	}
 
-	// load the containerlab topo to make sure its all good
-	containerlabTopo, err := clabernetesutilcontainerlab.LoadContainerlabTopology(clab.Spec.Config)
+	// load the containerlab topo from the CR to make sure its all good
+	containerlabTopo, err := clabernetesutilcontainerlab.LoadContainerlabTopology(
+		containerlab.Spec.Config,
+	)
 	if err != nil {
 		c.BaseController.Log.Criticalf("failed parsing containerlab config, error: %s", err)
 
 		return ctrlruntime.Result{}, err
 	}
 
-	clabernetesConfigs, tunnels, configShouldUpdate, err := c.processConfig(clab, containerlabTopo)
+	err = c.processConfig(containerlab, containerlabTopo, reconcileData)
 	if err != nil {
 		c.BaseController.Log.Criticalf("failed processing containerlab config, error: %s", err)
 
@@ -69,60 +63,67 @@ func (c *Controller) Reconcile(
 
 	err = c.TopologyReconciler.ReconcileConfigMap(
 		ctx,
-		clab,
-		clabernetesConfigs,
-		tunnels,
+		containerlab,
+		reconcileData,
 	)
 	if err != nil {
-		c.BaseController.Log.Criticalf("failed reconciling clabernetes config map, error: %s", err)
+		c.BaseController.Log.Criticalf(
+			"failed reconciling clabernetes config map, error: %s",
+			err,
+		)
 
 		return ctrlruntime.Result{}, err
 	}
 
-	err = c.TopologyReconciler.ReconcileServiceFabric(ctx, clab, clabernetesConfigs)
+	err = c.TopologyReconciler.ReconcileServiceFabric(
+		ctx,
+		containerlab,
+		reconcileData,
+	)
 	if err != nil {
 		c.BaseController.Log.Criticalf("failed reconciling clabernetes services, error: %s", err)
 
 		return ctrlruntime.Result{}, err
 	}
 
-	var exposeServicesShouldUpdate bool
-
-	if !clab.Spec.DisableExpose {
-		exposeServicesShouldUpdate, err = c.TopologyReconciler.ReconcileServicesExpose(
-			ctx,
-			clab,
-			clabernetesConfigs,
+	err = c.TopologyReconciler.ReconcileServicesExpose(
+		ctx,
+		containerlab,
+		reconcileData,
+	)
+	if err != nil {
+		c.BaseController.Log.Criticalf(
+			"failed reconciling clabernetes expose services, error: %s", err,
 		)
-		if err != nil {
-			c.BaseController.Log.Criticalf(
-				"failed reconciling clabernetes expose services, error: %s", err,
-			)
 
-			return ctrlruntime.Result{}, err
-		}
+		return ctrlruntime.Result{}, err
 	}
 
 	err = c.TopologyReconciler.ReconcileDeployments(
 		ctx,
-		clab,
-		preReconcileConfigs,
-		clabernetesConfigs,
+		containerlab,
+		reconcileData,
 	)
 	if err != nil {
 		c.BaseController.Log.Criticalf("failed reconciling clabernetes deployments, error: %s", err)
 
 		return ctrlruntime.Result{}, err
 	}
 
-	if clabernetesutil.AnyBoolTrue(configShouldUpdate, exposeServicesShouldUpdate) {
-		// we should update because config hash or something changed, so push update to the object
-		err = c.BaseController.Client.Update(ctx, clab)
+	if reconcileData.ShouldUpdateResource {
+		// we should update because config hash or something changed, so snag the updated status
+		// data out of the reconcile data, put it in the resource, and push the update
+		containerlab.Status.Configs = string(reconcileData.PostReconcileConfigsBytes)
+		containerlab.Status.ConfigsHash = reconcileData.PostReconcileConfigsHash
+		containerlab.Status.Tunnels = reconcileData.PostReconcileTunnels
+		containerlab.Status.TunnelsHash = reconcileData.PostReconcileTunnelsHash
+
+		err = c.BaseController.Client.Update(ctx, containerlab)
 		if err != nil {
 			c.BaseController.Log.Criticalf(
 				"failed updating object '%s/%s' error: %s",
-				clab.Namespace,
-				clab.Name,
+				containerlab.Namespace,
+				containerlab.Name,
 				err,
 			)