-
Notifications
You must be signed in to change notification settings - Fork 231
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(plugin): Support customplugin (#301) #377
Conversation
@@ -19,6 +19,7 @@ package v1alpha2 | |||
import ( | |||
"bytes" | |||
"fmt" | |||
"github.com/fluent/fluent-operator/apis/fluentbit/v1alpha2/plugins/custom" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imports need to sort.
@@ -1,6 +1,7 @@ | |||
package v1alpha2 | |||
|
|||
import ( | |||
"github.com/fluent/fluent-operator/apis/fluentbit/v1alpha2/plugins/custom" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imports need to sort.
if len(tmp) < 2 { | ||
continue | ||
} | ||
if strings.EqualFold(tmp[0], "Name") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The key Name
also can be name
of NAME
.
return "NONE" | ||
} | ||
|
||
func (a *CustomPlugin) Params(_ plugins.SecretLoader) (*params.KVs, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why need to parser the config
? Why need to skip the name
field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A custom plugin is also a plugin that implements the Plugin interface. The plugin name is already resolved in the Name method, so there is no need to resolve the Name again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A more reasonable approach.
Add the name
filed just when the p.Name()
is not nil
https://github.com/fluent/fluent-operator/blob/master/apis/fluentbit/v1alpha2/clusterinput_types.go#L94
and make the customPlugin.Name()
return nil
@Gentleelephant We also need to update docs for plugins https://github.com/fluent/fluent-operator/tree/master/docs/plugins/fluentbit @wanjunlei How to generate these docs automatically? The index should be added manually as I can remember |
make docs-update? https://github.com/fluent/fluent-operator/blob/master/Makefile#L174 |
Yes |
Regarding the modification of the document, do I need to create a new pr? @benjaminhuo @wanjunlei |
No, In the same PR is ok |
@Gentleelephant You can add another doc in https://github.com/fluent/fluent-operator/tree/master/docs/best-practice to elaborate on how to use the custom plugin like docs in https://github.com/fluent/fluent-operator/blob/master/docs/best-practice/collect-systemd-logs.md. You can paste sample YAML like below in https://github.com/kubesphere-sigs/fluent-operator-walkthrough instead of kubectl apply a file: |
Signed-off-by: Gentleelephant <birdhk@kubesphere.io>
Signed-off-by: Gentleelephant birdhk@kubesphere.io
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #301
Does this PR introduced a user-facing change?
Additional documentation, usage docs, etc.: