Don't update condition if status stays False/Unknown for custom plugin #320
Conversation
k8s-ci-robot
added
cncf-cla: yes
wangzhen127
force-pushed
the
custom-plugin-fix
branch
from
c48b635 to
b9e4a68
Compare
|
/assign @Random-Liu |
|
@wangzhen127: GitHub didn't allow me to request PR reviews from the following users: xueweiz. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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/test-infra repository. |
wangzhen127
force-pushed
the
custom-plugin-fix
branch
from
b4170c7 to
8e2ac9e
Compare
wangzhen127
changed the title
wangzhen127
force-pushed
the
custom-plugin-fix
branch
from
8e2ac9e to
16819b0
Compare
|
logic looks correct to me, I have the feeling there is a more logical ordering for the if/else, like first going over all the |
|
might be better to turn the new validation into it's own PR so that it can be merged more quickly (also needs a test ?) |
wangzhen127
force-pushed
the
custom-plugin-fix
branch
3 times, most recently
from
c65194c to
b5b61e4
Compare
wangzhen127
force-pushed
the
custom-plugin-fix
branch
from
b5b61e4 to
30e20c6
Compare
|
this bot is annoying ... there is commit status for that :/
…On Fri, Aug 2, 2019 at 1:42 PM Kubernetes Prow Robot < ***@***.***> wrote:
@wangzhen127 <https://github.com/wangzhen127>: The following test *failed*,
say /retest to rerun them all:
Test name Commit Details Rerun command
pull-npd-e2e-kubernetes-gce-ubuntu 30e20c6
<30e20c6>
link
<https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/node-problem-detector/320/pull-npd-e2e-kubernetes-gce-ubuntu/1157179442464821249> /test
pull-npd-e2e-kubernetes-gce-ubuntu
Full PR test history
<https://prow.k8s.io/pr-history?org=kubernetes&repo=node-problem-detector&pr=320>.
Your PR dashboard <https://gubernator.k8s.io/pr/wangzhen127>. Please help
us cut down on flakes by linking to
<https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests>
an open issue
<https://github.com/kubernetes/node-problem-detector/issues?q=is:issue+is:open>
when you hit one in your PR.
Instructions for interacting with me using PR comments are available here
<https://git.k8s.io/community/contributors/guide/pull-requests.md>. If
you have questions or suggestions related to my behavior, please file an
issue against the kubernetes/test-infra
<https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:>
repository. I understand the commands that are listed here
<https://go.k8s.io/bot-commands>.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#320?email_source=notifications&email_token=AAACYZ7G54WPCSOAO7EKRNLQCPJNNA5CNFSM4IIATNYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3MZEVA#issuecomment-517575252>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAACYZ36E2DMEBQLN7USJ53QCPJNNANCNFSM4IIATNYA>
.
|
|
/retest |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Random-Liu, wangzhen127 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 |
Fixes #202
When condition status stays True, we can update condition if reason or message changes. But when condition status stays False/Unknown, we should just ignore and do not update condition.
Here is the example:
result.Rule.Reasonis always different from the existing reason, which is the default reason set in step 1. In this case, we should just ignore and do not update condition.Also added some validation to check custom plugin config.
Fixes #306