Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ingress-gateways: don't log error when registering gateway #15001

Merged
merged 2 commits into from
Oct 25, 2022

Conversation

lkysow
Copy link
Member

@lkysow lkysow commented Oct 16, 2022

Previously, when an ingress gateway was registered without a corresponding ingress gateway config entry, an error was logged because the watch on the config entry returned a nil result. This is expected since the first call to watch will return a nil entry so don't log an error.

Fixes #11719

Error that used to be logged:

agent.proxycfg: Failed to handle update from watch: service_id=ingress-gateway id=gateway-config error="invalid type for config entry: <nil>"

PR Checklist

  • updated test coverage
  • not a security concern

Previously, when an ingress gateway was registered without a
corresponding ingress gateway config entry, an error was logged
because the watch on the config entry returned a nil result.
This is expected so don't log an error.
@lkysow lkysow force-pushed the lkysow/igw-logging-err branch from 8c95237 to f9667eb Compare October 16, 2022 02:34
@@ -91,6 +91,9 @@ func (s *handlerIngressGateway) handleUpdate(ctx context.Context, u UpdateEvent,
if !ok {
return fmt.Errorf("invalid type for response: %T", u.Result)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is to guard on resp.Entry being nil. One thing this doesn't handle is if the config entry is deleted. If it's deleted, I think the expectation would be that everything is reset back to the baseline. This can be done in a different PR though and likely isn't worth it if the api gateway work is completed.

@lkysow lkysow marked this pull request as ready for review October 16, 2022 16:00
@lkysow lkysow requested a review from a team October 21, 2022 18:11
@lkysow lkysow merged commit d3aa2bd into main Oct 25, 2022
@lkysow lkysow deleted the lkysow/igw-logging-err branch October 25, 2022 17:55
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consul logs error when ingress gateway is registered
2 participants