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

Fix fluentdconfig and syslogngconfig deletion #1672

Merged
merged 9 commits into from
Feb 28, 2024
Merged

Fix fluentdconfig and syslogngconfig deletion #1672

merged 9 commits into from
Feb 28, 2024

Conversation

OverOrion
Copy link
Contributor

@OverOrion OverOrion commented Feb 15, 2024 •
edited by pepov
Loading

This PR aims to fix two problems:

  • deleting a fluentdconfig/syslogngconfig resource should also delete the child resources (solved by changing the ownerreferences)
  • deleting a logging resource should not be possible until a fluentdconfig/syslogngconfig resource exists. we don't delete the fluentdconfig/syslogngconfig forcefully because those are created by the user, but we can protect against this by using a finalizer. we send an event to the logging object for the user to be able to see other than looking at the operator logs

Fixes #1667.

Signed-off-by: Szilard Parrag [email protected]

pepov
pepov reviewed Feb 19, 2024
View reviewed changes
controllers/logging/logging_controller.go Outdated Show resolved Hide resolved
controllers/logging/logging_controller.go Outdated Show resolved Hide resolved
pepov
pepov reviewed Feb 19, 2024
View reviewed changes
controllers/logging/logging_controller.go Outdated Show resolved Hide resolved
controllers/logging/logging_controller.go Outdated Show resolved Hide resolved
pkg/resources/model/reconciler.go Outdated Show resolved Hide resolved
controllers/logging/logging_controller.go Outdated Show resolved Hide resolved
@OverOrion OverOrion force-pushed the fluentdconfig-fix branch 2 times, most recently from 22a887f to 34e978b Compare February 19, 2024 14:01
@OverOrion
Copy link
Contributor Author

I have reworked the PR according to pepov's notes and using kubebuilder's docs: https://book.kubebuilder.io/reference/using-finalizers.

I also added a secondary fix which occured when logging switched from fluentdConfig to inline spec.

orymate
orymate approved these changes Feb 23, 2024
View reviewed changes
controllers/logging/logging_controller.go Outdated Show resolved Hide resolved
controllers/logging/logging_controller.go Outdated Show resolved Hide resolved
controllers/logging/logging_controller.go Outdated Show resolved Hide resolved
@pepov pepov added the enhancement New feature or request label Feb 28, 2024
@pepov pepov added bugfix and removed enhancement New feature or request labels Feb 28, 2024
OverOrion and others added 8 commits February 28, 2024 14:58
@OverOrion @pepov
fix(fluentdconfig): remove fluentd pod once fluentdconfig gets removed
92c0ad3 
Signed-off-by: Szilard Parrag <[email protected]>
@OverOrion @pepov
fix(fluentdconfig): remove obsolete fluentdconfig references at the s…
6045851 
…tart of reconcilation

This fixes a crash if a FluentdConfig was used, got deleted
and logging switched to inline fluentd spec.

Signed-off-by: Szilard Parrag <[email protected]>
@OverOrion @pepov
fix(syslogngconfig): remove syslogng pod once syslogngonfig gets removed
d44a32b 
Signed-off-by: Szilard Parrag <[email protected]>
@OverOrion @pepov
fix(logging controller): propagate errors, do not update status when …
5a2c595 
…an object is marked for deletion

Signed-off-by: Szilard Parrag <[email protected]>
@pepov
chore: reword log message
71c70db 
Signed-off-by: Peter Wilcsinszky <[email protected]>
@pepov
chore: fix variable names
0bd2da0 
Signed-off-by: Peter Wilcsinszky <[email protected]>
@pepov
refactor: simplify finalizer logic by testing it from all use cases
f8b3566 
Signed-off-by: Peter Wilcsinszky <[email protected]>
@pepov
feat: event recorder for the logging finalizer
f27aafb 
Signed-off-by: Peter Wilcsinszky <[email protected]>
@pepov pepov changed the base branch from master to release-4.5 February 28, 2024 13:59
@pepov
trigger ci
0f49d06 
Signed-off-by: Peter Wilcsinszky <[email protected]>
@pepov pepov changed the title Fix fluentdconfig deletion not deleting pods Fix fluentdconfig and syslogngconfig deletion Feb 28, 2024
@pepov pepov merged commit 7c2e729 into release-4.5 Feb 28, 2024
17 checks passed
@pepov pepov deleted the fluentdconfig-fix branch February 28, 2024 20:59
OverOrion added a commit that referenced this pull request Mar 4, 2024
@OverOrion @pepov
Fix fluentdconfig and syslogngconfig deletion (#1672)
130c63a 
Signed-off-by: Szilard Parrag <[email protected]>
Signed-off-by: Peter Wilcsinszky <[email protected]>
Co-authored-by: Peter Wilcsinszky <[email protected]>
@pepov pepov mentioned this pull request Mar 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orymate orymate orymate approved these changes

@pepov pepov pepov approved these changes

@tarokkk tarokkk tarokkk approved these changes

@kristofgyuracz kristofgyuracz kristofgyuracz approved these changes

Assignees
No one assigned
Labels
bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fluentdconfig object deleted, pod is still running
5 participants
@OverOrion @orymate @pepov @tarokkk @kristofgyuracz