Modify hook-injector plugin to monitor directories to match cri-o#84
Conversation
bfb6068 to
b122b2d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #84 +/- ##
==========================================
- Coverage 64.55% 58.54% -6.01%
==========================================
Files 10 4 -6
Lines 1845 1305 -540
==========================================
- Hits 1191 764 -427
+ Misses 503 420 -83
+ Partials 151 121 -30 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
nice..
Suggest adding --disableWatch as an option ... might want to consider outputing the paths being monitored in help for the client or as debug out (unless that is already being done in the hooks package) ..
Could you dump your console output on the PR? Would be nice if we had hook tests running in critest or some other integration testing.
Maybe open an issue for a test bucket?
Signed-off-by: AbdelrahmanElawady <abdoelawady125@gmail.com>
b122b2d to
06841c2
Compare
|
new flag added and the output looks like this: $ sudo ./hook-injector -idx 10
INFO [0000] Created plugin 10-hook-injector (hook-injector, handles CreateContainer)
INFO [0000] watching directories "/usr/share/containers/oci/hooks.d /etc/containers/oci/hooks.d" for new changes
INFO [0000] Registering plugin 10-hook-injector...
INFO [0000] Configuring plugin 10-hook-injector for runtime containerd/1.7.2...
INFO [0000] Started plugin 10-hook-injector...
INFO [0069] redis/redis: OCI hooks injected |
Also, created an issue to track hook tests: #86 |
|
@haircommander FYI in case you want to review this one or tag someone |
|
LGTM |
As mentioned in hook-injector plugin README, this plugin achieves CRI-O compatible hooks injection. However, there is a difference between this implementation and CRI-O's implementation as CRI-O watches hook directories for any changes which allows for new hooks to be added to new containers. This is not the case here where this plugin only uses hooks created while the plugin is first running. So, they are not really compatible. Reference CRI-O code.
Also, there are some use-cases where services set up their hooks after the cluster and the plugin are up. CRI-O would register these new hooks but again the plugin currently won't. This PR aims to fix that.
We can also make this optional if you don't want to change the current behavior with a flag like
--watchor something like that.