Skip to content

fix(fluentd): sort plugin @id numerically to preserve CR definition order #1927

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.

Sign up for GitHub

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

Open
sugaf1204 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(fluentd): sort plugin @id numerically to preserve CR definition order #1927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion apis/fluentd/v1alpha1/plugins/params/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"sort"
"strconv"
"strings"

"github.com/fluent/fluent-operator/v3/pkg/utils"
)
Expand Down Expand Up @@ -253,8 +254,36 @@ func (a PluginStoreByNameById) Less(i, j int) bool {
if a[i].GetTag() != "**" && a[j].GetTag() == "**" {
return true
}
return a[i].GetId() < a[j].GetId()
return lessId(a[i].GetId(), a[j].GetId())
} else {
return a[i].Name < a[j].Name
}
}

// lessId orders @id strings so that numeric suffixes compare numerically once
// both ids share a "<prefix>-<index>" shape. A plain lexicographic compare
// would place "<p>-10" before "<p>-2" because '1' < '2'. Falls back to
// lexicographic order when either side lacks a numeric trailing segment or
// when the parsed prefixes differ.
func lessId(id1, id2 string) bool {
p1, n1, ok1 := splitIdIndex(id1)
p2, n2, ok2 := splitIdIndex(id2)
if ok1 && ok2 && p1 == p2 {
return n1 < n2
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

When ok1 && ok2 and p1 == p2, lessId returns n1 < n2 without a tie-breaker for n1 == n2. If two ids differ only in formatting (e.g., leading zeros like "...-2" vs "...-02"), Less will return false in both directions, letting sort.Sort reorder them non-deterministically. Add a deterministic tie-break (for example, fall back to full id1 < id2 when n1 == n2).

Suggested change
return n1 < n2
if n1 != n2 {
return n1 < n2
}
return id1 < id2

Copilot uses AI. Check for mistakes.
}
return id1 < id2
}

// splitIdIndex splits an @id at the last '-'. It returns the prefix, the
// parsed numeric index, and true when the trailing segment is an integer.
func splitIdIndex(id string) (string, int, bool) {
idx := strings.LastIndex(id, "-")
if idx < 0 {
return "", 0, false
}
n, err := strconv.Atoi(id[idx+1:])
if err != nil {
return "", 0, false
}
return id[:idx], n, true
}
102 changes: 102 additions & 0 deletions apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-filter-order-by-index.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<source>
@type forward
bind 0.0.0.0
port 24224
</source>
<match **>
@id main
@type label_router
<route>
@label @a2170d34e9940ec56d328100e375c43e
<match>
namespaces default,kube-system
</match>
</route>
</match>
<label @a2170d34e9940ec56d328100e375c43e>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-0
@type record_transformer
<record>
k00 v00
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-1
@type record_transformer
<record>
k01 v01
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-2
@type record_transformer
<record>
k02 v02
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-3
@type record_transformer
<record>
k03 v03
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-4
@type record_transformer
<record>
k04 v04
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-5
@type record_transformer
<record>
k05 v05
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-6
@type record_transformer
<record>
k06 v06
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-7
@type record_transformer
<record>
k07 v07
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-8
@type record_transformer
<record>
k08 v08
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-9
@type record_transformer
<record>
k09 v09
</record>
</filter>
<filter **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-10
@type record_transformer
<record>
k10 v10
</record>
</filter>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-es-0
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log
port 9200
</match>
</label>
91 changes: 91 additions & 0 deletions apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-input-order-by-index.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<source>
@type forward
bind 0.0.0.0
port 24224
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-0
@type sample
rate 1
tag input-order-00
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-1
@type sample
rate 2
tag input-order-01
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-2
@type sample
rate 3
tag input-order-02
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-3
@type sample
rate 4
tag input-order-03
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-4
@type sample
rate 5
tag input-order-04
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-5
@type sample
rate 6
tag input-order-05
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-6
@type sample
rate 7
tag input-order-06
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-7
@type sample
rate 8
tag input-order-07
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-8
@type sample
rate 9
tag input-order-08
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-9
@type sample
rate 10
tag input-order-09
</source>
<source>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-10
@type sample
rate 11
tag input-order-10
</source>
<match **>
@id main
@type label_router
<route>
@label @a2170d34e9940ec56d328100e375c43e
<match>
namespaces default,kube-system
</match>
</route>
</match>
<label @a2170d34e9940ec56d328100e375c43e>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-es-0
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log
port 9200
</match>
</label>
105 changes: 105 additions & 0 deletions apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-output-order-by-index.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<source>
@type forward
bind 0.0.0.0
port 24224
</source>
<match **>
@id main
@type label_router
<route>
@label @a2170d34e9940ec56d328100e375c43e
<match>
namespaces default,kube-system
</match>
</route>
</match>
<label @a2170d34e9940ec56d328100e375c43e>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-0
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-00
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-1
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-01
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-2
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-02
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-3
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-03
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-4
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-04
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-5
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-05
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-6
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-06
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-7
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-07
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-8
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-08
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-9
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-09
port 9200
</match>
<match **>
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-10
@type elasticsearch
host elasticsearch-logging-data.kubesphere-logging-system.svc
logstash_format true
logstash_prefix ks-logstash-log-10
port 9200
</match>
</label>
41 changes: 41 additions & 0 deletions apis/fluentd/v1alpha1/tests/helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,47 @@ func Test_DuplicateRemovalCRSpecs(t *testing.T) {
}
}

// Test_ClusterCfgOutputOrderByIndex guards against a previously-seen string-sort
// regression: outputs with @id suffixes "-0..-10" must render in numeric order,
// not "-0, -1, -10, -2, -3, ...".
func Test_ClusterCfgOutputOrderByIndex(t *testing.T) {
sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{})
testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{}, []fluentdv1alpha1.ClusterOutput{FluentdClusterOutputOrderByIndex}, "./expected/fluentd-cluster-cfg-output-order-by-index.cfg", false)
}

// Test_ClusterCfgFilterOrderByIndex guards against the same string-sort regression
// for filters attached as children of a label plugin.
func Test_ClusterCfgFilterOrderByIndex(t *testing.T) {
sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{})
testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{FluentdClusterFilterOrderByIndex}, []fluentdv1alpha1.ClusterOutput{FluentdclusterOutput2ES}, "./expected/fluentd-cluster-cfg-filter-order-by-index.cfg", false)
}

// Test_ClusterCfgInputOrderByIndex asserts that inputs declared in a single CR
// render in CR definition order. Inputs are not re-sorted by @id in
// RenderMainConfig, so this acts as a boundary guard for any future change that
// would accidentally introduce such a sort on the input rendering path.
func Test_ClusterCfgInputOrderByIndex(t *testing.T) {
g := NewGomegaWithT(t)
sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{})

psr := fluentdv1alpha1.NewGlobalPluginResources("main")
psr.CombineGlobalInputsPlugins(sl, Fluentd.Spec.GlobalInputs)

clustercfgRouter, err := psr.BuildCfgRouter(&FluentdClusterFluentdConfig1)
g.Expect(err).NotTo(HaveOccurred())
clusterInputs := []fluentdv1alpha1.ClusterInput{FluentdClusterInputOrderByIndex}
clusterOutputs := []fluentdv1alpha1.ClusterOutput{FluentdclusterOutput2ES}
clustercfgResources, _ := psr.PatchAndFilterClusterLevelResources(sl, FluentdClusterFluentdConfig1.GetCfgId(), clusterInputs, []fluentdv1alpha1.ClusterFilter{}, clusterOutputs)
err = psr.WithCfgResources(*clustercfgRouter.Label, clustercfgResources)
g.Expect(err).NotTo(HaveOccurred())

for i := 0; i < maxRuntimes; i++ {
config, errs := psr.RenderMainConfig(false)
g.Expect(errs).NotTo(HaveOccurred())
g.Expect(string(getExpectedCfg("./expected/fluentd-cluster-cfg-input-order-by-index.cfg"))).To(Equal(config))
}
}

func Test_RecordTransformer(t *testing.T) {
sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{})
testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{FluentdClusterRecordTransformerFilter}, []fluentdv1alpha1.ClusterOutput{FluentdClusterOutputCluster}, "./expected/fluentd-cluster-cfg-filter-recordTransformer.cfg", false)
Expand Down
Loading
Loading