-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update error messages to show why no interface is selected #434
Update error messages to show why no interface is selected #434
Conversation
Thanks for your PR,
To skip the vendors CIs use one of:
|
27c69c0
to
25d211b
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
25d211b
to
086268c
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
086268c
to
053ce3f
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
053ce3f
to
07b6352
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
Please ensure that this PR: #422 gets merged before merging the current PR. |
I think these changes would benefit from a GitHub issue being created for sriov-network-operator. One showing the deficiency with the current logging (with example config and log) with maybe several test cases with the example config. Then showing with these set of PRs applied, what the logs would look like instead. Highlighting the improvements and debug-ability. |
This PR needs to be rebased on the other PR. Git will automatically detect that the other PR already contains the cleanup and if that other PR is merged, this PR will only contain the final commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments.
in general I think this will spam the logs with info about all the devices on the system everytime someone apply a policy no?
Also I don't understand the change is only on the logs the end user when applying a yaml with oc apply
will not see any change in the message?
07b6352
to
975a928
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
975a928
to
beed237
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
beed237
to
a7108b2
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
It's only when there's no compatible devices, it will provide a summary of all the interfaces on every node and why that failed to match.
It does output to the logs to improve debugability. It will output if there's no matching error. |
a7108b2
to
599a8b4
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
599a8b4
to
9fda8e0
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
@wizhaoredhat I tested out the changes on my system.It lists out the interfaces and describes exactly what is not matching on each node. This is what partial matching logs look:
|
No matching logs look like this:
|
9fda8e0
to
d9c440d
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
6d765c2
to
0907f22
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
0907f22
to
eeab302
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
eeab302
to
0ed13f9
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small comments, otherwise lgtm
0ed13f9
to
e8c834d
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
e8c834d
to
7175b89
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
7175b89
to
8294bf3
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
8294bf3
to
4ebb54f
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
4ebb54f
to
6c0d99f
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
6c0d99f
to
f18f336
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
f18f336
to
5e0b05d
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
When the SRIOV network node state is not properly initialized it can hit the error "no supported NIC is selected by the nicSelector" even though the NIC may be indeed be selected. This commit updates the error message to ensure that if the user is configuring a NIC that is supported, then the error is because the SRIOV network node state is not properly initialized.
5e0b05d
to
efb48ad
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
When the SRIOV network node state is not properly initialized it can hit the error "no supported NIC is selected by the nicSelector" even though the NIC may be indeed be selected. This commit updates the error message to ensure that if the user is configuring a NIC that is supported, then the error is because the SRIOV network node state is not properly initialized.