Add unit tests for the custom plugin monitor

[DigitalVeer]

This PR bumps the unit test coverage for custom plugin monitor from 0.8% to 95.3%

---

This PR adds tests for generateStatus, the monitor lifecycle and the metrics path.

This is the first item of #1328.

Package	Before	After
pkg/custompluginmonitor	0.8%	95.3%

What the tests check

• TestGenerateStatusForConditions replays plugin results
  against one monitor and checks the condition and the event after each step for five scenarios.

• TestGenerateStatusForTemporaryProblem checks that a temporary rule creates an event and
  moves no condition.

• TestGenerateStatusMetrics checks the problem counter and the problem gauge through the
  existing metrics stub. The counter increments for new problems only.

• TestMonitorLoopReportsPluginResults and TestMonitorLoopExitsWhenResultChannelCloses
  check the goroutine lifecycle. Stop must return. A closed result channel must end the
  loop.

• TestNewCustomPluginMonitorOrDie writes a config file and checks that the constructor
  applies the defaults.

The lifecycle tests use the plugin scripts in pkg/custompluginmonitor/plugin/test-data.

Validation

Check	Result
go test -cover ./pkg/custompluginmonitor/	95.3%
go test -timeout=1m -race -short -count=5	pass, 3.6s
golangci-lint run --config .golangci.yml	0 issues

[kubernetes-prow]

Hi @DigitalVeer. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: DigitalVeer
Once this PR has been reviewed and has the lgtm label, please assign random-liu for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hakman]

Thanks @DigitalVeer, I added a few suggestions.

[hakman]

The expected event is built with the same helper production uses, so this doesn't independently check severity. If that helper always returned Warn, resolution events would become warnings and the whole suite would still pass.

Suggested change

	event := got.Events[0]
	event := got.Events[0]
	// Assert severity literally: `want` below is built with the same helper
	// generateStatus uses, so it cannot catch a bad severity mapping.
	wantSeverity := types.Info
	if s.wantStatus == types.True {
	wantSeverity = types.Warn
	}
	assert.Equal(t, wantSeverity, event.Severity, "step %d", i)

[hakman]

requireStop shows that Stop() returned, but not that the plugin it owns actually stopped. Delete c.plugin.Stop() at custom_plugin_monitor.go:148 and this package still passes under -race, with the plugin's ticker and rule goroutines left running.

Suggested change

	requireStop(t, c)
	requireStop(t, c)
	// Stop must also shut down the plugin it owns.
	select {
	case _, open := <-c.plugin.GetResultChan():
	assert.False(t, open, "Stop did not close the plugin result channel")
	case <-time.After(testWait):
	t.Fatal("Stop did not close the plugin result channel")
	}

[hakman]

This doesn't observe whether monitorLoop returns. With no rules, no result-derived status can follow the initial one, so requireNoStatus at the end passes either way — change the closed-channel return at custom_plugin_monitor.go:141 to break and the loop spins forever while this test stays green. Running the loop directly makes its completion observable; Start is still covered by TestMonitorLoopReportsPluginResults.

Suggested change

	statusChan, err := c.Start()
	require.NoError(t, err)
	status := receiveStatus(t, statusChan)
	startedAt := time.Now()
	// Start is not used here: monitorLoop runs directly so its return is observable.
	// Stop would deadlock, because the closed-channel path never calls tomb.Done.
	go c.plugin.Run()
	returned := make(chan struct{})
	go func() {
	c.monitorLoop()
	close(returned)
	}()
	status := receiveStatus(t, c.statusChan)

[hakman]

These are the only conditions in the suite with a real initialization time — everywhere else newTestMonitor overwrites Transition with testEpoch. Drop the assignment at custom_plugin_monitor.go:321 and every condition ships a zero LastTransitionTime, with the suite still passing. Uses the startedAt captured above.

Suggested change

	assert.Equal(t, types.False, cond.Status)
	assert.Equal(t, types.False, cond.Status)
	assert.WithinRange(t, cond.Transition, startedAt, time.Now())

[hakman]

This is what makes the test check what its name says. The deadline keeps a regression here a focused failure instead of a hang that times out the whole run.

Suggested change

	c.plugin.Stop()
	requireNoStatus(t, statusChan)
	c.plugin.Stop()
	requireNoStatus(t, c.statusChan)
	select {
	case <-returned:
	case <-time.After(testWait):
	t.Fatal("monitorLoop did not return after the result channel closed")
	}