kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.5k stars 1.3k forks source link

Create shared package for watching Nodes in remote Clusters #2414

Closed JoelSpeed closed 4 years ago

JoelSpeed commented 4 years ago

Note: this is only applicable once #2250 is merged

⚠️ Cluster API maintainers can ask to turn an issue-proposal into a CAEP when necessary, this is to be expected for large changes that impact multiple components, breaking changes, or new large features.

Goals

  1. Make it easy for controller developers within the CAPI space to react to events on Node objects in target clusters
  2. Provide a common way of watching Node objects within remote clusters

Non-Goals/Future Work

  1. Implement remote watching of Nodes in all controllers

User Story

As a developer I would like the controllers I am writing to be able to react to events on Nodes in remote clusters so that my controller can reconcile the status of objects or take action based on the state of the Nodes

Detailed Description

The Machine Health Checking controller implemented primarily in #2250 needed to watch Nodes in remote clusters to be able to react as quickly as possible to Nodes which might be going unhealthy.

To achieve this, a method was implemented that keeps a map of clusters to informers that allows the controller to add event handlers for events coming from nodes in remote clusters.

This could be moved into the external package and made more reusable by making it a method on a new struct which keeps the map, lock and controller as fields

type RemoteClusterWatcher struct {
  controller               controller.Controller
  clusterNodeInformers.    map[types.NamespacedName]cache.Informer
  clusterNodeInformersLock *sync.Mutex
}

And changing the signature of the WatchClusterNodes method to allow the event handler to be passed.

It is currently assumed that each controller would only want to register one event handler per cluster, I'm not sure how true that is and as such, we should try to add the ability to check if event handler functions have been registered and add more if required.

/kind proposal

vincepri commented 4 years ago

/milestone v0.3.4

vincepri commented 4 years ago

/kind cleanup

vincepri commented 4 years ago

/milestone v0.3.x

vincepri commented 4 years ago

Closed https://github.com/kubernetes-sigs/cluster-api/issues/2577 in favor of this issue

vincepri commented 4 years ago

/milestone v0.3.6

vincepri commented 4 years ago

/close

k8s-ci-robot commented 4 years ago

@vincepri: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/2414#issuecomment-628814461): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.