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 DefaultFilterSelector and DefaultOutputSelector to fluentd #804

Merged
merged 6 commits into from
Jun 27, 2023

Conversation

Jakob3xD
Copy link
Contributor

What this PR does / why we need it:

  • Update comments in codes
  • Fix missing join for PatchAndFilter function errors returning []string
  • Add DefaultFilterSelector and DefaultOutputSelector to fluentdSpec

Which issue(s) this PR fixes:

Fixes #800

Does this PR introduced a user-facing change?

Feat: Add DefaultFilterSelector and DefaultOutputSelector to fluentd

Additional documentation, usage docs, etc.:


@Jakob3xD Jakob3xD force-pushed the jh-default-route branch 2 times, most recently from ffaa3fc to ba73087 Compare June 21, 2023 09:58
@Jakob3xD Jakob3xD changed the title [WIP] Feat: Add DefaultFilterSelector and DefaultOutputSelector to fluentd Feat: Add DefaultFilterSelector and DefaultOutputSelector to fluentd Jun 21, 2023
@Jakob3xD Jakob3xD marked this pull request as ready for review June 21, 2023 11:12
@@ -36,6 +36,10 @@ const (
type FluentdSpec struct {
// Fluentd global inputs.
GlobalInputs []input.Input `json:"globalInputs,omitempty"`
// Select cluster filter plugins used to filter for the default cluster output
DefaultFilterSelector *metav1.LabelSelector `json:"defaultFilterSelector,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

What is the processing logic for this? I don't see anything in the code that handles that field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is handled in the controllers/fluentdconfig_controller.go file starting from line 140.

Copy link
Member

Choose a reason for hiding this comment

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

This is handled in the controllers/fluentdconfig_controller.go file starting from line 140.

@Jakob3xD I cannot see how DefaultFilterSelector/DefaultOutputSelector and default_route is handled in controllers/fluentdconfig_controller.go

Comment on lines +140 to +162
filters, outputs, err := r.ListNamespacedLevelResources(ctx, fd.Namespace, fd.Spec.DefaultFilterSelector, fd.Spec.DefaultOutputSelector)
if err != nil {
r.Log.Info("List namespace level resources failed", "config", "default", "err", err.Error())
return ctrl.Result{}, err
}
if len(filters) > 0 || len(outputs) > 0 {
// Combine the namespaced filter/output pluginstores in this fluentd config
cfgResouces, errs := gpr.PatchAndFilterNamespacedLevelResources(sl, fmt.Sprintf("%s-%s-%s", fd.Kind, fd.Namespace, fd.Name), filters, outputs)
if len(errs) > 0 {
r.Log.Info("Patch and filter namespace level resources failed", "config", "default", "err", strings.Join(errs, ","))
return ctrl.Result{}, fmt.Errorf(strings.Join(errs, ","))
}

// WithCfgResources will collect all plugins to generate main config
err = gpr.WithCfgResources("@default", cfgResouces)
if err != nil {
r.Log.Info("Combine resources failed", "config", "default", "err", err.Error())
return ctrl.Result{}, err
}
// Add the default route to the main routing plugin
gpr.MainRouterPlugins.InsertPairs("default_route", "@default")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@benjaminhuo @wenchajun this code block handles the the newly defined default selectors.

Copy link
Member

Choose a reason for hiding this comment

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

Got you. Sorry, my bad it's folded by default.
Thanks anyway for this great enhancement! @Jakob3xD

@benjaminhuo benjaminhuo merged commit 251932e into fluent:master Jun 27, 2023
@Jakob3xD Jakob3xD deleted the jh-default-route branch June 27, 2023 07:39
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.

[Feature] Set default route via default_route with label_router
3 participants