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.

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

Create shared package for watching Nodes in remote Clusters #2414

Closed
JoelSpeed opened this issue Feb 24, 2020 · 7 comments
Closed

Create shared package for watching Nodes in remote Clusters #2414

JoelSpeed opened this issue Feb 24, 2020 · 7 comments
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Milestone

Comments

@JoelSpeed
Copy link
Contributor

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
Copy link
Member

/milestone v0.3.4

@k8s-ci-robot k8s-ci-robot modified the milestones: v0.3.x, v0.3.4 Mar 31, 2020
@vincepri vincepri removed the kind/proposal Issues or PRs related to proposals. label Apr 27, 2020
@vincepri
Copy link
Member

/kind cleanup

@k8s-ci-robot k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Apr 27, 2020
@vincepri
Copy link
Member

/milestone v0.3.x

@k8s-ci-robot k8s-ci-robot modified the milestones: v0.3.4, v0.3.x Apr 27, 2020
@vincepri
Copy link
Member

Closed #2577 in favor of this issue

@vincepri
Copy link
Member

/milestone v0.3.6

@k8s-ci-robot k8s-ci-robot modified the milestones: v0.3.x, v0.3.6 May 14, 2020
@vincepri
Copy link
Member

/close

@k8s-ci-robot
Copy link
Contributor

@vincepri: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

No branches or pull requests

3 participants