Closed andreaskaris closed 1 year ago
Hi @andreaskaris. Thanks for your PR.
I'm waiting for a nmstate 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. Regular contributors should join the org to skip this step.
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.
/ok-to-test
/lgtm /approve
/retest-required
/retest
/retest
@andreaskaris are the unit test failure related ?
I thought it was due to flakes but I'll take another look
@andreaskaris: The following test failed, say /retest
to rerun all failed tests or /retest-required
to rerun all mandatory failed tests:
Test name | Commit | Details | Required | Rerun command |
---|---|---|---|---|
pull-kubernetes-nmstate-e2e-handler-k8s-future | eac5862d3fe9ea3774777520a4d8f1f64f269513 | link | false | /test pull-kubernetes-nmstate-e2e-handler-k8s-future |
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: qinqon
The full list of commands accepted by this bot can be found here.
The pull request process is described here
On top of reporting status via nodenetworkconfigurationenactments, also generate events for failures. These events directly show up in the output of
kubectl describe nodenetworkconfigurationpolicies
and thus provide a standardized way for administrators to understand why a reconciliation failed.Is this a BUG FIX or a FEATURE ?:
/kind enhancement
What this PR does / why we need it:
Administrators might not know that they can check their node's status with the nodenetworkconfigurationenactments resource. It's also a non-standard way of reporting the failure of a resource.
Special notes for your reviewer:
Consider this a suggestion / brainstorming for how issues with reconciling resources are reported to the users. Would it make sense to use events? If so, should they contain the full error message, or refer to the enactment resource? I personally really didn't know about the enactment resource until I started working on this PR, and I guess it might be the same for other people using kubernetes-nmstate. Reporting reconciliation issues through the enactment resource seems a bit non-standard to me, and the nodenetworkconfigurationpolicy atm only reports:
With this change, we'd get something like this:
Release note: