panic in bpfman-agent due to err not being returned

Originally reported in
[bpfman:#1311](bpfman/bpfman#1311) and tracked
in #328, `GetID()` and `reconcileBpfProgram()` both have instances where
they detect errors but return nil for err, causing the calling functions
to continue and eventually panic.

Signed-off-by: Billy McFall <22157057+Billy99@users.noreply.github.com>

controllers/bpfman-agent/common.go

@@ -217,7 +217,7 @@ func (r *ReconcilerCommon) reconcileBpfProgram(ctx context.Context,
 		id, err := bpfmanagentinternal.GetID(bpfProgram)
 		if err != nil {
 			r.Logger.Error(err, "Failed to get bpf program ID")
-			return bpfmaniov1alpha1.BpfProgCondNotLoaded, nil
+			return bpfmaniov1alpha1.BpfProgCondNotLoaded, err
 		}
 		switch shouldBeLoaded {
 		case true:
@@ -232,7 +232,7 @@ func (r *ReconcilerCommon) reconcileBpfProgram(ctx context.Context,
 				r.Logger.V(1).Info("bpf program is in wrong state, unloading and reloading", "reason", reasons, "bpfProgram Name", bpfProgram.Name, "bpf program ID", id)
 				if err := bpfmanagentinternal.UnloadBpfmanProgram(ctx, r.BpfmanClient, *id); err != nil {
 					r.Logger.Error(err, "Failed to unload BPF Program")
-					return bpfmaniov1alpha1.BpfProgCondNotUnloaded, nil
+					return bpfmaniov1alpha1.BpfProgCondNotUnloaded, err
 				}
 
 				r.Logger.Info("Calling bpfman to load bpf program on Node", "bpfProgram Name", bpfProgram.Name)
@@ -251,7 +251,7 @@ func (r *ReconcilerCommon) reconcileBpfProgram(ctx context.Context,
 			r.Logger.Info("Calling bpfman to unload program on node", "bpfProgram Name", bpfProgram.Name, "Program ID", id)
 			if err := bpfmanagentinternal.UnloadBpfmanProgram(ctx, r.BpfmanClient, *id); err != nil {
 				r.Logger.Error(err, "Failed to unload Program")
-				return bpfmaniov1alpha1.BpfProgCondNotUnloaded, nil
+				return bpfmaniov1alpha1.BpfProgCondNotUnloaded, err
 			}
 		}
 	case false:
@@ -267,7 +267,7 @@ func (r *ReconcilerCommon) reconcileBpfProgram(ctx context.Context,
 			r.progId, err = bpfmanagentinternal.LoadBpfmanProgram(ctx, r.BpfmanClient, loadRequest)
 			if err != nil {
 				r.Logger.Error(err, "Failed to load Program")
-				return bpfmaniov1alpha1.BpfProgCondNotLoaded, nil
+				return bpfmaniov1alpha1.BpfProgCondNotLoaded, err
 			}
 		case false:
 			// The program isn't loaded and it shouldn't be loaded.
@@ -785,10 +785,10 @@ func (r *ReconcilerCommon) handleProgCreateOrUpdate(
 				}
 			}
 
-			existingId, err := bpfmanagentinternal.GetID(&existingBpfProgram)
-			if err != nil {
-				return internal.Requeue, fmt.Errorf("failed to get kernel id from bpfProgram: %v", err)
-			}
+			// GetID() will fail if ProgramId is not in the annotations, which is expected on a
+			// create. In this case existingId will be nil and DeepEqual() will fail and cause
+			// annotation to be set.
+			existingId, _ := bpfmanagentinternal.GetID(&existingBpfProgram)
 
 			// If bpfProgram Maps OR the program ID annotation isn't up to date just update it and return
 			if !reflect.DeepEqual(existingId, r.progId) {

controllers/bpfman-agent/internal/bpfman-core.go

@@ -210,12 +210,12 @@ func Build_kernel_info_annotations(p *gobpfman.ListResponse_ListResult) map[stri
 func GetID(p *bpfmaniov1alpha1.BpfProgram) (*uint32, error) {
 	idString, ok := p.Annotations[internal.IdAnnotation]
 	if !ok {
-		return nil, nil
+		return nil, fmt.Errorf("failed to get program ID because no annotations")
 	}
 
 	id, err := strconv.ParseUint(idString, 10, 32)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get program ID: %v", err)
+		return nil, fmt.Errorf("failed to parse program ID: %v", err)
 	}
 	uid := uint32(id)
 	return &uid, nil