Merge pull request #102 from sunya-ch/v1.0.4

set stable channel and replaces for bundle V1.0.4 and minor bug fixes

Author: sunya-ch

.github/workflows/build_push_controller.yaml

@@ -24,6 +24,7 @@ env:
 jobs:
   build-push-bundle:
     runs-on: ubuntu-latest
+    needs: build-push-controller
     env:
       IMAGE_NAME: ghcr.io/${{ github.repository }}-bundle
       CHANNELS: stable

.github/workflows/golangci-lint.yaml

@@ -11,7 +11,10 @@ jobs:
         with:
           go-version: 1.18
       - uses: actions/checkout@v3
+      - name: Tidy
+        run: |
+          go mod tidy
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.52.2
+          args: --timeout=10m

Makefile

@@ -141,8 +141,6 @@ uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified
 	$(KUSTOMIZE) build config/crd | kubectl delete -f -
 
 predeploy: manifests kustomize
-	rm -f config/samples/multinic.fms.io_config.yaml
-	envsubst < config/samples/multinic.fms.io_config.template > config/samples/multinic.fms.io_config.yaml
 	cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG}
 	cd config/samples && $(KUSTOMIZE) edit set image multi-nic-cni-daemon=${DAEMON_IMG}
 

config/manifests/kustomization.yaml

@@ -39,4 +39,7 @@ patchesJson6902:
         value: "--zap-stacktrace-level=error"
       - op: add
         path: /spec/template/spec/containers/1/args/1
-        value: "--zap-log-level=4"
+        value: "--zap-log-level=4"
+      - op: add
+        path: /spec/template/spec/containers/1/args/2
+        value: "--zap-time-encoding=iso8601"

config/samples/kustomization.yaml

@@ -15,4 +15,4 @@ kind: Kustomization
 images:
 - name: multi-nic-cni-daemon
   newName: ghcr.io/foundation-model-stack/multi-nic-cni-daemon
-  newTag: v1.0.0
+  newTag: v1.0.4

config/samples/multinic.fms.io_config.template

@@ -36,4 +36,9 @@ spec:
     resources:
       requests:
         cpu: "100m"
-        memory: "50Mi"
+        memory: "50Mi"
+  urgentReconcileSeconds: 5
+  normalReconcileMinutes: 1
+  longReconcileMinutes: 10
+  contextTimeoutMinutes: 2
+  logLevel: 4

config/samples/multinic.fms.io_config.yaml

@@ -55,7 +55,7 @@ type CIDRHandler struct {
 func NewCIDRHandler(client client.Client, config *rest.Config, hostInterfaceHandler *HostInterfaceHandler, daemonCache *DaemonCacheHandler, quit chan struct{}) *CIDRHandler {
 	clientset, _ := kubernetes.NewForConfig(config)
 	cidrCompute := compute.CIDRCompute{}
-	updateReq := make(chan struct{}, MAX_QSIZE)
+	updateReq := make(chan struct{}, vars.MaxQueueSize)
 	handler := &CIDRHandler{
 		Client:               client,
 		Clientset:            clientset,

controllers/cidr_handler.go

@@ -16,6 +16,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	multinicv1 "github.com/foundation-model-stack/multi-nic-cni/api/v1"
 	"github.com/foundation-model-stack/multi-nic-cni/controllers/vars"
@@ -31,28 +32,20 @@ import (
 	"os"
 )
 
-const (
-	SERVICE_ACCOUNT_NAME       = "multi-nic-cni-operator-controller-manager"
-	DEFAULT_OPERATOR_NAMESPACE = "multi-nic-cni-operator-system"
-
-	// NetworkAttachmentDefinition watching queue size
-	MAX_QSIZE = 100
-)
+const configFinalizer = "finalizers.config.multinic.fms.io"
 
 var (
 	OPERATOR_NAMESPACE string = getOperatorNamespace()
 	ConfigReady        bool   = false
 	// referred by daemon watcher
-	DAEMON_LABEL_NAME         = "app"
-	DAEMON_LABEL_VALUE        = "multi-nicd"
-	DaemonName         string = DAEMON_LABEL_VALUE
+	DaemonName string = vars.DaemonLabelValue
 )
 
 func getOperatorNamespace() string {
 	key := "OPERATOR_NAMESPACE"
 	val, found := os.LookupEnv(key)
 	if !found {
-		return DEFAULT_OPERATOR_NAMESPACE
+		return vars.DefaultOperatorNamespace
 	}
 	return val
 }
@@ -74,13 +67,10 @@ type ConfigReconciler struct {
 //+kubebuilder:rbac:groups=multinic.fms.io,resources=configs,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=multinic.fms.io,resources=configs/status,verbs=get;update;patch
 
-func (r *ConfigReconciler) CreateDefaultDaemonConfig() error {
-	objMeta := metav1.ObjectMeta{
-		Name: DaemonName,
-	}
+func (r *ConfigReconciler) getDefaultConfigSpec() multinicv1.ConfigSpec {
 	daemonEnv := corev1.EnvVar{
 		Name:  "DAEMON_PORT",
-		Value: "11000",
+		Value: fmt.Sprintf("%d", vars.DefaultDaemonPort),
 	}
 	routeEnv := corev1.EnvVar{
 		Name:  "RT_TABLE_PATH",
@@ -90,7 +80,7 @@ func (r *ConfigReconciler) CreateDefaultDaemonConfig() error {
 	binMnt := multinicv1.HostPathMount{
 		Name:        "cnibin",
 		PodCNIPath:  "/host/opt/cni/bin",
-		HostCNIPath: "/var/lib/cni/bin",
+		HostCNIPath: r.getCNIHostPath(),
 	}
 	devPluginMnt := multinicv1.HostPathMount{
 		Name:        "device-plugin",
@@ -114,25 +104,33 @@ func (r *ConfigReconciler) CreateDefaultDaemonConfig() error {
 		Privileged: &privileged,
 	}
 	daemonSpec := multinicv1.DaemonSpec{
-		Image:           "ghcr.io/foundation-model-stack/multi-nic-cni-daemon:v1.0.4",
+		Image:           vars.DefaultDaemonImage,
 		Env:             env,
 		HostPathMounts:  hostPathMounts,
 		Resources:       resources,
 		SecurityContext: securityContext,
-		DaemonPort:      11000,
+		DaemonPort:      vars.DefaultDaemonPort,
 	}
 	spec := multinicv1.ConfigSpec{
-		CNIType:         "multi-nic",
-		IPAMType:        "multi-nic-ipam",
+		CNIType:         vars.DefaultCNIType,
+		IPAMType:        vars.DefaultIPAMType,
 		Daemon:          daemonSpec,
-		JoinPath:        "/join",
-		InterfacePath:   "/interface",
-		AddRoutePath:    "/addl3",
-		DeleteRoutePath: "/deletel3",
+		JoinPath:        vars.DefaultJoinPath,
+		InterfacePath:   vars.DefaultInterfacePath,
+		AddRoutePath:    vars.DefaultAddRoutePath,
+		DeleteRoutePath: vars.DefaultDeleteRoutePath,
 	}
+	return spec
+}
+
+func (r *ConfigReconciler) CreateDefaultDaemonConfig() error {
+	objMeta := metav1.ObjectMeta{
+		Name: DaemonName,
+	}
+
 	cfg := &multinicv1.Config{
 		ObjectMeta: objMeta,
-		Spec:       spec,
+		Spec:       r.getDefaultConfigSpec(),
 	}
 	err := r.Client.Create(context.TODO(), cfg)
 	return err
@@ -148,16 +146,39 @@ func (r *ConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		if errors.IsNotFound(err) {
 			// Request object not found, could have been deleted after reconcile request.
 			// Return and don't requeue
-			err = r.callFinalizer(vars.ConfigLog, req.NamespacedName.Name)
-			if err != nil {
-				vars.ConfigLog.V(1).Info(fmt.Sprintf("Failed to finalize %s: %v ", req.NamespacedName.Name, err))
-			}
 			return ctrl.Result{}, nil
 		}
 		vars.ConfigLog.V(7).Info(fmt.Sprintf("Cannot get #%v ", err))
 		// Error reading the object - requeue the request.
 		return ctrl.Result{RequeueAfter: vars.LongReconcileTime}, nil
 	}
+
+	// Add finalizer to instance
+	if !controllerutil.ContainsFinalizer(instance, configFinalizer) {
+		controllerutil.AddFinalizer(instance, configFinalizer)
+		err = r.Update(ctx, instance)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
+	// If config is deleted, delete corresponding daemonset
+	is_deleted := instance.GetDeletionTimestamp() != nil
+	if is_deleted {
+		if controllerutil.ContainsFinalizer(instance, configFinalizer) {
+			if err = r.callFinalizer(vars.ConfigLog, req.NamespacedName.Name); err != nil {
+				return ctrl.Result{}, err
+			}
+
+			controllerutil.RemoveFinalizer(instance, configFinalizer)
+			err := r.Client.Update(ctx, instance)
+			if err != nil {
+				return ctrl.Result{}, err
+			}
+		}
+		return ctrl.Result{}, nil
+	}
+
 	r.UpdateConfigBySpec(&instance.Spec)
 
 	if !ConfigReady {
@@ -172,17 +193,17 @@ func (r *ConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	_, err = r.Clientset.AppsV1().DaemonSets(OPERATOR_NAMESPACE).Get(context.TODO(), dsName, metav1.GetOptions{})
 	daemonset := r.newCNIDaemonSet(r.Clientset, dsName, instance.Spec.Daemon)
 	if err != nil {
-		// - create CNI daemonset, and NetworkAttachmentDefinition watcher if not exist
+		// - create CNI daemonset
 		if errors.IsNotFound(err) {
-			r.newNetAttachDefWatcher(instance)
+			SetDaemonConnector(instance.Spec)
 			_, err = r.Clientset.AppsV1().DaemonSets(OPERATOR_NAMESPACE).Create(context.TODO(), daemonset, metav1.CreateOptions{})
 			vars.ConfigLog.V(7).Info(fmt.Sprintf("Create new multi-nic daemonset #%s (cni=%s,ipam=%s), err=%v ", dsName, instance.Spec.CNIType, instance.Spec.IPAMType, err))
 		} else {
 			vars.ConfigLog.Info(fmt.Sprintf("Cannot get daemonset #%v ", err))
 		}
 	} else {
-		// - otherwise, update the existing daemonset and restart NetworkAttachmentDefinition watcher
-		r.newNetAttachDefWatcher(instance)
+		// - otherwise, update the existing daemonset
+		SetDaemonConnector(instance.Spec)
 		_, err = r.Clientset.AppsV1().DaemonSets(OPERATOR_NAMESPACE).Update(context.TODO(), daemonset, metav1.UpdateOptions{})
 		vars.ConfigLog.V(7).Info(fmt.Sprintf("Update multi-nic daemonset #%s, err=%v ", dsName, err))
 	}
@@ -224,19 +245,13 @@ func (r *ConfigReconciler) UpdateConfigBySpec(spec *multinicv1.ConfigSpec) {
 			vars.SetLog()
 		}
 	}
-}
-
-// newNetAttachDefWatcher restarts NetworkAttachmentDefinition watcher
-func (r *ConfigReconciler) newNetAttachDefWatcher(instance *multinicv1.Config) {
-	vars.DaemonPort = instance.Spec.Daemon.DaemonPort
-	vars.TargetCNI = instance.Spec.CNIType
-	DaemonName = instance.GetName()
-	SetDaemon(instance.Spec)
+	vars.DaemonPort = spec.Daemon.DaemonPort
+	vars.TargetCNI = spec.CNIType
 }
 
 // newCNIDaemonSet creates new CNI daemonset
 func (r *ConfigReconciler) newCNIDaemonSet(client *kubernetes.Clientset, name string, daemonSpec multinicv1.DaemonSpec) *appsv1.DaemonSet {
-	labels := map[string]string{DAEMON_LABEL_NAME: DAEMON_LABEL_VALUE}
+	labels := map[string]string{vars.DeamonLabelKey: vars.DaemonLabelValue}
 
 	// prepare container port
 	containerPort := corev1.ContainerPort{ContainerPort: int32(daemonSpec.DaemonPort)}
@@ -306,7 +321,7 @@ func (r *ConfigReconciler) newCNIDaemonSet(client *kubernetes.Clientset, name st
 				ObjectMeta: metav1.ObjectMeta{Labels: labels},
 				Spec: corev1.PodSpec{
 					HostNetwork:        true,
-					ServiceAccountName: SERVICE_ACCOUNT_NAME,
+					ServiceAccountName: vars.ServiceAccountName,
 					NodeSelector:       daemonSpec.NodeSelector,
 					Tolerations:        daemonSpec.Tolerations,
 					Containers: []corev1.Container{
@@ -320,12 +335,40 @@ func (r *ConfigReconciler) newCNIDaemonSet(client *kubernetes.Clientset, name st
 	}
 }
 
+func (r *ConfigReconciler) getCNIHostPath() string {
+	ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+	defer cancel()
+	// find multus pod
+	labels := fmt.Sprintf("%s=%s", vars.MultusLabelKey, vars.MultusLabelValue)
+	listOptions := metav1.ListOptions{
+		LabelSelector: labels,
+		Limit:         1,
+	}
+	multusPods, err := r.Clientset.CoreV1().Pods(metav1.NamespaceAll).List(ctx, listOptions)
+	if err != nil {
+		return vars.DefaultCNIHostPath
+	}
+	for _, multusPod := range multusPods.Items {
+		volumes := multusPod.Spec.Volumes
+		for _, volume := range volumes {
+			if volume.Name == vars.CNIBinVolumeName {
+				if volume.HostPath != nil {
+					cniPath := volume.HostPath.Path
+					vars.ConfigLog.Info(fmt.Sprintf("Find Multus CNI path: %s", cniPath))
+					return cniPath
+				} else {
+					// hostpath is not defined
+					return vars.DefaultCNIHostPath
+				}
+			}
+		}
+	}
+	return vars.DefaultCNIHostPath
+}
+
 // callFinalizer deletes all CIDRs, waits for all ippools deleted, deletes CNI deamonset, and stops NetworkAttachmentDefinition watcher
 func (r *ConfigReconciler) callFinalizer(reqLogger logr.Logger, dsName string) error {
-	// reset default name
-	DaemonName = DAEMON_LABEL_VALUE
 	reqLogger.Info(fmt.Sprintf("Finalize %s", dsName))
-
 	// delete all CIDRs
 	cidrMap, err := r.CIDRHandler.ListCIDR()
 	if err == nil {

controllers/config_controller.go

@@ -6,7 +6,9 @@
 package controllers
 
 import (
-	"fmt"
+	"errors"
+
+	"github.com/foundation-model-stack/multi-nic-cni/controllers/vars"
 )
 
 type DaemonPod struct {
@@ -28,7 +30,7 @@ func (h *DaemonCacheHandler) SetCache(key string, value DaemonPod) {
 func (h *DaemonCacheHandler) GetCache(key string) (DaemonPod, error) {
 	value := h.SafeCache.GetCache(key)
 	if value == nil {
-		return DaemonPod{}, fmt.Errorf("Not Found")
+		return DaemonPod{}, errors.New(vars.NotFoundError)
 	}
 	return value.(DaemonPod), nil
 }

controllers/daemon_cache_handler.go

@@ -23,8 +23,9 @@ import (
 var DAEMON_NAMESPACE, DAEMON_PORT, INTERFACE_PATH, ADD_ROUTE_PATH, DELETE_ROUTE_PATH, REGISTER_IPAM_PATH string
 
 // SetDaemon sets daemon environments
-func SetDaemon(daemonSpec multinicv1.ConfigSpec) {
-	DAEMON_PORT = fmt.Sprintf("%d", daemonSpec.Daemon.DaemonPort)
+func SetDaemonConnector(daemonSpec multinicv1.ConfigSpec) {
+	daemonPort := fmt.Sprintf("%d", daemonSpec.Daemon.DaemonPort)
+	DAEMON_PORT = daemonPort
 	DAEMON_NAMESPACE = OPERATOR_NAMESPACE
 	INTERFACE_PATH = daemonSpec.InterfacePath
 	ADD_ROUTE_PATH = daemonSpec.AddRoutePath