Skip to content

Commit 63194c9

Browse files
committed
Implement sorting by NS and test object sorting.
- the original sort code in Reconcile() didn't take into account the case where two objects of the same kind and name might reside in different namespace, allowing the sort being non-deterministic at times. - testing of the sort code is now implemented. There might be some refactor pending, since `kubernetes_test.go` also contains code testing `Reconcile()`, but this is outside the scope of this PR. Signed-off-by: Milan Plzik <[email protected]>
1 parent 7237d80 commit 63194c9

File tree

2 files changed

+102
-0
lines changed

2 files changed

+102
-0
lines changed

pkg/kubernetes/reconcile.go

+5
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ func Reconcile(raw map[string]interface{}, spec v1alpha1.Spec, targets []*regexp
113113
return out[i].Kind() < out[j].Kind()
114114
}
115115

116+
// If namespaces differ, sort by the namespace.
117+
if out[i].Metadata().Namespace() != out[j].Metadata().Namespace() {
118+
return out[i].Metadata().Namespace() < out[j].Metadata().Namespace()
119+
}
120+
116121
// Otherwise, order the objects by name.
117122
return out[i].Metadata().Name() < out[j].Metadata().Name()
118123
})

pkg/kubernetes/reconcile_test.go

+97
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package kubernetes
22

33
import (
4+
"regexp"
45
"testing"
56

7+
"github.com/grafana/tanka/pkg/kubernetes/manifest"
8+
"github.com/grafana/tanka/pkg/spec/v1alpha1"
69
"github.com/stretchr/testify/assert"
710
"github.com/stretchr/testify/require"
811
)
@@ -54,3 +57,97 @@ func TestExtract(t *testing.T) {
5457
})
5558
}
5659
}
60+
61+
func mkobj(kind string, name string, ns string) map[string]interface{} {
62+
if ns == "" {
63+
return map[string]interface{}{
64+
"kind": kind,
65+
"apiVersion": "apiversion",
66+
"metadata": map[string]interface{}{
67+
"name": name,
68+
},
69+
}
70+
}
71+
72+
return map[string]interface{}{
73+
"kind": kind,
74+
"apiVersion": "apiversion",
75+
"metadata": map[string]interface{}{
76+
"name": name,
77+
"namespace": ns,
78+
},
79+
}
80+
}
81+
82+
func TestReconcileSorting(t *testing.T) {
83+
tests := []struct {
84+
raw map[string]interface{}
85+
targets []*regexp.Regexp
86+
state manifest.List
87+
err error
88+
}{
89+
{
90+
// sorting by kinds in the `kindOrder` list
91+
raw: map[string]interface{}{
92+
"a": mkobj("Service", "service", "default"),
93+
"b": mkobj("Deployment", "deployment", "default"),
94+
"c": mkobj("CustomResourceDefinition", "crd", ""),
95+
},
96+
targets: nil,
97+
state: manifest.List{
98+
mkobj("CustomResourceDefinition", "crd", ""),
99+
mkobj("Service", "service", "default"),
100+
mkobj("Deployment", "deployment", "default"),
101+
},
102+
},
103+
{
104+
// alphabtical sorting by kinds outside `kindOrder` list
105+
raw: map[string]interface{}{
106+
"a": mkobj("B", "b", "default"),
107+
"b": mkobj("C", "c", "default"),
108+
"c": mkobj("A", "a", "default"),
109+
},
110+
targets: nil,
111+
state: manifest.List{
112+
mkobj("A", "a", "default"),
113+
mkobj("B", "b", "default"),
114+
mkobj("C", "c", "default"),
115+
},
116+
},
117+
{
118+
// sorting by the namespace if kinds match
119+
raw: map[string]interface{}{
120+
"a": mkobj("Service", "service", "default2"),
121+
"b": mkobj("Service", "service", "default"),
122+
"c": mkobj("Service", "service", "default1"),
123+
},
124+
targets: nil,
125+
state: manifest.List{
126+
mkobj("Service", "service", "default"),
127+
mkobj("Service", "service", "default1"),
128+
mkobj("Service", "service", "default2"),
129+
},
130+
},
131+
{
132+
// sorting by the names if both kinds and namespaces match
133+
raw: map[string]interface{}{
134+
"a": mkobj("Service", "service2", "default"),
135+
"b": mkobj("Service", "service", "default"),
136+
"c": mkobj("Service", "service1", "default"),
137+
},
138+
targets: nil,
139+
state: manifest.List{
140+
mkobj("Service", "service", "default"),
141+
mkobj("Service", "service1", "default"),
142+
mkobj("Service", "service2", "default"),
143+
},
144+
},
145+
}
146+
147+
for _, test := range tests {
148+
res, err := Reconcile(test.raw, v1alpha1.New().Spec, test.targets)
149+
150+
require.NoError(t, err)
151+
require.Equal(t, test.state, res)
152+
}
153+
}

0 commit comments

Comments
 (0)