Add IncompatibleFilters reason for Accepted condition #2150
Conversation
New reason for configuration errors arising from invalid combination of route filters. Signed-off-by: Sunjay Bhatia <[email protected]>
k8s-ci-robot
added
release-note
k8s-ci-robot
added
the
size/M
| // document that limitation. In all cases where incompatible or unsupported | ||
| // filters are specified, implementations MUST add a warning condition to status. | ||
| // document that limitation. In cases where incompatible or unsupported | ||
| // filters are specified and cause the `Accepted` condition to be set to status |
There was a problem hiding this comment.
this section cause the Accepted condition to be set to status False is because you might have a partially valid route, in that case it would still be Accepted=True so doesn't really make sense to set the IncompatibleFilters reason
There was a problem hiding this comment.
Yeah, I ran into the same problem in #2155 where it would be really useful to be able to communicate "something's not quite right, but we're still able to at least partially reconcile this route". In that case it was if the timeout specified was more precise (ie ms or ns) than the underlying implementation could support.
There was a problem hiding this comment.
Thanks @sunjayBhatia! Will defer to someone else for final LGTM.
/cc @arkodg @mlavacca @howardjohn @shaneutt
/approve
| // document that limitation. In all cases where incompatible or unsupported | ||
| // filters are specified, implementations MUST add a warning condition to status. | ||
| // document that limitation. In cases where incompatible or unsupported | ||
| // filters are specified and cause the `Accepted` condition to be set to status |
There was a problem hiding this comment.
Yeah, I ran into the same problem in #2155 where it would be really useful to be able to communicate "something's not quite right, but we're still able to at least partially reconcile this route". In that case it was if the timeout specified was more precise (ie ms or ns) than the underlying implementation could support.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: robscott, sunjayBhatia 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 |
k8s-ci-robot
added
the
approved
| cannot support other combinations of filters, they must clearly document that | ||
| limitation. In all cases where incompatible or unsupported filters are | ||
| specified, implementations MUST add a warning condition to status. | ||
| If an implementation can not support a combinations of filters, they must clearly |
There was a problem hiding this comment.
nit: cannot (is more widely used)
|
/lgtm |
k8s-ci-robot
added
the
lgtm
What type of PR is this?
/kind cleanup
/kind documentation
What this PR does / why we need it:
Adds new reason for configuration errors arising from invalid combination of route filters.
Pulled out of #1540 per @robscott
Which issue(s) this PR fixes:
Fixes #1521
Does this PR introduce a user-facing change?: