Skip to content
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

fix: endless retry if namespace doesn't exist #882

Merged
merged 2 commits into from
Feb 23, 2022

Conversation

lingsamuel
Copy link
Member

Signed-off-by: Ling Samuel lingsamuelgrace@gmail.com

Type of change:

Fixes #816

  • Bugfix

What this PR does / why we need it:

I couldn't find a stable way to reproduce this issue without changing the code, so there is no e2e test.
The only case this PR addresses is the error not found. Please add other cases if necessary.

If anyone else wants to reproduce this issue, please postpone the Create/Update events (example: wrap the function in a goroutine and add a time.Sleep at the beginning).

Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2022

Codecov Report

Merging #882 (884358e) into master (545d22d) will decrease coverage by 0.03%.
The diff coverage is 7.46%.

❗ Current head 884358e differs from pull request most recent head c02c697. Consider uploading reports for the commit c02c697 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #882      +/-   ##
==========================================
- Coverage   32.07%   32.03%   -0.04%     
==========================================
  Files          71       71              
  Lines        7739     7748       +9     
==========================================
  Hits         2482     2482              
- Misses       4982     4991       +9     
  Partials      275      275              
Impacted Files Coverage Δ
pkg/ingress/apisix_consumer.go 0.00% <0.00%> (ø)
pkg/ingress/apisix_pluginconfig.go 0.00% <0.00%> (ø)
pkg/ingress/apisix_route.go 0.00% <0.00%> (ø)
pkg/ingress/apisix_tls.go 0.00% <0.00%> (ø)
pkg/ingress/apisix_upstream.go 0.00% <0.00%> (ø)
pkg/ingress/compare.go 0.00% <0.00%> (ø)
pkg/ingress/endpoint.go 0.00% <0.00%> (ø)
pkg/ingress/endpointslice.go 0.00% <0.00%> (ø)
pkg/ingress/gateway.go 0.00% <0.00%> (ø)
pkg/ingress/ingress.go 6.81% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 545d22d...c02c697. Read the comment docs.

Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

LGTM

@tao12345666333
Copy link
Member

ping @gxthrj @tokers PTAL, thanks!

Copy link
Contributor

@gxthrj gxthrj left a comment

Choose a reason for hiding this comment

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

LGTM

zap.String("event_type", event.Type.String()),
zap.String("namespace", event.Object.(string)),
)
c.workqueue.Forget(event)
Copy link
Contributor

Choose a reason for hiding this comment

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

So here is the point :)

Copy link
Member

Choose a reason for hiding this comment

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

yep

@tao12345666333 tao12345666333 merged commit d30bef5 into apache:master Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: namespace update event will be retried unconditionally even if it was deleted
4 participants