handle hostname type waypoint in workload#1021
Conversation
d0c33bf to
6d6f71e
Compare
Codecov ReportAttention: Patch coverage is
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
/retest |
|
@hzxuzhonghu PTAL |
| return nil, nil | ||
| } | ||
|
|
||
| type associatedObjects struct { |
There was a problem hiding this comment.
| type associatedObjects struct { | |
| type waypointAssociatedObjects struct { |
| } | ||
|
|
||
| func (w *associatedObjects) isResolved() bool { | ||
| return w.address != nil |
There was a problem hiding this comment.
donot you need to protect address?
| // AddOrUpdateWorkload add or update workload in this cache, return true if the | ||
| // workload's waypoint doesn't need to be resolved or resolved successfully. | ||
| AddOrUpateWorkload(workload *workloadapi.Workload) bool | ||
| DeleteWorkload(uid string) |
There was a problem hiding this comment.
And they are duplicate with workload cache
There was a problem hiding this comment.
Yes, but it's a tradeoff and duplicating a little code now maybe better than making further abstractions.
| // Anyway, add svc to the association list. | ||
| w.waypointAssociatedObjects[waypointResourceName].addService(resourceName, svc) | ||
|
|
||
| return ret |
There was a problem hiding this comment.
Could you add ut coverage for this branch
| } | ||
|
|
||
| log.Debugf("Refreshing services associated with waypoint %s", resourceName) | ||
| return associated.update(address) |
There was a problem hiding this comment.
Also add ut coverage for this
|
@YaoZengzeng please add some ut, then we can merge |
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
Signed-off-by: YaoZengzeng <yaozengzeng@huawei.com>
e5b1dda to
b67b286
Compare
config/kmesh_marcos_def.h
Outdated
| * used to obtain the original destination address. | ||
| */ | ||
| #define MDA_NAT_ACCEL 1 | ||
| #define MDA_NAT_ACCEL 0 |
|
|
||
| func (w *waypointCache) AddOrUpdateService(svc *workloadapi.Service) bool { | ||
| w.mutex.Lock() | ||
| defer w.mutex.Unlock() |
There was a problem hiding this comment.
Do you need to lock the whole function?
There was a problem hiding this comment.
It's OK, we should make it easy at first. If we find this is a performance bottleneck later, we can further optimize it and make more precise control.
|
@hzxuzhonghu updated |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hzxuzhonghu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Starting from istio 1.24, waypoint supports the address type of hostname and Kmesh need to support it.
Supplement to PR #995
Which issue(s) this PR fixes:
Fixes part of #984
Special notes for your reviewer:
Does this PR introduce a user-facing change?: