Fix handling for adding/deleting programs from an existing BpfApp CRD

Also cleaned up a couple of log messages.

Fixes: #347

Signed-off-by: Andre Fredette <[email protected]>

Author: anfredette

apis/v1alpha1/shared_types.go

@@ -352,6 +352,8 @@ const (
 	AppUnloadError AppLoadStatus = "UnloadError"
 	// The app is not selected to run on the node
 	NotSelected AppLoadStatus = "NotSelected"
+	// The app is not selected to run on the node
+	ProgListChangedError AppLoadStatus = "ProgramListChangedError"
 )
 
 type ProgramLinkStatus string

bundle/manifests/bpfman-operator.clusterserviceversion.yaml

@@ -861,7 +861,7 @@ metadata:
     capabilities: Basic Install
     categories: OpenShift Optional
     containerImage: quay.io/bpfman/bpfman-operator:latest
-    createdAt: "2025-03-03T23:26:55Z"
+    createdAt: "2025-03-04T15:37:43Z"
     description: The bpfman Operator is designed to manage eBPF programs for applications.
     features.operators.openshift.io/cnf: "false"
     features.operators.openshift.io/cni: "false"

config/samples/bpfman.io_v1alpha1_test1_clusterbpfapplication.yaml renamed to config/samples/bpfman.io_v1alpha1_clusterbpfapplication_test1.yaml

@@ -0,0 +1,37 @@
+apiVersion: bpfman.io/v1alpha1
+kind: ClusterBpfApplication
+metadata:
+  labels:
+    app.kubernetes.io/name: clusterbpfapplication
+  name: clusterbpfapplication-sample-test3
+spec:
+  nodeSelector: {}
+  byteCode:
+    image:
+      url: quay.io/bpfman-bytecode/app-test:latest
+  programs:
+    - name: kprobe_test
+      type: kprobe
+      kprobeInfo:
+        links:
+          - function: try_to_wake_up
+            offset: 0
+    - name: tracepoint_test
+      type: tracepoint
+      tracepointInfo:
+        links:
+          - name: syscalls/sys_enter_openat
+    - name: fentry_test
+      type: fentry
+      fentryInfo:
+        function: do_unlinkat
+        links:
+          - attach: true
+    # - name: tcx_next_test
+    #   type: tcx
+    #   tcxInfo:
+    #     links:
+    #       - interfaceSelector:
+    #           primaryNodeInterface: true
+    #         priority: 500
+    #         direction: ingress

config/samples/bpfman.io_v1alpha1_test2_clusterbpfapplication.yaml renamed to config/samples/bpfman.io_v1alpha1_clusterbpfapplication_test2.yaml

@@ -175,7 +175,7 @@ func (r *ClBpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Req
 			bpfAppStateOriginal = r.currentAppState.DeepCopy()
 		}
 
-		r.Logger.Info("From getBpfAppState", "new", bpfAppStateNew)
+		r.Logger.Info("BpfApplicationState status", "new", bpfAppStateNew)
 
 		if bpfAppStateNew {
 			// Create the object and return. We'll get the updated object in the
@@ -248,9 +248,9 @@ func (r *ClBpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 				err = rec.reconcileProgram(ctx, rec, r.isBeingDeleted())
 				if err != nil {
-					r.Logger.Info("Error reconciling program", "Name", rec.getProgName(), "Index", appProgramIndex)
+					r.Logger.Info("Error reconciling program", "Name", rec.getProgName())
 				} else {
-					r.Logger.Info("Successfully reconciled program", "Name", rec.getProgName(), "Index", appProgramIndex)
+					r.Logger.Info("Successfully reconciled program", "Name", rec.getProgName())
 				}
 			}
 
@@ -789,3 +789,47 @@ func (r *ClBpfApplicationReconciler) deleteLinks(program *bpfmaniov1alpha1.ClBpf
 		r.Logger.Error(fmt.Errorf("unexpected EBPFProgType"), "unexpected EBPFProgType", "Type", program.Type)
 	}
 }
+
+// validateProgramList checks the BpfApplicationPrograms to ensure that none
+// have been added or deleted.
+func (r *ClBpfApplicationReconciler) validateProgramList() error {
+	// Create a map of the programs in r.currentAppState.Spec.Programs to make
+	// the checks more efficient.
+	appStateProgMap := make(map[string]bool)
+	for _, program := range r.currentAppState.Spec.Programs {
+		appStateProgMap[program.Name] = true
+	}
+
+	// Check that all the programs in r.currentApp.Spec.Programs are on the
+	// list.  If not, that indicates that the program has been added, which is
+	// not allowed.  Remove them if they are on the list so we can check if any
+	// are left over which would indicate that they have been removed from the
+	// list.
+	addedPrograms := ""
+	for _, program := range r.currentApp.Spec.Programs {
+		if _, ok := appStateProgMap[program.Name]; !ok {
+			addedPrograms = addedPrograms + program.Name + " "
+		} else {
+			delete(appStateProgMap, program.Name)
+		}
+	}
+
+	if addedPrograms != "" {
+		r.Logger.Info("Programs added error", "Programs", addedPrograms)
+		return fmt.Errorf("programs have been added: %s", addedPrograms)
+	}
+
+	// Now, see if there are any programs left on the list, which would indicate that
+	// they have been removed from the list.
+	if len(appStateProgMap) > 0 {
+		// create a string containing the names of the programs that have been removed
+		removedPrograms := ""
+		for program := range appStateProgMap {
+			removedPrograms = removedPrograms + program + " "
+		}
+		r.Logger.Info("Programs removed error", "Programs", removedPrograms)
+		return fmt.Errorf("programs have been removed: %s", removedPrograms)
+	}
+
+	return nil
+}

config/samples/bpfman.io_v1alpha1_clusterbpfapplication_test3.yaml

@@ -87,6 +87,7 @@ type ApplicationReconciler interface {
 	isBeingDeleted() bool
 	updateBpfAppStatus(ctx context.Context, condition metav1.Condition) error
 	updateLoadStatus(updateStatus bpfmaniov1alpha1.AppLoadStatus)
+	validateProgramList() error
 	load(ctx context.Context) error
 	isLoaded(ctx context.Context) bool
 	getLoadRequest() (*gobpfman.LoadRequest, error)
@@ -120,7 +121,7 @@ type ProgramReconciler interface {
 func (r *ReconcilerCommon) reconcileLoad(ctx context.Context, rec ApplicationReconciler) error {
 	isNodeSelected, err := isNodeSelected(rec.getNodeSelector(), rec.getNode().Labels)
 	if err != nil {
-		return fmt.Errorf("failed to check if node is selected: %v", err)
+		return fmt.Errorf("check if node is selected failed: %v", err)
 	}
 
 	if !isNodeSelected {
@@ -132,7 +133,11 @@ func (r *ReconcilerCommon) reconcileLoad(ctx context.Context, rec ApplicationRec
 		rec.unload(ctx)
 		rec.updateLoadStatus(bpfmaniov1alpha1.AppUnLoadSuccess)
 	} else {
-		// The program should be loaded.  Load it if necessary.
+		err := rec.validateProgramList()
+		if err != nil {
+			rec.updateLoadStatus(bpfmaniov1alpha1.ProgListChangedError)
+			return err
+		}
 		if rec.isLoaded(ctx) {
 			rec.updateLoadStatus(bpfmaniov1alpha1.AppLoadSuccess)
 		} else {

controllers/bpfman-agent/cl_application_program.go

@@ -176,7 +176,7 @@ func (r *NsBpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Req
 			bpfAppStateOriginal = r.currentAppState.DeepCopy()
 		}
 
-		r.Logger.Info("From getBpfAppState", "new", bpfAppStateNew)
+		r.Logger.Info("BpfApplicationState status", "new", bpfAppStateNew)
 
 		if bpfAppStateNew {
 			// Create the object and return. We'll get the updated object in the
@@ -249,9 +249,9 @@ func (r *NsBpfApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 				err = rec.reconcileProgram(ctx, rec, r.isBeingDeleted())
 				if err != nil {
-					r.Logger.Info("Error reconciling program", "Name", rec.getProgName(), "Index", appProgramIndex)
+					r.Logger.Info("Error reconciling program", "Name", rec.getProgName())
 				} else {
-					r.Logger.Info("Successfully reconciled program", "Name", rec.getProgName(), "Index", appProgramIndex)
+					r.Logger.Info("Successfully reconciled program", "Name", rec.getProgName())
 				}
 			}
 
@@ -717,3 +717,49 @@ func (r *NsBpfApplicationReconciler) deleteLinks(program *bpfmaniov1alpha1.BpfAp
 		r.Logger.Error(fmt.Errorf("unexpected EBPFProgType"), "unexpected EBPFProgType", "Type", program.Type)
 	}
 }
+
+// validateProgramList checks the BpfApplicationPrograms to ensure that none
+// have been added or deleted.
+func (r *NsBpfApplicationReconciler) validateProgramList() error {
+	if len(r.currentAppState.Spec.Programs) != len(r.currentApp.Spec.Programs) {
+		return fmt.Errorf("program list has changed")
+	}
+
+	// Create a map of the programs in r.currentAppState.Spec.Programs so we can
+	// quickly check if a program is in the list.
+	appStateProgMap := make(map[string]bool)
+	for _, program := range r.currentAppState.Spec.Programs {
+		appStateProgMap[program.Name] = true
+	}
+
+	// Check that all the programs in r.currentApp.Spec.Programs are on the
+	// list.  If not, that indicates that the program has been added, which is
+	// not allowed.  Remove them if they are on the list so we can check if any
+	// are left over which would indicate that they have been removed from the
+	// list.
+	addedPrograms := ""
+	for _, program := range r.currentApp.Spec.Programs {
+		if _, ok := appStateProgMap[program.Name]; !ok {
+			addedPrograms = addedPrograms + program.Name + " "
+		} else {
+			delete(appStateProgMap, program.Name)
+		}
+	}
+
+	if addedPrograms != "" {
+		return fmt.Errorf("programs have been added: %s", addedPrograms)
+	}
+
+	// Now, see if there are any programs left on the list, which would indicate that
+	// they have been removed from the list.
+	if len(appStateProgMap) > 0 {
+		// create a string containing the names of the programs that have been removed
+		removedPrograms := ""
+		for program := range appStateProgMap {
+			removedPrograms = removedPrograms + program + " "
+		}
+		return fmt.Errorf("programs have been removed: %s", removedPrograms)
+	}
+
+	return nil
+}