destination: Remove support for IP Queries in Get API#6018
destination: Remove support for IP Queries in Get API#6018Pothulapati merged 12 commits intomainfrom
Get API#6018Conversation
Fixes #5246 This PR updates the destination to report an error when `Get` is called for IP Queries. As the issue mentions, The proxies are not using this API anymore and it helps to simplify and remove unecessary logic. This change is aimed for `stable-2.11`, and depends on linkerd/linkerd2-proxy#972 Signed-off-by: Tarun Pothulapati <[email protected]>
Signed-off-by: Tarun Pothulapati <[email protected]>
as `Get` no longer returns pod IP queries, We don't have to maintain the logic for subscriptions. Signed-off-by: Tarun Pothulapati <[email protected]>
kleimkuhler
left a comment
kleimkuhler
left a comment
There was a problem hiding this comment.
So the IP watcher is used for GetSvcID and GetPod now. However, both of those functions only use the k8sAPI field which we have access to in the server. We should change those from methods on IPWatcher to methods on Server.
NewIPWatcher has a side effect which adds the podIPIndex and hostIPIndex indexers to the k8sAPI that is passed in. Adding those indexers should probably move to Server's initialization. After doing that, I think we should be able to fully get rid of IPWatcher.
Signed-off-by: Tarun Pothulapati <[email protected]>
Signed-off-by: Tarun Pothulapati <[email protected]>
This commit moves the indexer creations into Server.go, so that there is a single place where all indexers are created instead of each watcher doing its own. This works because the same `k8s.API` is used across watchers and the server. With this change, The golang watcher tests are expected to fail as the informers are outside of the watcher logic that they test. Signed-off-by: Tarun Pothulapati <[email protected]>
… tests This commit updates the unit tests to call `initializeIndexers` to initialize indexers before running the watcher specific unit tests. The `initializeIndexers` had to be in `k8s.go` in `watcher` pkg as it had to be used across watcher and the destination pkg. Signed-off-by: Tarun Pothulapati <[email protected]>
Signed-off-by: Tarun Pothulapati <[email protected]>
Signed-off-by: Tarun Pothulapati <[email protected]>
kleimkuhler
left a comment
kleimkuhler
left a comment
There was a problem hiding this comment.
Overall this looks good. I think moving the watcher setup into a function makes sense so that it's more obvious that we are adding indexers as part of its initialization.
I was going to suggest making getSvcID and getPod as methods on Server, but since we don't use any other fields from the server I think they are okay as regular functions. That being said, it may be worth making them methods on k8s.API in the controller package. That way we can keep functions like this in a singular place.
Old:
svcID, err := getSvcID(s.k8sAPI, ip.String(), log)
New:
svcID, err := s.k8sAPI.getSvcID(ip.String(), log)
Signed-off-by: Tarun Pothulapati <[email protected]>
I'm not sure if it is the right way to do it for the following reasons:
How about we keep this logic in the same pkg but into a different file, so that |
kleimkuhler
left a comment
kleimkuhler
left a comment
There was a problem hiding this comment.
I'm okay with it is as is then. I don't think introducing a new file will necessarily make it easier to read or understand. It'd be nice if we had a way to track these functions that are based off having a k8sAPI—and making them methods does seem ideal—but if it brings in a lot of extra dependencies and all then I also don't think that's the right solution.
Signed-off-by: Tarun Pothulapati <[email protected]>
Signed-off-by: Tarun Pothulapati <[email protected]>
3 participants
Fixes #5246
This PR updates the destination to report an error when
Getis called for IP Queries. As the issue mentions, The proxies
are not using this API anymore and it helps to simplify and
remove unecessary logic.
This change is aimed for
stable-2.11, and depends onlinkerd/linkerd2-proxy#972
Signed-off-by: Tarun Pothulapati [email protected]