Add events for config daemon #519
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
Example: |
|
@zeeke @adrianchiris please take a look |
zeeke
reviewed
Oct 10, 2023
There was a problem hiding this comment.
Left a few minor comments but the code is straightforward. great work!
Do you mind adding a basic test check to test/conformance/tests/_test_sriov_operator.go#L168 ?
Something like
Eventually(func() bool {
events, err := clients.Events(operatorNamespace).List(context.Background(), metav1.ListOptions{})
Expect(err).ToNot(HaveOccurred())
for _, e := range events.Items {
if strings.Contains(e.Message, "Config Daemon starting") {
return true
}
}
return false
}, 30*time.Second, 5*time.Second).Should(BeTrue(), "Config Daemon should record an event when starting")
I can propose it in a different pull request if you have trouble with that suite
| func NewEventRecorder(c snclientset.Interface, n string, kubeclient kubernetes.Interface) *EventRecorder { | ||
| eventBroadcaster := record.NewBroadcaster() | ||
| eventBroadcaster.StartStructuredLogging(4) | ||
| eventBroadcaster.StartRecordingToSink(&typedv1core.EventSinkImpl{Interface: kubeclient.CoreV1().Events("")}) |
There was a problem hiding this comment.
Can we use operator namespace for the Events? I think that way we can get a list of all events happened in the cluster via
kubectl -n sriov-network-operator get events
There was a problem hiding this comment.
The events are on sriovnetworknodestate which is a namespaced object. so they will be available.
Note that the events will disappear after 1 hour. (kubernetes/kubernetes#52521)
$ kubectl get events -n network-operator
LAST SEEN TYPE REASON OBJECT MESSAGE
8m35s Normal SyncStatusChanged sriovnetworknodestate/cloud-dev-10 Status changed from: Succeeded to: InProgress
2m7s Normal DrainNode sriovnetworknodestate/cloud-dev-10 Drain node has been initiated
2m7s Normal DrainNode sriovnetworknodestate/cloud-dev-10 Drain node completed
2m7s Normal RebootNode sriovnetworknodestate/cloud-dev-10 Reboot node has been initiated
8m35s Normal SyncStatusChanged sriovnetworknodestate/cloud-dev-11 Status changed from: Succeeded to: InProgress
8m4s Normal DrainNode sriovnetworknodestate/cloud-dev-11 Drain node has been initiated
8m4s Normal DrainNode sriovnetworknodestate/cloud-dev-11 Drain node completed
8m4s Normal RebootNode sriovnetworknodestate/cloud-dev-11 Reboot node has been initiated
2m59s Normal ConfigDaemonStart sriovnetworknodestate/cloud-dev-11 Config Daemon starting
2m59s Normal SyncStatusChanged sriovnetworknodestate/cloud-dev-11 Status changed from: InProgress to: Unknown
2m53s Normal SyncStatusChanged sriovnetworknodestate/cloud-dev-11 Status changed from: Unknown to: InProgress
2m8s Normal SyncStatusChanged sriovnetworknodestate/cloud-dev-11 Status changed from: InProgress to: Succeeded
87s Warning NodeNotReady pod/cni-plugins-ds-259wh Node is not ready
7m32s Warning NodeNotReady pod/cni-plugins-ds-jtpkk Node is not ready
There was a problem hiding this comment.
Oh, that's exactly what I was referring to. Thanks for explaining.
About the deletion of the events, it looks like it can be tuned via a kubeapi-server argument:
--event-ttl duration Default: 1h0m0s
IMO it's OK that is on the cluster administrator's side
K8s Events on SriovNetworkNodeStates objects will be created on following stages: - Status change of SriovNetworkNodeStates - Start of config daemon - Node reboot started - Node drain started/finished Signed-off-by: Fred Rolland <[email protected]>
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Yeah, it would be great. I tried to run the suite on my env and it failed. |
e0ne
approved these changes
Oct 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
K8s Events on SriovNetworkNodeStates objects
will be created on following stages:
Fixes #510