Skip to content

Commit f548ad3

Browse files
sugaf1204sugaf1204
committed
fix(fluentd): sort plugin @id numerically to preserve CR definition order
Lexicographic comparison placed index 10 before index 2 (-10 < -2 as strings). Use a numeric-aware comparator when both @ids share a "<prefix>-<int>" shape; fall back to lexicographic otherwise. Adds regression tests for inputs, filters, and outputs with 11 plugins per CR. Signed-off-by: sugaf1204 <sugaf1204@icloud.com>
1 parent eab602e commit f548ad3

File tree

6 files changed

+546
-1
lines changed

6 files changed

+546
-1
lines changed

‎apis/fluentd/v1alpha1/plugins/params/model.go‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"sort"
77
"strconv"
8+
"strings"
89

910
"github.com/fluent/fluent-operator/v3/pkg/utils"
1011
)
@@ -253,8 +254,36 @@ func (a PluginStoreByNameById) Less(i, j int) bool {
253254
if a[i].GetTag() != "**" && a[j].GetTag() == "**" {
254255
return true
255256
}
256-
return a[i].GetId() < a[j].GetId()
257+
return lessId(a[i].GetId(), a[j].GetId())
257258
} else {
258259
return a[i].Name < a[j].Name
259260
}
260261
}
262+
263+
// lessId orders @id strings so that numeric suffixes compare numerically once
264+
// both ids share a "<prefix>-<index>" shape. A plain lexicographic compare
265+
// would place "<p>-10" before "<p>-2" because '1' < '2'. Falls back to
266+
// lexicographic order when either side lacks a numeric trailing segment or
267+
// when the parsed prefixes differ.
268+
func lessId(id1, id2 string) bool {
269+
p1, n1, ok1 := splitIdIndex(id1)
270+
p2, n2, ok2 := splitIdIndex(id2)
271+
if ok1 && ok2 && p1 == p2 {
272+
return n1 < n2
273+
}
274+
return id1 < id2
275+
}
276+
277+
// splitIdIndex splits an @id at the last '-'. It returns the prefix, the
278+
// parsed numeric index, and true when the trailing segment is an integer.
279+
func splitIdIndex(id string) (string, int, bool) {
280+
idx := strings.LastIndex(id, "-")
281+
if idx < 0 {
282+
return "", 0, false
283+
}
284+
n, err := strconv.Atoi(id[idx+1:])
285+
if err != nil {
286+
return "", 0, false
287+
}
288+
return id[:idx], n, true
289+
}

‎apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-filter-order-by-index.cfg‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
<source>
2+
@type forward
3+
bind 0.0.0.0
4+
port 24224
5+
</source>
6+
<match **>
7+
@id main
8+
@type label_router
9+
<route>
10+
@label @a2170d34e9940ec56d328100e375c43e
11+
<match>
12+
namespaces default,kube-system
13+
</match>
14+
</route>
15+
</match>
16+
<label @a2170d34e9940ec56d328100e375c43e>
17+
<filter **>
18+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-0
19+
@type record_transformer
20+
<record>
21+
k00 v00
22+
</record>
23+
</filter>
24+
<filter **>
25+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-1
26+
@type record_transformer
27+
<record>
28+
k01 v01
29+
</record>
30+
</filter>
31+
<filter **>
32+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-2
33+
@type record_transformer
34+
<record>
35+
k02 v02
36+
</record>
37+
</filter>
38+
<filter **>
39+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-3
40+
@type record_transformer
41+
<record>
42+
k03 v03
43+
</record>
44+
</filter>
45+
<filter **>
46+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-4
47+
@type record_transformer
48+
<record>
49+
k04 v04
50+
</record>
51+
</filter>
52+
<filter **>
53+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-5
54+
@type record_transformer
55+
<record>
56+
k05 v05
57+
</record>
58+
</filter>
59+
<filter **>
60+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-6
61+
@type record_transformer
62+
<record>
63+
k06 v06
64+
</record>
65+
</filter>
66+
<filter **>
67+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-7
68+
@type record_transformer
69+
<record>
70+
k07 v07
71+
</record>
72+
</filter>
73+
<filter **>
74+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-8
75+
@type record_transformer
76+
<record>
77+
k08 v08
78+
</record>
79+
</filter>
80+
<filter **>
81+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-9
82+
@type record_transformer
83+
<record>
84+
k09 v09
85+
</record>
86+
</filter>
87+
<filter **>
88+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterfilter::fluentd-filter-order-10
89+
@type record_transformer
90+
<record>
91+
k10 v10
92+
</record>
93+
</filter>
94+
<match **>
95+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-es-0
96+
@type elasticsearch
97+
host elasticsearch-logging-data.kubesphere-logging-system.svc
98+
logstash_format true
99+
logstash_prefix ks-logstash-log
100+
port 9200
101+
</match>
102+
</label>

‎apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-input-order-by-index.cfg‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<source>
2+
@type forward
3+
bind 0.0.0.0
4+
port 24224
5+
</source>
6+
<source>
7+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-0
8+
@type sample
9+
rate 1
10+
tag input-order-00
11+
</source>
12+
<source>
13+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-1
14+
@type sample
15+
rate 2
16+
tag input-order-01
17+
</source>
18+
<source>
19+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-2
20+
@type sample
21+
rate 3
22+
tag input-order-02
23+
</source>
24+
<source>
25+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-3
26+
@type sample
27+
rate 4
28+
tag input-order-03
29+
</source>
30+
<source>
31+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-4
32+
@type sample
33+
rate 5
34+
tag input-order-04
35+
</source>
36+
<source>
37+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-5
38+
@type sample
39+
rate 6
40+
tag input-order-05
41+
</source>
42+
<source>
43+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-6
44+
@type sample
45+
rate 7
46+
tag input-order-06
47+
</source>
48+
<source>
49+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-7
50+
@type sample
51+
rate 8
52+
tag input-order-07
53+
</source>
54+
<source>
55+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-8
56+
@type sample
57+
rate 9
58+
tag input-order-08
59+
</source>
60+
<source>
61+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-9
62+
@type sample
63+
rate 10
64+
tag input-order-09
65+
</source>
66+
<source>
67+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusterinput::fluentd-input-order-10
68+
@type sample
69+
rate 11
70+
tag input-order-10
71+
</source>
72+
<match **>
73+
@id main
74+
@type label_router
75+
<route>
76+
@label @a2170d34e9940ec56d328100e375c43e
77+
<match>
78+
namespaces default,kube-system
79+
</match>
80+
</route>
81+
</match>
82+
<label @a2170d34e9940ec56d328100e375c43e>
83+
<match **>
84+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-es-0
85+
@type elasticsearch
86+
host elasticsearch-logging-data.kubesphere-logging-system.svc
87+
logstash_format true
88+
logstash_prefix ks-logstash-log
89+
port 9200
90+
</match>
91+
</label>

‎apis/fluentd/v1alpha1/tests/expected/fluentd-cluster-cfg-output-order-by-index.cfg‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
<source>
2+
@type forward
3+
bind 0.0.0.0
4+
port 24224
5+
</source>
6+
<match **>
7+
@id main
8+
@type label_router
9+
<route>
10+
@label @a2170d34e9940ec56d328100e375c43e
11+
<match>
12+
namespaces default,kube-system
13+
</match>
14+
</route>
15+
</match>
16+
<label @a2170d34e9940ec56d328100e375c43e>
17+
<match **>
18+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-0
19+
@type elasticsearch
20+
host elasticsearch-logging-data.kubesphere-logging-system.svc
21+
logstash_format true
22+
logstash_prefix ks-logstash-log-00
23+
port 9200
24+
</match>
25+
<match **>
26+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-1
27+
@type elasticsearch
28+
host elasticsearch-logging-data.kubesphere-logging-system.svc
29+
logstash_format true
30+
logstash_prefix ks-logstash-log-01
31+
port 9200
32+
</match>
33+
<match **>
34+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-2
35+
@type elasticsearch
36+
host elasticsearch-logging-data.kubesphere-logging-system.svc
37+
logstash_format true
38+
logstash_prefix ks-logstash-log-02
39+
port 9200
40+
</match>
41+
<match **>
42+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-3
43+
@type elasticsearch
44+
host elasticsearch-logging-data.kubesphere-logging-system.svc
45+
logstash_format true
46+
logstash_prefix ks-logstash-log-03
47+
port 9200
48+
</match>
49+
<match **>
50+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-4
51+
@type elasticsearch
52+
host elasticsearch-logging-data.kubesphere-logging-system.svc
53+
logstash_format true
54+
logstash_prefix ks-logstash-log-04
55+
port 9200
56+
</match>
57+
<match **>
58+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-5
59+
@type elasticsearch
60+
host elasticsearch-logging-data.kubesphere-logging-system.svc
61+
logstash_format true
62+
logstash_prefix ks-logstash-log-05
63+
port 9200
64+
</match>
65+
<match **>
66+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-6
67+
@type elasticsearch
68+
host elasticsearch-logging-data.kubesphere-logging-system.svc
69+
logstash_format true
70+
logstash_prefix ks-logstash-log-06
71+
port 9200
72+
</match>
73+
<match **>
74+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-7
75+
@type elasticsearch
76+
host elasticsearch-logging-data.kubesphere-logging-system.svc
77+
logstash_format true
78+
logstash_prefix ks-logstash-log-07
79+
port 9200
80+
</match>
81+
<match **>
82+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-8
83+
@type elasticsearch
84+
host elasticsearch-logging-data.kubesphere-logging-system.svc
85+
logstash_format true
86+
logstash_prefix ks-logstash-log-08
87+
port 9200
88+
</match>
89+
<match **>
90+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-9
91+
@type elasticsearch
92+
host elasticsearch-logging-data.kubesphere-logging-system.svc
93+
logstash_format true
94+
logstash_prefix ks-logstash-log-09
95+
port 9200
96+
</match>
97+
<match **>
98+
@id ClusterFluentdConfig-cluster-fluentd-config::cluster::clusteroutput::fluentd-output-order-10
99+
@type elasticsearch
100+
host elasticsearch-logging-data.kubesphere-logging-system.svc
101+
logstash_format true
102+
logstash_prefix ks-logstash-log-10
103+
port 9200
104+
</match>
105+
</label>

‎apis/fluentd/v1alpha1/tests/helper_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,47 @@ func Test_DuplicateRemovalCRSpecs(t *testing.T) {
498498
}
499499
}
500500

501+
// Test_ClusterCfgOutputOrderByIndex guards against a previously-seen string-sort
502+
// regression: outputs with @id suffixes "-0..-10" must render in numeric order,
503+
// not "-0, -1, -10, -2, -3, ...".
504+
func Test_ClusterCfgOutputOrderByIndex(t *testing.T) {
505+
sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{})
506+
testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{}, []fluentdv1alpha1.ClusterOutput{FluentdClusterOutputOrderByIndex}, "./expected/fluentd-cluster-cfg-output-order-by-index.cfg", false)
507+
}
508+
509+
// Test_ClusterCfgFilterOrderByIndex guards against the same string-sort regression
510+
// for filters attached as children of a label plugin.
511+
func Test_ClusterCfgFilterOrderByIndex(t *testing.T) {
512+
sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{})
513+
testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{FluentdClusterFilterOrderByIndex}, []fluentdv1alpha1.ClusterOutput{FluentdclusterOutput2ES}, "./expected/fluentd-cluster-cfg-filter-order-by-index.cfg", false)
514+
}
515+
516+
// Test_ClusterCfgInputOrderByIndex asserts that inputs declared in a single CR
517+
// render in CR definition order. Inputs are not re-sorted by @id in
518+
// RenderMainConfig, so this acts as a boundary guard for any future change that
519+
// would accidentally introduce such a sort on the input rendering path.
520+
func Test_ClusterCfgInputOrderByIndex(t *testing.T) {
521+
g := NewGomegaWithT(t)
522+
sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{})
523+
524+
psr := fluentdv1alpha1.NewGlobalPluginResources("main")
525+
psr.CombineGlobalInputsPlugins(sl, Fluentd.Spec.GlobalInputs)
526+
527+
clustercfgRouter, err := psr.BuildCfgRouter(&FluentdClusterFluentdConfig1)
528+
g.Expect(err).NotTo(HaveOccurred())
529+
clusterInputs := []fluentdv1alpha1.ClusterInput{FluentdClusterInputOrderByIndex}
530+
clusterOutputs := []fluentdv1alpha1.ClusterOutput{FluentdclusterOutput2ES}
531+
clustercfgResources, _ := psr.PatchAndFilterClusterLevelResources(sl, FluentdClusterFluentdConfig1.GetCfgId(), clusterInputs, []fluentdv1alpha1.ClusterFilter{}, clusterOutputs)
532+
err = psr.WithCfgResources(*clustercfgRouter.Label, clustercfgResources)
533+
g.Expect(err).NotTo(HaveOccurred())
534+
535+
for i := 0; i < maxRuntimes; i++ {
536+
config, errs := psr.RenderMainConfig(false)
537+
g.Expect(errs).NotTo(HaveOccurred())
538+
g.Expect(string(getExpectedCfg("./expected/fluentd-cluster-cfg-input-order-by-index.cfg"))).To(Equal(config))
539+
}
540+
}
541+
501542
func Test_RecordTransformer(t *testing.T) {
502543
sl := plugins.NewSecretLoader(nil, Fluentd.Namespace, logr.Logger{})
503544
testClusterConfigWithFiltersAndOutputs(t, sl, Fluentd, &FluentdClusterFluentdConfig1, []fluentdv1alpha1.ClusterFilter{FluentdClusterRecordTransformerFilter}, []fluentdv1alpha1.ClusterOutput{FluentdClusterOutputCluster}, "./expected/fluentd-cluster-cfg-filter-recordTransformer.cfg", false)

0 commit comments

Comments
 (0)