Add support for profile lookups by IP address#4727
Conversation
Signed-off-by: Kevin Leimkuhler <[email protected]>
Signed-off-by: Kevin Leimkuhler <[email protected]>
Signed-off-by: Kevin Leimkuhler <[email protected]>
Signed-off-by: Kevin Leimkuhler <[email protected]>
Signed-off-by: Kevin Leimkuhler <[email protected]>
Signed-off-by: Kevin Leimkuhler <[email protected]>
Signed-off-by: Kevin Leimkuhler <[email protected]>
zaharidichev
left a comment
There was a problem hiding this comment.
Looks good. Also tested it with the dst client.
controller/api/destination/server.go
Outdated
| log.Warnf("Failed to subscribe to profile %s: %s", path, err) | ||
| return err | ||
| } | ||
| defer s.profiles.Unsubscribe(nil, primary) |
There was a problem hiding this comment.
I think the error that Unsubsribe can sometimes return will be muted when calling it via defer. Maybe better to leave the function without a return value and just log on error.
There was a problem hiding this comment.
Yes you are correct here that errors that occur in Unsubscribe will fail silently.
I tested this by inserting return fmt.Errorf("this is an error") in Unsubscribe, and then subscribed to and quit a profile stream.
I expected to see this error in the server, but nothing occurred.
This occurs in the other places that we unsubscribe in this file, so I lean towards making this change separately.
Signed-off-by: Kevin Leimkuhler <[email protected]>
…le-ip Signed-off-by: Kevin Leimkuhler <[email protected]>
Signed-off-by: Kevin Leimkuhler <[email protected]>
Remove additional changes from a previous simplification Signed-off-by: Kevin Leimkuhler <[email protected]>
| if err != nil { | ||
| log.Debugf("Invalid service %s", dest.GetPath()) | ||
| return status.Errorf(codes.InvalidArgument, "invalid service: %s", err) | ||
| return nil |
There was a problem hiding this comment.
So, Thinking a bit more. I do think we're going to have to watch for updates. Just because a service isn't in our index doesn't mean it's not about to show up there. That doesn't have to be a hard blocker -- we can merge this and be incremental. But I think we should support the service being known by the app before it is known by the controller...
This is also jogging some ideas for things we may want to do in a followup re: supporting returning all profiles for a pod:ip target.
Signed-off-by: Kevin Leimkuhler <[email protected]>
adleong
left a comment
There was a problem hiding this comment.
Cool, I think we can ship this as-is with a TODO or a follow up issue to make the mapping from ip to service dynamic.
#4743 is the follow-up issue for this |
## Motivation Closes linkerd#3916 This adds the ability to get profiles for services by IP address. ### Change in behavior When the destination server receives a `GetProfile` request with an IP address, it now tries to map that IP address to a service. If the IP address maps to an existing service, then the destination server returns the profile stream subscribes for updates to the _service_--this is the existing behavior. If the IP changes to a new service, the stream will still send updates for the first service the IP address corresponded to since that is what it is subscribed to. If the IP address does not map to an existing service, then the destination server returns the profile stream but does not subscribe for updates. The stream will receive one update, the default profile. ### Solution This change uses the `IPWatcher` within the destination server to check for what services an IP address correspond to. By adding a new method `GetSvc` to `IPWatcher`, the server now calls this method if `GetProfile` receives a request with an IP address. ## Testing Install linkerd on a cluster and get the cluster IP of any service: ```bash ❯ kubectl get -n linkerd svc/linkerd-tap -o wide NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE SELECTOR linkerd-tap ClusterIP 10.104.57.90 <none> 8088/TCP,443/TCP 16h linkerd.io/control-plane-component=tap ``` Run the destination server: ```bash ❯ go run controller/cmd/main.go destination -kubeconfig ~/.kube/config ``` Get the profile for the tap service by IP address: ```bash ❯ go run controller/script/destination-client/main.go -method getProfile -path 10.104.57.90:8088 INFO[0000] retry_budget:{retry_ratio:0.2 min_retries_per_second:10 ttl:{seconds:10}} INFO[0000] ``` Get the profile for an IP address that does not correspond to a service: ```bash ❯ go run controller/script/destination-client/main.go -method getProfile -path 10.256.0.1:8088 INFO[0000] retry_budget:{retry_ratio:0.2 min_retries_per_second:10 ttl:{seconds:10}} INFO[0000] ``` You can add and remove settings for the service profile for tap and get updates. Signed-off-by: Kevin Leimkuhler <[email protected]> Signed-off-by: Eric Solomon <[email protected]>
The proxy performs endpoint discovery for unnamed services, but not service profiles. The destination controller and proxy have been updated to support lookups for unnamed services in #4727 and linkerd/linkerd2-proxy#626, respectively. This change modifies the injection template so that the `proxy.destinationGetNetworks` configuration enables profile discovery for all networks on which endpoint discovery is permitted.
…#4960) The proxy performs endpoint discovery for unnamed services, but not service profiles. The destination controller and proxy have been updated to support lookups for unnamed services in #4727 and linkerd/linkerd2-proxy#626, respectively. This change modifies the injection template so that the `proxy.destinationGetNetworks` configuration enables profile discovery for all networks on which endpoint discovery is permitted.
Motivation
Closes #3916
This adds the ability to get profiles for services by IP address.
Change in behavior
When the destination server receives a
GetProfilerequest with an IP address,it now tries to map that IP address to a service.
If the IP address maps to an existing service, then the destination server
returns the profile stream subscribes for updates to the service--this is the
existing behavior. If the IP changes to a new service, the stream will still
send updates for the first service the IP address corresponded to since that is
what it is subscribed to.
If the IP address does not map to an existing service, then the destination
server returns the profile stream but does not subscribe for updates. The stream
will receive one update, the default profile.
Solution
This change uses the
IPWatcherwithin the destination server to check for whatservices an IP address correspond to. By adding a new method
GetSvctoIPWatcher, the server now calls this method ifGetProfilereceives a requestwith an IP address.
Testing
Install linkerd on a cluster and get the cluster IP of any service:
Run the destination server:
❯ go run controller/cmd/main.go destination -kubeconfig ~/.kube/configGet the profile for the tap service by IP address:
❯ go run controller/script/destination-client/main.go -method getProfile -path 10.104.57.90:8088 INFO[0000] retry_budget:{retry_ratio:0.2 min_retries_per_second:10 ttl:{seconds:10}} INFO[0000]Get the profile for an IP address that does not correspond to a service:
❯ go run controller/script/destination-client/main.go -method getProfile -path 10.256.0.1:8088 INFO[0000] retry_budget:{retry_ratio:0.2 min_retries_per_second:10 ttl:{seconds:10}} INFO[0000]You can add and remove settings for the service profile for tap and get updates.
Signed-off-by: Kevin Leimkuhler [email protected]