-
Notifications
You must be signed in to change notification settings - Fork 478
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: when there is a non-ip IPAddr in Eureka, delete it to avoid a failure in EDS #1322
base: main
Are you sure you want to change the base?
Conversation
break | ||
} | ||
} | ||
if shouldDelete == true { |
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.
这种 == true
是不是就不用写了。。。
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.
这个确实不需要了。。
|
||
var ( | ||
keysToDelete []string | ||
shouldDelete bool |
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.
这个变量是不是没必要放这么外面呀,留在for里面就行了
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.
这个要在for循环外面使用,要放在外面吧
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.
我是说 shouldDelete
。。。
for name, app := range apps { | ||
shouldDelete = false | ||
for _, instant := range app.Instances { |
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.
instant?
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.
instance,我改一下
@@ -128,14 +129,34 @@ func (c *eurekaHttpClient) getApplications(path string) (*Applications, error) { | |||
app := rj.Response.Applications[idx] | |||
apps[app.Name] = app |
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.
看了一下,是不是直接把下面的逻辑放到这个循环里就行了。。。
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.
对!
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.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1322 +/- ##
==========================================
+ Coverage 35.91% 44.21% +8.30%
==========================================
Files 69 76 +7
Lines 11576 9895 -1681
==========================================
+ Hits 4157 4375 +218
+ Misses 7104 5184 -1920
- Partials 315 336 +21 |
Ⅰ. Describe what this PR did
当eureka中注册的服务IPAddr为域名而非ip时,会导致其他EDS下发失败,故打印waring并忽略该应用
Ⅱ. Does this pull request fix one issue?
fixes #1284
fixes #1299
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
在启动higress之前往eureka注册中心注册一个服务,ipaddr为域名而非ip,修复前查看配置下发是失败的,修复后不报错,controller中有waring日志
Ⅴ. Special notes for reviews