Closed ronaknnathani closed 6 months ago
@mustafaakin Can I get a review for this?
https://github.com/resmoio/kubernetes-event-exporter/pull/34 is abandoned and this change takes care of what it was trying to do. Once this PR is shipped, https://github.com/resmoio/kubernetes-event-exporter/pull/34 can be closed. I already discussed it with @xmcqueen and he's okay with me taking this forward.
Let me know if you have any questions.
Thanks!
@mustafaakin gentle ping. any thoughts on this? if you're busy, is there someone else on the team who can review this?
Thanks a lot for a detailed PR. I wanted to switch to Lease at some point and there are issues where events are missed so I think it would resolve at least a few of them just due to lag in leader election.
Details
Currently, when a replica loses its leadership, a new leader isn't elected until leaseDuration seconds. Here, that is 15s. The max time till we get a new leader is leaseDuration (15s) + retryPeriod (2s) = 17s.
This commit updates the shutdown process such that if the leader replica is sent a shutdown signal, it sleeps for leaseDuration seconds. This allows the leader replica to continue to export events until a new leader is elected. And a new leader is elected only if lease hasn't been renewed and leaseDuration expires.
In addition to this, other smaller changes include:
sync.WaitGroup
so that we cleanly shut down the process after informer has stopped.For use cases where no event loss is tolerable, users should use maxEventAgeSeconds to > 1.
Issues addressed
https://github.com/resmoio/kubernetes-event-exporter/pull/34 is abandoned and this change takes care of what it was trying to do. Once this PR is shipped, https://github.com/resmoio/kubernetes-event-exporter/pull/34 can be closed. I already discussed it with @xmcqueen and he's okay with me taking this forward.
Testing done
Tests
``` go test -cover -mod=mod -v ./... ? github.com/resmoio/kubernetes-event-exporter [no test files] === RUN TestSimpleWriter --- PASS: TestSimpleWriter (0.00s) === RUN TestCorrectnessManyTimes --- PASS: TestCorrectnessManyTimes (0.07s) === RUN TestLargerThanBatchSize --- PASS: TestLargerThanBatchSize (0.00s) === RUN TestSimpleInterval --- PASS: TestSimpleInterval (0.06s) === RUN TestIntervalComplex --- PASS: TestIntervalComplex (0.06s) === RUN TestIntervalComplexAfterFlush --- PASS: TestIntervalComplexAfterFlush (0.06s) === RUN TestRetry --- PASS: TestRetry (0.20s) PASS github.com/resmoio/kubernetes-event-exporter/pkg/batch coverage: 100.0% of statements ok github.com/resmoio/kubernetes-event-exporter/pkg/batch 0.740s coverage: 100.0% of statements ? github.com/resmoio/kubernetes-event-exporter/pkg/metrics [no test files] ? github.com/resmoio/kubernetes-event-exporter/pkg/version [no test files] === RUN Test_ParseConfig --- PASS: Test_ParseConfig (0.00s) === RUN TestValidate_IsCheckingMaxEventAgeSeconds_WhenNotSet {"level":"info","time":"2024-01-17T15:48:01-05:00","message":"setting config.maxEventAgeSeconds=5 (default)"} {"level":"warn","time":"2024-01-17T15:48:01-05:00","message":"metrics name prefix is empty, setting config.metricsNamePrefix='event_exporter_' is recommended"} --- PASS: TestValidate_IsCheckingMaxEventAgeSeconds_WhenNotSet (0.00s) === RUN TestValidate_IsCheckingMaxEventAgeSeconds_WhenThrottledPeriodSet --- PASS: TestValidate_IsCheckingMaxEventAgeSeconds_WhenThrottledPeriodSet (0.00s) === RUN TestValidate_IsCheckingMaxEventAgeSeconds_WhenMaxEventAgeSecondsSet --- PASS: TestValidate_IsCheckingMaxEventAgeSeconds_WhenMaxEventAgeSecondsSet (0.00s) === RUN TestValidate_IsCheckingMaxEventAgeSeconds_WhenMaxEventAgeSecondsAndThrottledPeriodSet --- PASS: TestValidate_IsCheckingMaxEventAgeSeconds_WhenMaxEventAgeSecondsAndThrottledPeriodSet (0.00s) === RUN TestValidate_MetricsNamePrefix_WhenEmpty --- PASS: TestValidate_MetricsNamePrefix_WhenEmpty (0.00s) === RUN TestValidate_MetricsNamePrefix_WhenValid --- PASS: TestValidate_MetricsNamePrefix_WhenValid (0.00s) === RUN TestValidate_MetricsNamePrefix_WhenInvalid --- PASS: TestValidate_MetricsNamePrefix_WhenInvalid (0.00s) === RUN TestSetDefaults --- PASS: TestSetDefaults (0.00s) === RUN TestEngineNoRoutes --- PASS: TestEngineNoRoutes (0.00s) === RUN TestEngineSimple --- PASS: TestEngineSimple (0.00s) === RUN TestEngineDropSimple --- PASS: TestEngineDropSimple (0.00s) === RUN TestEmptyRoute --- PASS: TestEmptyRoute (0.00s) === RUN TestBasicRoute --- PASS: TestBasicRoute (0.00s) === RUN TestDropRule --- PASS: TestDropRule (0.00s) === RUN TestSingleLevelMultipleMatchRoute --- PASS: TestSingleLevelMultipleMatchRoute (0.00s) === RUN TestSubRoute --- PASS: TestSubRoute (0.00s) === RUN TestSubSubRoute --- PASS: TestSubSubRoute (0.00s) === RUN TestSubSubRouteWithDrop --- PASS: TestSubSubRouteWithDrop (0.00s) === RUN Test_GHIssue51 --- PASS: Test_GHIssue51 (0.00s) === RUN TestEmptyRule --- PASS: TestEmptyRule (0.00s) === RUN TestBasicRule --- PASS: TestBasicRule (0.00s) === RUN TestBasicNoMatchRule --- PASS: TestBasicNoMatchRule (0.00s) === RUN TestBasicRegexRule --- PASS: TestBasicRegexRule (0.00s) === RUN TestLabelRegexRule --- PASS: TestLabelRegexRule (0.00s) === RUN TestOneLabelMatchesRule --- PASS: TestOneLabelMatchesRule (0.00s) === RUN TestOneLabelDoesNotMatchRule --- PASS: TestOneLabelDoesNotMatchRule (0.00s) === RUN TestTwoLabelMatchesRule --- PASS: TestTwoLabelMatchesRule (0.00s) === RUN TestTwoLabelRequiredRule --- PASS: TestTwoLabelRequiredRule (0.00s) === RUN TestTwoLabelRequiredOneMissingRule --- PASS: TestTwoLabelRequiredOneMissingRule (0.00s) === RUN TestOneAnnotationMatchesRule --- PASS: TestOneAnnotationMatchesRule (0.00s) === RUN TestOneAnnotationDoesNotMatchRule --- PASS: TestOneAnnotationDoesNotMatchRule (0.00s) === RUN TestTwoAnnotationsMatchesRule --- PASS: TestTwoAnnotationsMatchesRule (0.00s) === RUN TestTwoAnnotationsRequiredOneMissingRule --- PASS: TestTwoAnnotationsRequiredOneMissingRule (0.00s) === RUN TestComplexRuleNoMatch --- PASS: TestComplexRuleNoMatch (0.00s) === RUN TestComplexRuleMatches --- PASS: TestComplexRuleMatches (0.00s) === RUN TestComplexRuleAnnotationsNoMatch --- PASS: TestComplexRuleAnnotationsNoMatch (0.00s) === RUN TestComplexRuleMatchesRegexp --- PASS: TestComplexRuleMatchesRegexp (0.00s) === RUN TestComplexRuleNoMatchRegexp --- PASS: TestComplexRuleNoMatchRegexp (0.00s) === RUN TestMessageRegexp --- PASS: TestMessageRegexp (0.00s) === RUN TestCount --- PASS: TestCount (0.00s) PASS github.com/resmoio/kubernetes-event-exporter/pkg/exporter coverage: 68.9% of statements ok github.com/resmoio/kubernetes-event-exporter/pkg/exporter 1.635s coverage: 68.9% of statements === RUN TestEnhancedEvent_DeDot === RUN TestEnhancedEvent_DeDot/nothing === RUN TestEnhancedEvent_DeDot/dedot --- PASS: TestEnhancedEvent_DeDot (0.00s) --- PASS: TestEnhancedEvent_DeDot/nothing (0.00s) --- PASS: TestEnhancedEvent_DeDot/dedot (0.00s) === RUN TestEnhancedEvent_DeDot_MustNotAlternateOriginal --- PASS: TestEnhancedEvent_DeDot_MustNotAlternateOriginal (0.00s) === RUN TestEventWatcher_EventAge_whenEventCreatedBeforeStartup --- PASS: TestEventWatcher_EventAge_whenEventCreatedBeforeStartup (0.00s) === RUN TestEventWatcher_EventAge_whenEventCreatedAfterStartupAndBeforeMaxAge --- PASS: TestEventWatcher_EventAge_whenEventCreatedAfterStartupAndBeforeMaxAge (0.00s) === RUN TestEventWatcher_EventAge_whenEventCreatedAfterStartupAndAfterMaxAge --- PASS: TestEventWatcher_EventAge_whenEventCreatedAfterStartupAndAfterMaxAge (0.00s) === RUN TestOnEvent_WithObjectMetadata --- PASS: TestOnEvent_WithObjectMetadata (0.00s) === RUN TestOnEvent_DeletedObjects --- PASS: TestOnEvent_DeletedObjects (0.00s) PASS github.com/resmoio/kubernetes-event-exporter/pkg/kube coverage: 29.7% of statements ok github.com/resmoio/kubernetes-event-exporter/pkg/kube 0.614s coverage: 29.7% of statements === RUN Test_ParseConfigFromBytes_ExampleConfigIsCorrect --- PASS: Test_ParseConfigFromBytes_ExampleConfigIsCorrect (0.00s) === RUN Test_ParseConfigFromBytes_NoErrors --- PASS: Test_ParseConfigFromBytes_NoErrors (0.00s) === RUN Test_ParseConfigFromBytes_ErrorWhenCurlyBracesNotEscaped --- PASS: Test_ParseConfigFromBytes_ErrorWhenCurlyBracesNotEscaped (0.00s) === RUN Test_ParseConfigFromBytes_OkWhenCurlyBracesEscaped --- PASS: Test_ParseConfigFromBytes_OkWhenCurlyBracesEscaped (0.00s) === RUN Test_ParseConfigFromBytes_ErrorErrorNotWithCurlyBraces --- PASS: Test_ParseConfigFromBytes_ErrorErrorNotWithCurlyBraces (0.00s) PASS github.com/resmoio/kubernetes-event-exporter/pkg/setup coverage: 100.0% of statements ok github.com/resmoio/kubernetes-event-exporter/pkg/setup 2.660s coverage: 100.0% of statements === RUN TestOpsCenterSink_Send === RUN TestOpsCenterSink_Send/Simple_Create === RUN TestOpsCenterSink_Send/Invalid_Priority:_Want_err --- PASS: TestOpsCenterSink_Send (0.00s) --- PASS: TestOpsCenterSink_Send/Simple_Create (0.00s) --- PASS: TestOpsCenterSink_Send/Invalid_Priority:_Want_err (0.00s) === RUN TestTeams_Send --- PASS: TestTeams_Send (0.00s) === RUN TestTeams_Send_WhenTeamsReturnsRateLimited --- PASS: TestTeams_Send_WhenTeamsReturnsRateLimited (0.00s) === RUN TestLayoutConvert --- PASS: TestLayoutConvert (0.00s) PASS github.com/resmoio/kubernetes-event-exporter/pkg/sinks coverage: 13.4% of statements ok github.com/resmoio/kubernetes-event-exporter/pkg/sinks 3.337s coverage: 13.4% of statements ```When leader election is disabled
**No sleeping during shutdown** ``` ➜ ~ kube logs event-exporter-7ddc6ff9b-rbs6w -f {"level":"info","time":"2024-01-17T20:53:00Z","message":"Reading config file /data/config.yaml"} {"level":"info","time":"2024-01-17T20:53:00Z","message":"Starting with config: {LogLevel:info LogFormat:json ThrottlePeriod:0 MaxEventAgeSeconds:0 ClusterName: Namespace: LeaderElection:{Enabled:false LeaderElectionID:} Route:{Drop:[] Match:[] Routes:[{Drop:[] Match:[{Labels:map[] Annotations:map[] Message: APIVersion: Kind: Namespace: Reason: Type: MinCount:0 Component: Host: Receiver:dump}] Routes:[]}]} Receivers:[{Name:dump InMemory:When leader election is enabled
**We have 2 replicas** ``` ➜ ~ kube get po NAME READY STATUS RESTARTS AGE event-exporter-7ddc6ff9b-fhcvq 1/1 Running 0 101s <------- LEADER event-exporter-7ddc6ff9b-hkzcw 1/1 Running 0 27s ``` **Non-leader instance stops right awat and doesn't wait for leaseDuration before stopping** ``` ✗ kube logs event-exporter-7ddc6ff9b-hkzcw -f {"level":"info","time":"2024-01-17T20:58:03Z","message":"Reading config file /data/config.yaml"} {"level":"info","time":"2024-01-17T20:58:03Z","message":"Starting with config: {LogLevel:info LogFormat:json ThrottlePeriod:0 MaxEventAgeSeconds:0 ClusterName: Namespace: LeaderElection:{Enabled:true LeaderElectionID:} Route:{Drop:[] Match:[] Routes:[{Drop:[] Match:[{Labels:map[] Annotations:map[] Message: APIVersion: Kind: Namespace: Reason: Type: MinCount:0 Component: Host: Receiver:dump}] Routes:[]}]} Receivers:[{Name:dump InMemory: