bpfman-agent: don't try to unload bpf program that isn't loaded #75
Conversation
Fixes: bpfman#74 Signed-off-by: Andre Fredette <afredette@redhat.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #75 +/- ##
==========================================
- Coverage 27.36% 27.36% -0.01%
==========================================
Files 81 81
Lines 6975 6976 +1
==========================================
Hits 1909 1909
- Misses 4880 4881 +1
Partials 186 186
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| r.Logger.Error(err, "Failed to unload Program") | ||
| return ctrl.Result{RequeueAfter: retryDurationAgent}, nil | ||
|
|
||
| if id != nil { |
There was a problem hiding this comment.
what about returning error here https://github.com/bpfman/bpfman-operator/blob/main/controllers/bpfman-agent/internal/bpfman-core.go#L213 instead ?
There was a problem hiding this comment.
I think that could be made to work, but I'm not sure why it was implemented the way it was. The way GetID is currently implemented, it tells you if it wasn't set at all and if it was set, whether there was an error. We don't handle all these cases in the code, but we should think about it. I'd like to be able to call ListBpfmanPrograms() to see if it's loaded, but you have to know the program type. That can be fixed, but it will take a little time. This is a quick fix that works. We can make it better in the future.
There was a problem hiding this comment.
sure I just found it confusing to get invalid Id will error is nil that is why suggesting the above I am fine with what you have for now maybe worth a tracking issue so we won't forget about it
/LGTM
interface names hook doesn't allow more than one function per intfs per dir
Fixes: #74