-
Notifications
You must be signed in to change notification settings - Fork 351
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: Solve the problem of final consistency when deleting #163
Conversation
pkg/apisix/resource.go
Outdated
@@ -116,6 +116,7 @@ func (i *item) upstream(clusterName string) (*v1.Upstream, error) { | |||
} | |||
|
|||
var ups upstreamItem | |||
log.Info(string(i.Value)) |
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.
It already logs i.Value
in line 112.
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.
Removed
apisixUpstreamYaml, err = c.apisixUpstreamList.ApisixUpstreams(namespace).Get(name) | ||
if err != nil { | ||
if errors.IsNotFound(err) { | ||
log.Infof("apisixUpstraem %s is removed", sqo.Key) |
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.
Type: apisixUpstraem
=> apisixUpstream
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.
Done
log.Infof("apisixUpstraem %s is removed", sqo.Key) | ||
return nil | ||
} | ||
runtime.HandleError(fmt.Errorf("failed to list apisixService %s/%s", sqo.Key, err.Error())) |
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.
Type: here we are listing ApisixUpstream
.
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.
Done
return | ||
} | ||
c.addFunc(newObj) | ||
var key string |
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.
Style:
var (
key string
err error
)
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.
Done
test/e2e/scaffold/ingress.go
Outdated
@@ -77,7 +77,7 @@ spec: | |||
tcpSocket: | |||
port: 8080 | |||
timeoutSeconds: 2 | |||
image: "apache/apisix-ingress-controller:dev" | |||
image: "apisix-ingress-controller:dev" |
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.
Why change it again?
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.
Done
Codecov Report
@@ Coverage Diff @@
## master #163 +/- ##
==========================================
- Coverage 50.21% 48.39% -1.82%
==========================================
Files 30 30
Lines 1414 1467 +53
==========================================
Hits 710 710
- Misses 620 673 +53
Partials 84 84
Continue to review full report at Codecov.
|
* test: add one more e2e case * chore: removed the useless method * fix: readd the error handling
Solve the problem of final consistency when deleting.
Support retry when remove upstream or service failed.
Fix #118