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

feat: add scheduler support for fluentbit collector #776

Merged
merged 1 commit into from Jun 6, 2023

Conversation

ksdpmx
Copy link
Collaborator

@ksdpmx ksdpmx commented Jun 6, 2023

What this PR does / why we need it:

allow customers to specify custom scheduler for fluentbit collector as requested from this comment. along with minor typo fix.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduced a user-facing change?

allow customers to specify custom scheduler for fluentbit collector

Additional documentation, usage docs, etc.:


Signed-off-by: Jason Zhang <ksdpmx@gmail.com>
@ksdpmx
Copy link
Collaborator Author

ksdpmx commented Jun 6, 2023

2023-06-06T03:10:30Z ERROR Reconciler error {"controller": "collector", "controllerGroup": "fluentbit.fluent.io", "controllerKind": "Collector", "Collector": {"name":"fluent-bit-collector","namespace":"fluent"}, "namespace": "fluent", "name": "fluent-bit-collector", "reconcileID": "b0128934-0ce5-4a21-9105-aa7df920cddd", "error": "error when handling finalizer: resource name may not be empty"}

btw i don't really agree with this review.

delete it directly.

resources haven't been generated before calling Delete() which leads to the remaining finalizer for collector.

Why does not delete the clusterrole and clusterrolebinding?

for this review, does it make sense to add an owner reference in the managed clusterrole and clusterrolebinding for gc controller, given this controller is not supposed to have permission to delete clusterrole/clusterrolebinding?

@benjaminhuo
Copy link
Member

2023-06-06T03:10:30Z ERROR Reconciler error {"controller": "collector", "controllerGroup": "fluentbit.fluent.io", "controllerKind": "Collector", "Collector": {"name":"fluent-bit-collector","namespace":"fluent"}, "namespace": "fluent", "name": "fluent-bit-collector", "reconcileID": "b0128934-0ce5-4a21-9105-aa7df920cddd", "error": "error when handling finalizer: resource name may not be empty"}

btw i don't really agree with this review.

delete it directly.

resources haven't been generated before calling Delete() which leads to the remaining finalizer for collector.

Why does not delete the clusterrole and clusterrolebinding?

for this review, does it make sense to add an owner reference in the managed clusterrole and clusterrolebinding for gc controller, given this controller is not supposed to have permission to delete clusterrole/clusterrolebinding?

@wanjunlei @wenchajun would you take a look at @ksdpmx‘s opinion?

@benjaminhuo benjaminhuo merged commit 1547896 into fluent:master Jun 6, 2023
6 checks passed
@benjaminhuo
Copy link
Member

2023-06-06T03:10:30Z ERROR Reconciler error {"controller": "collector", "controllerGroup": "fluentbit.fluent.io", "controllerKind": "Collector", "Collector": {"name":"fluent-bit-collector","namespace":"fluent"}, "namespace": "fluent", "name": "fluent-bit-collector", "reconcileID": "b0128934-0ce5-4a21-9105-aa7df920cddd", "error": "error when handling finalizer: resource name may not be empty"}

btw i don't really agree with this review.

delete it directly.

resources haven't been generated before calling Delete() which leads to the remaining finalizer for collector.

Why does not delete the clusterrole and clusterrolebinding?

for this review, does it make sense to add an owner reference in the managed clusterrole and clusterrolebinding for gc controller, given this controller is not supposed to have permission to delete clusterrole/clusterrolebinding?

You're welcome to create a PR to fix whatever you think it's a problem and @wanjunlei @wenchajun will review that

@ksdpmx ksdpmx deleted the scheduler_collector branch June 6, 2023 03:32
@ksdpmx
Copy link
Collaborator Author

ksdpmx commented Jun 6, 2023

does it make sense to add an owner reference in the managed clusterrole and clusterrolebinding for gc controller, given this controller is not supposed to have permission to delete clusterrole/clusterrolebinding?

fluentbit collector is a namespace-scoped resource which cannot own other cluster-scoped resources.

Cluster-scoped dependents can only specify cluster-scoped owners. In v1.20+, if a cluster-scoped dependent specifies a namespaced kind as an owner, it is treated as having an unresolvable owner reference, and is not able to be garbage collected.

https://kubernetes.io/docs/concepts/architecture/garbage-collection/#owners-dependents

@benjaminhuo
Copy link
Member

benjaminhuo commented Jul 19, 2023

@ksdpmx have added you as the fluent operator maintainer, thanks for your contribution! @wanjunlei @wenchajun @patrick-stephens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants