Skip to content
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.

Sign up for GitHub

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

Jump to bottom

dst: Update GetProfile's stream when pod associated to HostPort lookup changes #11334

Merged
merged 9 commits into from Sep 28, 2023
Merged

dst: Update GetProfile's stream when pod associated to HostPort lookup changes #11334

merged 9 commits into from Sep 28, 2023

Commits on Sep 27, 2023

  1. Complete fix for hostport staleness - pod watcher 

    Followup to #11328, based off of `alpeb/hostport-fixup-stopgap`.

Implements a new pod watcher, instantiated along the other ones in the
Destination server. It's generic enough to catch all pod events in the
cluster, so it's up to the subscribers to filter out the ones they're
interested in, and to set up any metrics.

In the Destination server's `subscribeToEndpointProfile` method, we
create a new `HostPortAdaptor` that is subscribed to the pod watcher,
and forwards the pod and protocol updates to the
`endpointProfileTranslator`. Handling of Server subscriptions are now
handled by this adaptor, which are recycled whenever the pod changes.

A new gauge metric `host_port_subscribers` has been created, tracking
the number of subscribers for a given HostIP+port combination.

## Other Changes

- Moved the `server.createAddress` method into a static function in
  `endpoints_watcher.go`, for better reusability.
- The "Return profile for host port pods" test introduced in #11328 was
  extended to track the ensuing events after a pod is deleted and then
  recreated (:taco: to @adleong for the test).
- Given that test consumes multiple events, we had to change the
  `profileStream` test helper to allow for the `GetProfile` call to
  block. Callers to `profileStream` now need to manually cancel the
  returned stream.
    @alpeb
    alpeb committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    aa1f7f8 View commit details
    Browse the repository at this point in the history

  2. @mateiidavid's feedback

    @alpeb
    alpeb committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    a6303b5 View commit details
    Browse the repository at this point in the history

  3. Move hostport adaptor logic into the endpoint profile translator

    @alpeb
    alpeb committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    352b518 View commit details
    Browse the repository at this point in the history

  4. Multiple refactorings 

    This change moves most of the logic into the PodWatcher, which now watches over pods and Servers. As suggested, ServerWatcher has been decomissioned.

The `CreateAddress()` function has been moved into a function of the PodWatcher, because now we're calling it on every every update given the pod associated to an ip:port might change and we need to regenerate the Address object. That function also takes care of capturing opaque protocol info from associated Servers, which is not new and had some logic that was duped in the now defunct ServerWatcher. `getAnnotatedOpaquePorts()` shared the same fate for similar reasons.

Other things to note about PodWatcher:
- It publishes a new pair of metrics `ip_port_subscribers` and `ip_port_updates` leveraging the framework in `prometheus.go`.
- The complexity in `updatePod()` is due to only send stream updates when there are changes in the pod's readiness, to avoid sending duped messages on every pod lifecycle event. Let me know if that logic isn't clear enough.

Finally, endpointProfileTranslator's `endpoint` (*pb.WeightedAddr) not being a static object anymore, its `Update()` function now receives an Address that allows it to rebuild the endpoint on the fly (and so `createEndpoint()` was converted into a method of endpointProfileTranslator.
    @alpeb
    alpeb committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    377aa8c View commit details
    Browse the repository at this point in the history

  5. Move Address/pod creation to podWatcher, consider all Server updates 

    Moved the Address/pod object creation logic into the podWatcher, on the `getOrNewPodPublisher()` method, called from `Subscribe()`. The latter receives `service`, `hostname` and `ip`, which can be empty depending on the case.

With this change also the auxiliary functions `getIndexedPod()` and `podReceivingTraffic()` were moved into the `watcher` package.

Also the Server updates handler now triggers updates for all podPublishers regardless of the Server selectors. The endpointProfileTranslator is now tracking the last message sent, to avoid sending dupe messages.

Other changes:
- Added fields `defaultOpaquePorts`, `k8sAPI` and `metadataAPI` into the podPublisher struct.
- Removed the fields `ip` and `port` from the endpointProfileTranslator, which are not used.
    @alpeb
    alpeb committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    d3a149d View commit details
    Browse the repository at this point in the history

  6. Apply suggestions from code review 

    Co-authored-by: Matei David <matei@buoyant.io>
    @alpeb @mateiidavid
    alpeb and mateiidavid committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    33f5683 View commit details
    Browse the repository at this point in the history

  7. Added missing lock

    @alpeb
    alpeb committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    5f0f7e9 View commit details
    Browse the repository at this point in the history

  8. SetServerProtocol() should also check for HostPort

    @alpeb
    alpeb committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    9c53067 View commit details
    Browse the repository at this point in the history

  9. Revert "Apply suggestions from code review" 

    This reverts commit ae0438f.
    @alpeb
    alpeb committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    9d932f8 View commit details
    Browse the repository at this point in the history