Skip to content

Commit 3a18c9d

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 3a18c9d

File tree

2 files changed

+116
-0
lines changed

2 files changed

+116
-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

+111
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,111 @@ 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+
// sorting by the names if both kinds match and there are no namespaces
147+
raw: map[string]interface{}{
148+
"a": mkobj("CustomResourceDefinition", "crd2", ""),
149+
"b": mkobj("CustomResourceDefinition", "crd", ""),
150+
"c": mkobj("CustomResourceDefinition", "crd1", ""),
151+
},
152+
targets: nil,
153+
state: manifest.List{
154+
mkobj("CustomResourceDefinition", "crd", ""),
155+
mkobj("CustomResourceDefinition", "crd1", ""),
156+
mkobj("CustomResourceDefinition", "crd2", ""),
157+
},
158+
},
159+
}
160+
161+
for _, test := range tests {
162+
res, err := Reconcile(test.raw, v1alpha1.New().Spec, test.targets)
163+
164+
require.NoError(t, err)
165+
require.Equal(t, test.state, res)
166+
}
167+
}

0 commit comments

Comments
 (0)