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

Add Event Controller #2649

Merged
merged 1 commit into from
Aug 17, 2020
Merged

Add Event Controller #2649

merged 1 commit into from
Aug 17, 2020

Conversation

BeckerMax
Copy link
Contributor

@BeckerMax BeckerMax commented Aug 3, 2020 •
edited by rfranzke
Loading

How to categorize this PR?
/area usability
/kind enhancement
/priority normal

What this PR does / why we need it:
Add event controller

The goal of the event controller is to persist Shoot-related events longer for debugging reasons.

The controller deletes all non-Shoot events after the configured ttlNonShootEvents.
Shoot-related events are not touched by the controller and only eventually get deleted after the configured ttl in the kube-api-server configuration.

Considered alternatives to this controller were:

  1. export relevant events from etcd to another persistent store.
  2. extend the global ttl in the api-server for all events

While option 1. would be preferable it imposes dificulties as with the Control Plane Migration the shoot CP(with the events) can move between Seed worker nodes.

See also issue #2390

Also:

  • Added hibernation events and more fine-grained maintenance events for shoots.
  • Replaced some Shoot related events with logs to decrease the volume of shoot events written.

Which issue(s) this PR fixes:
Fixes #2390

Special notes for your reviewer:
Very open for any suggestion as well regarding code/tests style.

Release note:

A new controller for `Event`s related to `Shoot` objects has been added to the Gardener Controller Manager (disabled by default). It can be used to extend the live-span of events regarding shoot clusters (the live-span of all other events can be configured separately). Please find more information in [this document](https://github.com/gardener/gardener/blob/master/docs/concepts/controller-manager.md#event-controller).

@BeckerMax BeckerMax requested a review from a team as a code owner August 3, 2020 05:10
@gardener-robot
Copy link

@BeckerMax Label area/operations does not exist.

@gardener-robot gardener-robot added area/usability Usability related kind/enhancement Enhancement, improvement, extension priority/normal labels Aug 3, 2020
@CLAassistant
Copy link

CLAassistant commented Aug 3, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@BeckerMax BeckerMax changed the title Feature.eventlogs Add Event Controller to persist maintenance and hibernation operations longer Aug 3, 2020
@BeckerMax BeckerMax changed the title Add Event Controller to persist maintenance and hibernation operations longer Add Event Controller Aug 3, 2020
rfranzke
rfranzke requested changes Aug 3, 2020
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/assign
Thanks for the PR, pretty well readable!

...ener/controlplane/charts/runtime/templates/controller-manager/configmap-componentconfig.yaml Outdated Show resolved Hide resolved
...ener/controlplane/charts/runtime/templates/controller-manager/configmap-componentconfig.yaml Outdated Show resolved Hide resolved
pkg/controllermanager/apis/config/v1alpha1/defaults.go Show resolved Hide resolved
pkg/controllermanager/apis/config/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/controllermanager/apis/config/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/controllermanager/controller/shoot/shoot_hibernation_types.go Outdated Show resolved Hide resolved
pkg/controllermanager/controller/event/event_control.go Outdated Show resolved Hide resolved
pkg/controllermanager/controller/event/event_control.go Outdated Show resolved Hide resolved
pkg/controllermanager/controller/event/event_control.go Outdated Show resolved Hide resolved
pkg/controllermanager/controller/event/event_control.go Outdated Show resolved Hide resolved
@rfranzke
Copy link
Member

rfranzke commented Aug 3, 2020

Can you please sign the CLA and check make verify?

@BeckerMax BeckerMax mentioned this pull request Aug 3, 2020
Closed
@gardener-robot
Copy link

@BeckerMax You have mentioned internal references in the public. Please check.

@timebertt
Copy link
Member

/assign

rfranzke
rfranzke reviewed Aug 14, 2020
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

Thanks, I'm okay with the PR and don't have further suggestions. The only thing I'd kindly ask you to add is a little bit of documentation here: https://github.com/gardener/gardener/blob/master/docs/concepts/controller-manager.md#control-loops
You can introduce a new section Event controller and explain what it does, how to configure it, etc. (does not need to be too long/artificially long, but detailed enough so that an external user understands it).
Thank you very much, and sorry for not mentioning this earlier.

/needs documentation
/status author-action

@gardener-robot
Copy link

@BeckerMax The pull request was assigned to you under author-action. Please unassign yourself when you are done. Thank you.

@BeckerMax
Copy link
Contributor Author

@rfranzke I already had the documentation included. But now I also added the ConcurrentSyncs flag as it became a required value.

@BeckerMax
Copy link
Contributor Author

/unassign

rfranzke
rfranzke previously approved these changes Aug 15, 2020
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

docs/concepts/controller-manager.md Outdated Show resolved Hide resolved
Add event controller
aca4fd6 
The goal of the event controller is to persist Shoot-related events
longer for debugging reasons.

The controller deletes all non-Shoot events after the configured
`ttlNonShootEvents`.
Shoot-related events are not touched by the controller and only eventually get
deleted after the configured `ttl` in the kube-api-server configuration.

Considered alternatives to this controller were:
1. export relevant events from etcd to another persistent store.
2. extend the global `ttl` in the api-server for all events

While option 1. would be preferable it imposes dificulties as with the Control
Plane Migration the shoot CP(with the events) can move between Seed worker
nodes.

In addition this commit:
- Replaces some Shoot-related events with logs to reduce the load written to etcd
- Creates more fine grained events regarding shoot cluster maintenance and hibernation
@rfranzke rfranzke requested a review from timuthy August 17, 2020 07:42
timebertt
timebertt approved these changes Aug 17, 2020
View reviewed changes
Copy link
Member

@timebertt timebertt left a comment

Choose a reason for hiding this comment

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

Thanks for adapting all the feedback!
/lgtm
🚀

timuthy
timuthy approved these changes Aug 17, 2020
View reviewed changes
Copy link
Member

@timuthy timuthy left a comment

Choose a reason for hiding this comment

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

/lgtm

@rfranzke rfranzke merged commit 6039bea into gardener:master Aug 17, 2020
@BeckerMax BeckerMax deleted the feature.eventlogs branch October 1, 2020 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfranzke rfranzke rfranzke approved these changes

@timuthy timuthy timuthy approved these changes

@timebertt timebertt timebertt approved these changes

Assignees

@rfranzke rfranzke

@timebertt timebertt

Labels
area/usability Usability related kind/enhancement Enhancement, improvement, extension
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Event logs for maintenance and hibernation operations
10 participants
@BeckerMax @gardener-robot @CLAassistant @rfranzke @timebertt @danielfoehrKn @timuthy @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3