Events refactoring #190
Events refactoring #190
Conversation
- using node not found event from common - adding indication of whether the message was omitted by the agent or manager Signed-off-by: Michael Shitrit <[email protected]>
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mshitrit The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test 4.15-openshift-e2e |
| @@ -167,7 +167,7 @@ var _ = Describe("SNR Controller", func() { | |||
| It("Remediation flow", func() { | |||
| node := verifyNodeIsUnschedulable() | |||
|
|
|||
| verifyEvent("Normal", "RemediationStarted", "Remediation started") | |||
| verifyEvent("Normal", "RemediationStarted", "Remediation started by SNR manager") | |||
There was a problem hiding this comment.
How about using EventTypeNormal and EventTypeNormal from https://github.com/kubernetes/api/blob/v0.29.2/core/v1/types.go#L6354-L6359 instead of typing the string? Here and in other occurrences in this file.
There was a problem hiding this comment.
Instead of using the string RemediationStarted we can also use the common remediation event reasons.
There was a problem hiding this comment.
I don't think it's very significant in test code.
One can even argue against it in test code, because it would be safer to validate vs a "decoupled " hardcoded value (personally not an issue as well IMO).
There was a problem hiding this comment.
TBH my original response for the event type question 😅 (I only saw the followup after I posted)
I was just luck that my answer fitted nicely the second question
|
/lgtm |
ECOPROJECT-1836, ECOPROJECT-1884