You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "Revolyssup (via GitHub)" <gi...@apache.org> on 2024/01/22 07:05:13 UTC
[PR] fix: spaming endpoint/service not found errors [apisix-ingress-controller]
Revolyssup opened a new pull request, #2146:
URL: https://github.com/apache/apisix-ingress-controller/pull/2146
<!-- Please answer these questions before submitting a pull request -->
### Type of change:
Replica of https://github.com/apache/apisix-ingress-controller/pull/1626
<!-- Please delete options that are not relevant. -->
<!-- Select all the options from below that matches the type your PR best -->
- [ ] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
- [ ] Documentation
- [ ] Refactor
- [ ] Chore
- [ ] CI/CD or Tests
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
<!--
Please follow the requirements:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. Use "request review" to notify the reviewer once you have resolved the review
-->
- [ ] Did you explain what problem does this PR solve? Or what new features have been added?
- [ ] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document?
- [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix-ingress-controller#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: retry syncing when the error is k8s.NotFound [apisix-ingress-controller]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2146:
URL: https://github.com/apache/apisix-ingress-controller/pull/2146#issuecomment-2087831002
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: spaming endpoint/service not found errors [apisix-ingress-controller]
Posted by "zll600 (via GitHub)" <gi...@apache.org>.
zll600 commented on code in PR #2146:
URL: https://github.com/apache/apisix-ingress-controller/pull/2146#discussion_r1463067664
##########
pkg/providers/translation/service.go:
##########
@@ -35,9 +35,15 @@ import (
func (t *translator) TranslateService(namespace, name, subset string, port int32) (*apisixv1.Upstream, error) {
endpoint, err := t.EndpointLister.GetEndpoint(namespace, name)
if err != nil {
- return nil, &TranslateError{
- Field: "endpoints",
- Reason: err.Error(),
+ if k8serrors.IsNotFound(err) {
+ //Do not error wrap so that IsNotFound error can be handled separately on upper level
+ //Reference https://github.com/apache/apisix-ingress-controller/issues/1625
+ return nil, err
+ } else {
Review Comment:
It seems that this `else` is not necessary. Can you remove it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: spaming endpoint/service not found errors [apisix-ingress-controller]
Posted by "zll600 (via GitHub)" <gi...@apache.org>.
zll600 commented on code in PR #2146:
URL: https://github.com/apache/apisix-ingress-controller/pull/2146#discussion_r1463384060
##########
pkg/providers/translation/service.go:
##########
@@ -35,9 +35,15 @@ import (
func (t *translator) TranslateService(namespace, name, subset string, port int32) (*apisixv1.Upstream, error) {
endpoint, err := t.EndpointLister.GetEndpoint(namespace, name)
if err != nil {
- return nil, &TranslateError{
- Field: "endpoints",
- Reason: err.Error(),
+ if k8serrors.IsNotFound(err) {
+ //Do not error wrap so that IsNotFound error can be handled separately on upper level
+ //Reference https://github.com/apache/apisix-ingress-controller/issues/1625
+ return nil, err
+ } else {
Review Comment:
But if the error is `NotFound`, this function will return immediately. Otherwise this function will return a `&TranslateEror`. right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: retry syncing when the error is k8s.NotFound [apisix-ingress-controller]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2146:
URL: https://github.com/apache/apisix-ingress-controller/pull/2146#issuecomment-2028519814
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: spaming endpoint/service not found errors [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2146:
URL: https://github.com/apache/apisix-ingress-controller/pull/2146#discussion_r1464358686
##########
pkg/providers/apisix/apisix_consumer.go:
##########
@@ -184,15 +184,6 @@ func (c *apisixConsumerController) handleSyncErr(obj interface{}, err error) {
c.MetricsCollector.IncrSyncOperation("consumer", "success")
return
}
- event := obj.(*types.Event)
Review Comment:
dito
##########
pkg/providers/apisix/apisix_cluster_config.go:
##########
@@ -219,15 +219,6 @@ func (c *apisixClusterConfigController) handleSyncErr(obj interface{}, err error
c.MetricsCollector.IncrSyncOperation("clusterConfig", "success")
return
}
- event := obj.(*types.Event)
Review Comment:
refs: https://github.com/apache/apisix-ingress-controller/issues/810
1. There may be an expired event during the retry, which means it was deleted when the retry occurred, and this event will continue to be triggered.
2. What is the reason why this resource status needs to be retried? What should be the retry strategy? I think rough deletion is not advisable.
3. The changes here are not related to the title of the PR. If you think you need to try again, please submit a separate PR for discussion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: retry syncing when the error is k8s.NotFound [apisix-ingress-controller]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #2146: fix: retry syncing when the error is k8s.NotFound
URL: https://github.com/apache/apisix-ingress-controller/pull/2146
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: spaming endpoint/service not found errors [apisix-ingress-controller]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2146:
URL: https://github.com/apache/apisix-ingress-controller/pull/2146#issuecomment-1903382484
## [Codecov](https://app.codecov.io/gh/apache/apisix-ingress-controller/pull/2146?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `9 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`96510b6`)](https://app.codecov.io/gh/apache/apisix-ingress-controller/commit/96510b68462e1a94370ed1d537beb24f2c16e234?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 37.39% compared to head [(`f5a5921`)](https://app.codecov.io/gh/apache/apisix-ingress-controller/pull/2146?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 37.62%.
> Report is 1 commits behind head on master.
> :exclamation: Current head f5a5921 differs from pull request most recent head 327808f. Consider uploading reports for the commit 327808f to get more accurate results
| [Files](https://app.codecov.io/gh/apache/apisix-ingress-controller/pull/2146?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [pkg/providers/translation/service.go](https://app.codecov.io/gh/apache/apisix-ingress-controller/pull/2146?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3Byb3ZpZGVycy90cmFuc2xhdGlvbi9zZXJ2aWNlLmdv) | 12.50% | [4 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/apisix-ingress-controller/pull/2146?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pkg/providers/ingress/ingress.go](https://app.codecov.io/gh/apache/apisix-ingress-controller/pull/2146?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3Byb3ZpZGVycy9pbmdyZXNzL2luZ3Jlc3MuZ28=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/apisix-ingress-controller/pull/2146?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #2146 +/- ##
==========================================
+ Coverage 37.39% 37.62% +0.22%
==========================================
Files 94 94
Lines 7972 7977 +5
==========================================
+ Hits 2981 3001 +20
+ Misses 4597 4576 -21
- Partials 394 400 +6
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/apisix-ingress-controller/pull/2146?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: spaming endpoint/service not found errors [apisix-ingress-controller]
Posted by "Vacant2333 (via GitHub)" <gi...@apache.org>.
Vacant2333 commented on code in PR #2146:
URL: https://github.com/apache/apisix-ingress-controller/pull/2146#discussion_r1463267844
##########
pkg/providers/translation/service.go:
##########
@@ -35,9 +35,15 @@ import (
func (t *translator) TranslateService(namespace, name, subset string, port int32) (*apisixv1.Upstream, error) {
endpoint, err := t.EndpointLister.GetEndpoint(namespace, name)
if err != nil {
- return nil, &TranslateError{
- Field: "endpoints",
- Reason: err.Error(),
+ if k8serrors.IsNotFound(err) {
+ //Do not error wrap so that IsNotFound error can be handled separately on upper level
+ //Reference https://github.com/apache/apisix-ingress-controller/issues/1625
+ return nil, err
+ } else {
Review Comment:
no i think its necessay, the issue was cause by it didnt handle the error `NotFound`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org