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

Reconcile all clusters on license change #2145

Merged
merged 2 commits into from
Nov 21, 2019

Conversation

pebrc
Copy link
Collaborator

@pebrc pebrc commented Nov 21, 2019

  • License changes now propagate almost immediately to all clusters.
  • e2e test has been adjusted (and shortened) as a result of that: no more mutation necessary to observe the license change
  • Trade-off: creating a reconcile request for every single cluster managed by ECK might turn out to be a problem in large installations, but my rationale was that controller-runtime resyncs anyway every 10 hours which is comparable in load generated.

Fixes #2131

License changes now propagate almost immediately to all clusters.
E2e test has been ajusted (and shortened) as a result of that.
@pebrc pebrc added >enhancement Enhancement of existing functionality v1.0.0 labels Nov 21, 2019
Copy link
Contributor

@david-kow david-kow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one nit.

// list all cluster licenses referencing the given enterprise license
matchLabels := license.NewLicenseByNameSelector(licenseName)
err := c.List(&list, matchLabels)
func reconcileRequestsForAllClusters(c k8s.Client) ([]reconcile.Request, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this name indicates (to me) that something will be reconciled here

Copy link
Collaborator Author

@pebrc pebrc Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's meant to be read as a noun not as a verb :-)

@pebrc pebrc merged commit 9891f4f into elastic:master Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality v1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enterprise license does not propagate to existing clusters
3 participants