Skip to content

Commit d4ca474

Browse files
authored
fix: error when receiving an AppWrapper with 0 or 1 ordered instances (#96)
* fix: error when receiving an AppWrapper with 0 or 1 ordered instances Co-authored-by: Fiona-Waters [email protected] * test receiving an AppWrapper with 0 or 1 ordered instances * add: log for no orderedInstances, and tests.
1 parent ac67ed4 commit d4ca474

File tree

2 files changed

+119
-4
lines changed

2 files changed

+119
-4
lines changed
+107
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package controllers
2+
3+
import (
4+
"github.com/onsi/gomega"
5+
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
6+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
7+
"testing"
8+
)
9+
10+
func TestDiscoverInstanceTypes(t *testing.T) {
11+
g := gomega.NewGomegaWithT(t)
12+
13+
tests := []struct {
14+
name string
15+
input *arbv1.AppWrapper
16+
expected map[string]int
17+
expectedErr error
18+
}{
19+
{
20+
name: "Test with multiple orderedinstance",
21+
input: &arbv1.AppWrapper{
22+
ObjectMeta: metav1.ObjectMeta{
23+
Labels: map[string]string{
24+
"orderedinstance": "test.instance1_test.instance2",
25+
},
26+
},
27+
Spec: arbv1.AppWrapperSpec{
28+
AggrResources: arbv1.AppWrapperResourceList{
29+
GenericItems: []arbv1.AppWrapperGenericResource{
30+
{
31+
CustomPodResources: []arbv1.CustomPodResourceTemplate{
32+
{
33+
Replicas: 1,
34+
},
35+
{
36+
Replicas: 2,
37+
},
38+
},
39+
TypeMeta: metav1.TypeMeta{},
40+
},
41+
},
42+
},
43+
},
44+
},
45+
expected: map[string]int{
46+
"test.instance1": 1,
47+
"test.instance2": 2,
48+
},
49+
},
50+
{
51+
name: "Test with one orderedinstance",
52+
input: &arbv1.AppWrapper{
53+
ObjectMeta: metav1.ObjectMeta{
54+
Labels: map[string]string{
55+
"orderedinstance": "test.instance1",
56+
},
57+
},
58+
Spec: arbv1.AppWrapperSpec{
59+
AggrResources: arbv1.AppWrapperResourceList{
60+
GenericItems: []arbv1.AppWrapperGenericResource{
61+
{
62+
CustomPodResources: []arbv1.CustomPodResourceTemplate{
63+
{
64+
Replicas: 1,
65+
},
66+
},
67+
TypeMeta: metav1.TypeMeta{},
68+
},
69+
},
70+
},
71+
},
72+
},
73+
expected: map[string]int{
74+
"test.instance1": 1,
75+
},
76+
},
77+
{
78+
name: "Test with empty orderedinstance",
79+
input: &arbv1.AppWrapper{
80+
ObjectMeta: metav1.ObjectMeta{
81+
Labels: map[string]string{
82+
"orderedinstance": "",
83+
},
84+
},
85+
},
86+
expected: map[string]int{},
87+
},
88+
{
89+
name: "Test with empty orderedinstance",
90+
input: &arbv1.AppWrapper{
91+
ObjectMeta: metav1.ObjectMeta{
92+
Labels: map[string]string{
93+
// zero instances
94+
},
95+
},
96+
},
97+
expected: map[string]int{},
98+
},
99+
}
100+
101+
for _, test := range tests {
102+
t.Run(test.name, func(t *testing.T) {
103+
result := discoverInstanceTypes(test.input)
104+
g.Expect(result).To(gomega.Equal(test.expected))
105+
})
106+
}
107+
}

controllers/appwrapper_controller.go

+12-4
Original file line numberDiff line numberDiff line change
@@ -280,13 +280,21 @@ func discoverInstanceTypes(aw *arbv1.AppWrapper) map[string]int {
280280
instanceRequired = strings.Split(v, "_")
281281
}
282282
}
283+
284+
if len(instanceRequired) < 1 {
285+
klog.Infof("Found AW %s that cannot be scaled due to missing orderedinstance label", aw.ObjectMeta.Name)
286+
return demandMapPerInstanceType
287+
}
288+
283289
klog.Infof("The instanceRequired array: %v", instanceRequired)
284290
for id, genericItem := range aw.Spec.AggrResources.GenericItems {
285291
for idx, val := range genericItem.CustomPodResources {
286-
instanceName := instanceRequired[idx+id]
287-
klog.Infof("Got instance name %v", instanceName)
288-
demandMapPerInstanceType[instanceName] = val.Replicas
289-
292+
combinedIndex := idx + id
293+
if combinedIndex < len(instanceRequired) {
294+
instanceName := instanceRequired[combinedIndex]
295+
klog.Infof("Got instance name %v", instanceName)
296+
demandMapPerInstanceType[instanceName] = val.Replicas
297+
}
290298
}
291299
}
292300
return demandMapPerInstanceType

0 commit comments

Comments
 (0)