wip: ContainerReachedMaxTime

dwertent commented 7 months ago

User description

Overview

Create an event when we reach max sniffing time so the managers will stop caching the events. This will save a lot of memory and CPU.

Type

enhancement

Description

Added handling for containers reaching maximum allowed time across various managers (ApplicationProfileManager, NetworkManager, RelevancyManager).
Introduced a new error ContainerReachedMaxTime in utils to signal the max time event.
Implemented no-operation ContainerReachedMaxTime methods in mock classes for testing.
Enhanced container watcher to propagate the max time event to relevant managers.

Changes walkthrough

Relevant files

Enhancement

11 files

applicationprofile_manager_interface.go `Add Max Time Handler to ApplicationProfileManager Interface` pkg/applicationprofilemanager/applicationprofile_manager_interface.go Added `ContainerReachedMaxTime` method to the `ApplicationProfileManagerClient` interface.	+1/-0
applicationprofile_manager_mock.go `Implement Max Time Handler in ApplicationProfileManager Mock` pkg/applicationprofilemanager/applicationprofile_manager_mock.go Implemented `ContainerReachedMaxTime` method as a no-operation in the mock.	+3/-0
applicationprofile_manager.go `Implement Container Max Time Handling in ApplicationProfileManager` pkg/applicationprofilemanager/v1/applicationprofile_manager.go Added `ContainerReachedMaxTime` method to trigger a max time event for a container. Save profile upon container reaching max time.	+9/-0
container_watcher_private.go `Propagate Container Max Time Event in ContainerWatcher` pkg/containerwatcher/v1/container_watcher_private.go Trigger `ContainerReachedMaxTime` across multiple managers when a container reaches max time.	+4/-0
network_manager_interface.go `Add Max Time Handler to NetworkManager Interface` pkg/networkmanager/network_manager_interface.go Added `ContainerReachedMaxTime` method to the `NetworkManagerClient` interface.	+1/-0
network_manager_mock.go `Implement Max Time Handler in NetworkManager Mock` pkg/networkmanager/network_manager_mock.go Implemented `ContainerReachedMaxTime` method as a no-operation in the mock.	+3/-0
network_manager.go `Implement Container Max Time Handling in NetworkManager` pkg/networkmanager/v1/network_manager.go Added `ContainerReachedMaxTime` method to handle max time event for a container. Save network events upon container reaching max time.	+10/-0
relevancy_manager_interface.go `Add Max Time Handler to RelevancyManager Interface` pkg/relevancymanager/relevancy_manager_interface.go Added `ContainerReachedMaxTime` method to the `RelevancyManagerClient` interface.	+1/-0
relevancy_manager_mock.go `Implement Max Time Handler in RelevancyManager Mock` pkg/relevancymanager/relevancy_manager_mock.go Implemented `ContainerReachedMaxTime` method as a no-operation in the mock.	+3/-0
relevancy_manager.go `Implement Container Max Time Handling in RelevancyManager` pkg/relevancymanager/v1/relevancy_manager.go Added `ContainerReachedMaxTime` method to trigger relevancy handling upon reaching max time.	+9/-0
utils.go `Add Container Max Time Error in Utils` pkg/utils/utils.go Introduced `ContainerReachedMaxTime` error to signal when a container reaches its maximum allowed time.	+3/-1

✨ PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

codiumai-pr-agent-free[bot] commented 7 months ago

PR Description updated to latest commit (https://github.com/kubescape/node-agent/commit/cc745367a6efa9a1032d621849577fdae1fdb28d)

codiumai-pr-agent-free[bot] commented 7 months ago

PR Review

⏱️ Estimated effort to review [1-5]	4, because the PR introduces significant changes across multiple components, including new functionality for handling container max time events, modifications to existing interfaces, and the addition of new methods in several managers and mock classes. The complexity and breadth of these changes require a thorough review to ensure compatibility and correctness across the system.
🧪 Relevant tests	No
🔍 Possible issues	Synchronization Concerns: The implementation of `ContainerReachedMaxTime` across different managers involves sending signals through channels. It's crucial to ensure that these signals are correctly synchronized and do not lead to race conditions or deadlocks.
	Error Handling: The new error `ContainerReachedMaxTime` is introduced and used in various places. It's important to verify that all possible error paths are correctly handled and that appropriate actions are taken when this error occurs.
	Integration Testing: Given the changes impact multiple components (e.g., `ApplicationProfileManager`, `NetworkManager`, `RelevancyManager`), it's essential to perform integration testing to ensure these components work harmoniously and the new functionality behaves as expected in real-world scenarios.
🔒 Security concerns	No

✨ Review tool usage guide:

**Overview:** The `review` tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be [added](https://pr-agent-docs.codium.ai/tools/review/#general-configurations) by configuring the tool. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on any PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L23) related to the review tool (`pr_reviewer` section), use the following template: ``` /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_reviewer] some_config1=... some_config2=... ``` See the review [usage page](https://pr-agent-docs.codium.ai/tools/review/) for a comprehensive guide on using this tool.

codiumai-pr-agent-free[bot] commented 7 months ago

PR Code Suggestions

Category	Suggestions
Maintainability	Refactor repetitive calls to a loop or method for notifying managers about `ContainerReachedMaxTime`. ___ Consider using a loop or a dedicated method to notify all managers about `ContainerReachedMaxTime` to reduce code duplication and improve maintainability. [pkg/containerwatcher/v1/container_watcher_private.go [39-42]](https://github.com/kubescape/node-agent/pull/250/files#diff-6f95b4caa6090a17da5aed1923600fd049392d228e0fba99cc212f48111f3ffeR39-R42) ```diff -ch.applicationProfileManager.ContainerReachedMaxTime(notif.Container.Runtime.ContainerID) -ch.relevancyManager.ContainerReachedMaxTime(notif.Container.Runtime.ContainerID) -ch.networkManagerv1.ContainerReachedMaxTime(notif.Container.Runtime.ContainerID) -ch.networkManager.ContainerReachedMaxTime(notif.Container.Runtime.ContainerID) +managers := []interface{ ContainerReachedMaxTime(containerID string) }{ + ch.applicationProfileManager, + ch.relevancyManager, + ch.networkManagerv1, + ch.networkManager, +} +for _, manager := range managers { + manager.ContainerReachedMaxTime(notif.Container.Runtime.ContainerID) +} ```
Possible issue	Prevent potential blocking on channel send in `ContainerReachedMaxTime`. ___ Ensure that sending to the channel does not block indefinitely by selecting on a context cancellation or using a non-blocking send. [pkg/networkmanager/v1/network_manager.go [273]](https://github.com/kubescape/node-agent/pull/250/files#diff-91001aa3daf6f273c1ae3ded661c9acea7486080c3ff3da88c268ec56258fed0R273-R273) ```diff -channel <- utils.ContainerReachedMaxTime +select { +case channel <- utils.ContainerReachedMaxTime: +case <-time.After(5 * time.Second): + // Handle the timeout case or log a warning +} ```
Enhancement	Add safety checks and non-blocking send to prevent panic when sending to a channel. ___ Consider handling the case where the channel is closed before sending to prevent panic. [pkg/applicationprofilemanager/v1/applicationprofile_manager.go [145]](https://github.com/kubescape/node-agent/pull/250/files#diff-fc815317651e17975c117749e7661127dbcde82fd9d4d36ebc76cb5b09b3c54eR145-R145) ```diff -channel <- utils.ContainerReachedMaxTime +if channel, ok := am.watchedContainerChannels.Get(containerID); ok && channel != nil { + select { + case channel <- utils.ContainerReachedMaxTime: + default: + // Log error or handle the case where the channel is blocked or closed + } +} ```
Enhancement	Add error handling for missing channels in `ContainerReachedMaxTime`. ___ Add error handling or logging when the channel is not found or is nil to aid in debugging and fault tolerance. [pkg/networkmanager/v1/network_manager.go [272-273]](https://github.com/kubescape/node-agent/pull/250/files#diff-91001aa3daf6f273c1ae3ded661c9acea7486080c3ff3da88c268ec56258fed0R272-R273) ```diff -if channel := am.watchedContainerChannels.Get(containerID); channel != nil { +if channel, ok := am.watchedContainerChannels.Get(containerID); ok { channel <- utils.ContainerReachedMaxTime +} else { + log.Printf("Channel not found for containerID: %s", containerID) } ```
Best practice	Merge switch cases with identical outcomes for cleaner code. ___ To improve code readability and reduce redundancy, consider merging cases in `monitorContainer` that have identical bodies. [pkg/relevancymanager/v1/relevancy_manager.go [260-262]](https://github.com/kubescape/node-agent/pull/250/files#diff-f665b80e0e8d6552b56e677f5579b3b90cb7cd78999a4bec61d41522469393a3R260-R262) ```diff -case errors.Is(err, utils.ContainerReachedMaxTime): +case errors.Is(err, utils.ContainerReachedMaxTime), errors.Is(err, utils.IncompleteSBOMError): rm.handleRelevancy(ctx, watchedContainer, container.Runtime.ContainerID) return nil ```

✨ Improve tool usage guide:

**Overview:** The `improve` tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on a PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L78) related to the improve tool (`pr_code_suggestions` section), use the following template: ``` /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_code_suggestions] some_config1=... some_config2=... ``` See the improve [usage page](https://pr-agent-docs.codium.ai/tools/improve/) for a comprehensive guide on using this tool.

github-actions[bot] commented 7 months ago

Summary:

License scan: failure
Credentials scan: failure
Vulnerabilities scan: success
Unit test: success
Go linting: success

kubescape / node-agent