tetragon: Process Manager args cleanup #236
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We are not going to enforce all our vendor code follows the format rules otherwise we get large patch diffs and break go vendor. Signed-off-by: John Fastabend <[email protected]>
Passing options through series of calls gets old fast. Lets clean these up by creating an options pkgs where options can be set and retrieved without having to wonder if we passed them through the stack correctly. Then any of our sensors can just read them as needed. End goal is to remove these New() creators after which we can lose per sensor shell structures used to hold only the args. We leave aggregator options alone for now because we will build a aaggregator config struct for them later. Signed-off-by: John Fastabend <[email protected]>
ProcessManager has a bool we call enableEventCache as an argument. In theory this could enable/disable using the event cache, but its not actually hooked up to anything. Lets remove it because its not used and then use options pkg later if we want to control if cache runs or not. Signed-off-by: John Fastabend <[email protected]>
Remove passing options through New() creators in the grpc tracing pkg. Instead access directly through the option pkg. Signed-off-by: John Fastabend <[email protected]>
Remove passing options through New() creators in the grpc exec pkg. Instead access directly through the option pkg. Signed-off-by: John Fastabend <[email protected]>
ProcessManager no longer needs to pass options around because they get pushed through pkg/option lets drop them. todo: testers need to set config.Enable* options directly Signed-off-by: John Fastabend <[email protected]>
Fix A bug where we enabled eventCache even when K8s api was disabled. In this case its useless because K8s watcher will never give us metadata so trying to map events to their metadata is always nil, there is no metadata. Signed-off-by: John Fastabend <[email protected]>
The users of execCache use a getter to pull it directly because there can only ever be one. Remove the extra var holding it in the struct. Signed-off-by: John Fastabend <[email protected]>
jrfastab
force-pushed
the
pr/jrfastab/pmArgsCleanup
branch
from
5f2436b to
e3bcfbd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This simplifies Process manager and arg handling creating common pkg to handle getting/setting args. After this series ./pkg/option is also used for standard configuration options such as ProcessNS and ProcessCreds.
With this we can simplify Process Manager arg passing and avoid pushing args down through multiple layers of calls. And additionally logs can pretty print the args at any debug or info point we need them.
Finally fixes up cache init flow to only enable the eventCache when the K8s watcher is also enabled. This further simplifies bringing the ProcessManager up because we no longer need to pass the arg through. And finally, we can drop storing the cache in the process manager object. Caches are singular objects you can't have multiple caches of the same type. Historically, we allowed this but it was thoroughly confusing and when some poor person had to debug it you end up asking questions like -- which cache is not working, is the event in the other cache , and so on-- so we removed this logic long ago, but process manager never came along with it until now.