feat(fluentbit): Support network relevant settings for s3 output#1955
Open
smallc2009 wants to merge 2 commits into
Open
feat(fluentbit): Support network relevant settings for s3 output#1955smallc2009 wants to merge 2 commits into
smallc2009 wants to merge 2 commits into
Conversation
Signed-off-by: Anson <anson.liu@live.com>
Contributor
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)
Comment on lines
+69
to
+101
| // 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"` | ||
| // On connection timeout, specify if it should log an error. When disabled, the timeout is logged as a debug message. | ||
| ConnectTimeoutLogError *bool `json:"connectTimeoutLogError,omitempty"` | ||
| // Select the primary DNS connection type (TCP or UDP). | ||
| // +kubebuilder:validation:Enum:="TCP";"UDP" | ||
| DNSMode *string `json:"DNSMode,omitempty"` | ||
| // Prioritize IPv4 DNS results when trying to establish a connection. | ||
| DNSPreferIPv4 *bool `json:"DNSPreferIPv4,omitempty"` | ||
| // Prioritize IPV6 DNS results when trying to establish a connection. | ||
| DNSPreferIPv6 *bool `json:"DNSPreferIPv6,omitempty"` | ||
| // Set maximum time a connection can stay idle while assigned. | ||
| IoTimeout *int32 `json:"IoTimeout,omitempty"` | ||
| // Set maximum number of times a keepalive connection can be used before it is retired. | ||
| KeepaliveMaxRecycle *int32 `json:"keepaliveMaxRecycle,omitempty"` | ||
| // Set maximum number of TCP connections that can be established per worker. | ||
| MaxWorkerConnections *int32 `json:"maxWorkerConnections,omitempty"` | ||
| // Ignore the environment variables HTTP_PROXY, HTTPS_PROXY and NO_PROXY when set. | ||
| ProxyEnvIgnore *bool `json:"proxyEnvIgnore,omitempty"` | ||
| // Specify network address to bind for data traffic. | ||
| SourceAddress *string `json:"sourceAddress,omitempty"` | ||
| // Enable or disable connection keepalive support. Accepts a boolean value: on / off. | ||
| // +kubebuilder:validation:Enum:="on";"off" | ||
| Keepalive *string `json:"keepalive,omitempty"` | ||
| // Interval between TCP keepalive probes when no response is received on a keepidle probe. | ||
| TCPKeepaliveInterval *int32 `json:"tcpKeepaliveInterval,omitempty"` | ||
| // Number of unacknowledged probes to consider a connection dead. | ||
| TCPKeepaliveProbes *int32 `json:"tcpKeepaliveProbes,omitempty"` | ||
| // Interval between the last data packet sent and the first TCP keepalive probe. | ||
| TCPKeepaliveTime *int32 `json:"tcpKeepaliveTime,omitempty"` | ||
| // Set maximum time expressed in seconds for an idle keepalive connection. | ||
| KeepaliveIdleTimeout *int32 `json:"keepaliveIdleTimeout,omitempty"` | ||
| *plugins.TLS `json:"tls,omitempty"` |
joshuabaird
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.: