-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
istio-gateway: Add debug logging when endpoints missing #2538
istio-gateway: Add debug logging when endpoints missing #2538
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. I understand the commands that are listed here. |
|
Welcome @mcwarman! |
@@ -172,6 +172,11 @@ func (sc *gatewaySource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, e | |||
return nil, err | |||
} | |||
|
|||
if len(gwEndpoints) == 0 { | |||
log.Debugf("No endpoints could be generated from gateway %s/%s", gateway.Namespace, gateway.Name) | |||
continue |
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.
This is not only logging, adding a continue
will have the effect of skipping lines 180 to 182. Is this desired?
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.
Probably a poor description and commit message but it is the intention.
180 reports that something will be added, where actually nothing is as the gwEndpoints
is empty.
Endpoints generated from gateway: example/gateway: []
181 and 182 will not do anything as there is nothing in the gwEndpoints
slice.
So the intention is to break out of the loop and provide a debug message that nothing was found.
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.
The continue
is also present is the virtual service equivalent istio_virtualservice.go#L168-L171.
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.
@Raffo are you happy with the description above?
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.
@Raffo is there anything else I need to do to get this merged?
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.
Yeah that looks good to me.
/assign @Raffo |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mcwarman, Raffo 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 |
Description
Add debug logging to
istio-gateway
for when an endpoint is missing, this adds logging parity toistio-virtualservice
.