feat(fluentbit): Support network relevant settings for s3 output#1955
feat(fluentbit): Support network relevant settings for s3 output#1955smallc2009 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Fluent Bit networking/connection-management options (timeouts, DNS preferences, keepalive, TCP keepalive, source address, proxy env handling, worker connections) to the S3 output plugin, exposing them via the CRDs and Helm charts and adding test coverage.
Changes:
- Add ~15 new networking fields to the
S3struct and emit them asnet.*params inS3.Params. - Regenerate the affected CRD YAMLs, Helm chart templates, deepcopy, and API docs.
- Extend
TestOutput_S3_Paramsto cover the new fields.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apis/fluentbit/v1alpha2/plugins/output/s3_types.go | Adds new networking fields and maps them to net.* params |
| apis/fluentbit/v1alpha2/plugins/output/s3_types_test.go | Tests new fields produce the expected net.* KVs |
| apis/fluentbit/v1alpha2/plugins/output/zz_generated.deepcopy.go | Generated deepcopy for new S3 pointer fields |
| config/crd/bases/fluentbit.fluent.io_outputs.yaml | Regenerated CRD with new S3 fields |
| config/crd/bases/fluentbit.fluent.io_clusteroutputs.yaml | Regenerated CRD with new S3 fields |
| charts/fluent-operator/crds/fluentbit.fluent.io_outputs.yaml | Helm CRD chart updated |
| charts/fluent-operator/crds/fluentbit.fluent.io_clusteroutputs.yaml | Helm CRD chart updated |
| charts/fluent-operator-fluent-bit-crds/templates/fluentbit.fluent.io_outputs.yaml | Helm CRD chart updated |
| charts/fluent-operator-fluent-bit-crds/templates/fluentbit.fluent.io_clusteroutputs.yaml | Helm CRD chart updated |
| manifests/setup/setup.yaml | Generated manifest updated |
| manifests/setup/fluent-operator-crd.yaml | Generated manifest updated |
| docs/plugins/fluentbit/output/s3.md | Doc lists new fields |
Files not reviewed (1)
- apis/fluentbit/v1alpha2/plugins/output/zz_generated.deepcopy.go: Language not supported
Comments suppressed due to low confidence (1)
apis/fluentbit/v1alpha2/plugins/output/s3_types.go:150
Keepaliveis documented as a boolean on/off switch for connection keepalive support, but it is being emitted asnet.tcp_keepalive. Per Fluent Bit's networking documentation (and the existingplugins.Networking.Paramsimplementation atapis/fluentbit/v1alpha2/plugins/net_types.go:55-56), the on/off keepalive toggle is thenet.keepalivesetting.net.tcp_keepaliveis a different (TCP-level) option. As written, enablingkeepalive: "on"on S3 will set the wrong Fluent Bit parameter.
plugins.InsertKVField(kvs, "net.tcp_keepalive", o.Keepalive)
|
Hi @smallc2009 can you please help resolve the conflicts? Thank you. |
Signed-off-by: Anson <anson.liu@live.com>
cce8ea9 to
ac49398
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 9 comments.
Files not reviewed (1)
- apis/fluentbit/v1alpha2/plugins/output/zz_generated.deepcopy.go: Language not supported
Comments suppressed due to low confidence (2)
manifests/setup/setup.yaml:1
- Fix grammar/casing in descriptions: change "this include" to "this includes", and "IPV6" to "IPv6" for consistency and correctness in user-facing CRD docs.
manifests/setup/setup.yaml:1 - Fix grammar/casing in descriptions: change "this include" to "this includes", and "IPV6" to "IPv6" for consistency and correctness in user-facing CRD docs.
| *plugins.TLS `json:"tls,omitempty"` | ||
| Workers *int32 `json:"Workers,omitempty"` | ||
| // Set maximum time expressed in seconds to wait for a TCP connection to be established, this include the TLS handshake time. | ||
| ConnectTimeout *int32 `json:"ConnectTimeout,omitempty"` |
There was a problem hiding this comment.
is it possible we can make such configurations shareable among different plugins, just like *plugins.TLS
What this PR does / why we need it:
This pull request adds a number of advanced network and connection management options to the S3 output plugin for Fluent Bit, enhancing its configurability and robustness.
Enhancements to S3 Plugin Networking and Connection Handling:
New S3 Plugin Fields and Parameter Mapping:
S3struct ins3_types.gofor fine-grained network and connection control (e.g.,ConnectTimeout,ConnectTimeoutLogError,DNSMode,DNSPreferIPv4,DNSPreferIPv6,IoTimeout,KeepaliveMaxRecycle,MaxWorkerConnections,ProxyEnvIgnore,SourceAddress,Keepalive,TCPKeepaliveInterval,TCPKeepaliveProbes,TCPKeepaliveTime,KeepaliveIdleTimeout). These fields are now mapped to their corresponding Fluent Bit configuration parameters in theParamsmethod.CRD and Helm Chart Updates:
Test Coverage:
s3_types_test.go) to cover all new fields, ensuring correct parameter insertion and behavior.Which issue(s) this PR fixes:
Fixes #1930
Does this PR introduced a user-facing change?
Additional documentation, usage docs, etc.: