feat: add event handling strategy changes in csp-health-monitor #641

tanishagoyal2 · 2025-12-23T04:52:20Z

Summary

Type of Change

Component(s) Affected

Testing

Tests pass locally
Manual testing completed
No breaking changes (or documented)

Checklist

Self-review completed
Documentation updated (if needed)
Ready for review

Summary by CodeRabbit

Release Notes

New Features
- Added configurable processing strategy for health events: EXECUTE_REMEDIATION (default) performs reactive operations on health events, while STORE_ONLY only stores and exports events without making cluster changes. Users can now control the response behavior for detected health issues.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Signed-off-by: Tanisha goyal <[email protected]>

coderabbitai · 2025-12-23T04:52:30Z

Walkthrough

A new ProcessingStrategy enum is added to HealthEvent protobuf with two values: EXECUTE_REMEDIATION and STORE_ONLY. This strategy is propagated throughout the system—from health monitor CLI flags through trigger engine, platform connectors, event exporters, and storage pipelines—to control whether health events trigger actual remediation actions or only store and export events.

Changes

Cohort / File(s)	Summary
Protobuf Core Definitions `data-models/protobufs/health_event.proto`, `health-monitors/gpu-health-monitor/gpu_health_monitor/protos/health_event_pb2.py`, `health-monitors/gpu-health-monitor/gpu_health_monitor/protos/health_event_pb2.pyi`	Added `ProcessingStrategy` enum with EXECUTE_REMEDIATION and STORE_ONLY values; added `processingStrategy` field to HealthEvent message (field 16); generated Python stub classes and type hints
CSP Health Monitor Configuration `distros/kubernetes/nvsentinel/charts/csp-health-monitor/values.yaml`, `distros/kubernetes/nvsentinel/charts/csp-health-monitor/templates/deployment.yaml`	Added `processingStrategy` Helm value (defaults to EXECUTE_REMEDIATION) and passed it as `--processing-strategy` argument to maintenance-notifier container
CSP Health Monitor CLI & Engine `health-monitors/csp-health-monitor/cmd/maintenance-notifier/main.go`, `health-monitors/csp-health-monitor/pkg/triggerengine/trigger.go`	Added processingStrategy CLI flag parsing and validation; updated `NewEngine` constructor signature to accept and store `ProcessingStrategy`; propagates strategy into emitted HealthEvents
Trigger Engine Tests `health-monitors/csp-health-monitor/pkg/triggerengine/trigger_test.go`	Updated all `NewEngine` calls to pass `pb.ProcessingStrategy_EXECUTE_REMEDIATION` argument
Platform Connector Event Processing `platform-connectors/pkg/connectors/kubernetes/process_node_events.go`, `platform-connectors/pkg/connectors/kubernetes/k8s_platform_connector_test.go`	Added `filterProcessableEvents` to skip STORE_ONLY events; added `createK8sEvent` helper; refactored processHealthEvents to operate on filtered events only; added `TestProcessHealthEvents_StoreOnlyStrategy` test validating no conditions/events created for STORE_ONLY strategy
Storage Pipeline Builders `store-client/pkg/client/pipeline_builder.go`, `store-client/pkg/client/mongodb_pipeline_builder.go`, `store-client/pkg/client/postgresql_pipeline_builder.go`, `store-client/pkg/client/pipeline_builder_test.go`	Added `BuildProcessableHealthEventInsertsPipeline` interface method and implementations for MongoDB and PostgreSQL; filters inserts where processingStrategy = EXECUTE_REMEDIATION
Fault Quarantine Initialization `fault-quarantine/pkg/initializer/init.go`, `fault-quarantine/pkg/evaluator/rule_evaluator_test.go`	Changed pipeline from `BuildAllHealthEventInsertsPipeline()` to `BuildProcessableHealthEventInsertsPipeline()`; updated test expectation to include processingStrategy field
Event Exporter `event-exporter/pkg/transformer/cloudevents.go`, `event-exporter/pkg/transformer/cloudevents_test.go`	Added processingStrategy to CloudEvent healthEventData map; updated test to include and validate processingStrategy field
Integration Test Infrastructure `tests/helpers/healthevent.go`, `tests/helpers/event_exporter.go`, `tests/helpers/kube.go`	Added `ProcessingStrategy` field and `WithProcessingStrategy` setter to HealthEventTemplate; added `FindEventByNodeAndCheckName` helper and extended `ValidateCloudEvent` to accept and validate expectedProcessingStrategy; added `WaitForDaemonSetRollout`, `SetDeploymentArgs`, `RemoveDeploymentArgs` helpers for deployment argument manipulation
Integration Tests `tests/csp_health_monitor_test.go`, `tests/event_exporter_test.go`	Added `TestCSPHealthMonitorStoreOnlyProcessingStrategy` validating no cordon/drain under STORE_ONLY strategy; updated `TestEventExporterChangeStream` to validate EXECUTE_REMEDIATION in CloudEvent

Sequence Diagram(s)

sequenceDiagram
    participant HM as Health Monitor
    participant TE as Trigger Engine
    participant PC as Platform Connector
    participant EE as Event Exporter
    participant Store
    
    HM->>HM: Parse processingStrategy flag
    HM->>TE: NewEngine(processingStrategy)
    TE->>TE: Store processingStrategy
    Note over HM,TE: Event Creation Flow
    HM->>TE: Health check result
    TE->>TE: Create HealthEvent<br/>with processingStrategy
    TE->>PC: Send HealthEvent
    
    alt EXECUTE_REMEDIATION
        rect rgb(200, 230, 200)
            Note over PC: Processable Event Path
            PC->>PC: Filter event (processable)
            PC->>PC: Create node conditions
            PC->>PC: Create Kubernetes events
            PC->>PC: Update node state
        end
    else STORE_ONLY
        rect rgb(255, 200, 200)
            Note over PC: Store Only Path
            PC->>PC: Skip event processing
            PC->>PC: Log skipped event
        end
    end
    
    PC->>EE: HealthEvent
    EE->>EE: Transform to CloudEvent<br/>includeProcessingStrategy
    EE->>Store: Export CloudEvent
    EE->>Store: Persist event
    Store->>Store: Index by processingStrategy

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: introducing event handling strategy changes to the csp-health-monitor, which aligns with the ProcessingStrategy field additions and strategy-based event filtering throughout the codebase.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (2)

tests/helpers/kube.go (1)
2338-2393: Minor: Docstring mentions environment variables instead of arguments.

The function docstring says "removes environment variables" but this function removes container arguments.
Suggested fix for docstring
-// RemoveDeploymentArgs removes environment variables from containers in a deployment.
+// RemoveDeploymentArgs removes arguments from containers in a deployment.
 // If containerName is empty, removes from all containers. Otherwise, removes only from the named container.
 // Uses retry.RetryOnConflict for automatic retry handling.
tests/csp_health_monitor_test.go (1)
522-535: Consider using the return value of TeardownCSPHealthMonitorTest.

The teardown correctly restores the processing strategy, but TeardownCSPHealthMonitorTest returns a context that is discarded. Other tests in this file return the result of this call. While this may be intentional, it should be consistent with other teardown patterns.
-		helpers.TeardownCSPHealthMonitorTest(ctx, t, c, testCtx)
-
-		return ctx
+		return helpers.TeardownCSPHealthMonitorTest(ctx, t, c, testCtx)

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 82e7180 and 61f34e7.

⛔ Files ignored due to path filters (1)

data-models/pkg/protos/health_event.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (23)

data-models/protobufs/health_event.proto
distros/kubernetes/nvsentinel/charts/csp-health-monitor/templates/deployment.yaml
distros/kubernetes/nvsentinel/charts/csp-health-monitor/values.yaml
event-exporter/pkg/transformer/cloudevents.go
event-exporter/pkg/transformer/cloudevents_test.go
fault-quarantine/pkg/evaluator/rule_evaluator_test.go
fault-quarantine/pkg/initializer/init.go
health-monitors/csp-health-monitor/cmd/maintenance-notifier/main.go
health-monitors/csp-health-monitor/pkg/triggerengine/trigger.go
health-monitors/csp-health-monitor/pkg/triggerengine/trigger_test.go
health-monitors/gpu-health-monitor/gpu_health_monitor/protos/health_event_pb2.py
health-monitors/gpu-health-monitor/gpu_health_monitor/protos/health_event_pb2.pyi
platform-connectors/pkg/connectors/kubernetes/k8s_platform_connector_test.go
platform-connectors/pkg/connectors/kubernetes/process_node_events.go
store-client/pkg/client/mongodb_pipeline_builder.go
store-client/pkg/client/pipeline_builder.go
store-client/pkg/client/pipeline_builder_test.go
store-client/pkg/client/postgresql_pipeline_builder.go
tests/csp_health_monitor_test.go
tests/event_exporter_test.go
tests/helpers/event_exporter.go
tests/helpers/healthevent.go
tests/helpers/kube.go

🧰 Additional context used

📓 Path-based instructions (5)

**/*.go