Skip to content

Commit 70c8b8f

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #178 from vikaschoudhary16/remove-containers-in-update
Bug 1710172: lib/resourcemerge: Remove containers from existing based on required
2 parents b0250fa + 3d1ad76 commit 70c8b8f

File tree

2 files changed

+126
-40
lines changed

2 files changed

+126
-40
lines changed

‎lib/resourcemerge/core.go

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -24,38 +24,8 @@ func ensurePodTemplateSpec(modified *bool, existing *corev1.PodTemplateSpec, req
2424
}
2525

2626
func ensurePodSpec(modified *bool, existing *corev1.PodSpec, required corev1.PodSpec) {
27-
// any container we specify, we require.
28-
for _, required := range required.InitContainers {
29-
var existingCurr *corev1.Container
30-
for j, curr := range existing.InitContainers {
31-
if curr.Name == required.Name {
32-
existingCurr = &existing.InitContainers[j]
33-
break
34-
}
35-
}
36-
if existingCurr == nil {
37-
*modified = true
38-
existing.Containers = append(existing.InitContainers, corev1.Container{})
39-
existingCurr = &existing.InitContainers[len(existing.InitContainers)-1]
40-
}
41-
ensureContainer(modified, existingCurr, required)
42-
}
43-
44-
for _, required := range required.Containers {
45-
var existingCurr *corev1.Container
46-
for j, curr := range existing.Containers {
47-
if curr.Name == required.Name {
48-
existingCurr = &existing.Containers[j]
49-
break
50-
}
51-
}
52-
if existingCurr == nil {
53-
*modified = true
54-
existing.Containers = append(existing.Containers, corev1.Container{})
55-
existingCurr = &existing.Containers[len(existing.Containers)-1]
56-
}
57-
ensureContainer(modified, existingCurr, required)
58-
}
27+
ensureContainers(modified, &existing.InitContainers, required.InitContainers)
28+
ensureContainers(modified, &existing.Containers, required.Containers)
5929

6030
// any volume we specify, we require.
6131
for _, required := range required.Volumes {
@@ -91,6 +61,37 @@ func ensurePodSpec(modified *bool, existing *corev1.PodSpec, required corev1.Pod
9161
setInt32Ptr(modified, &existing.Priority, required.Priority)
9262
}
9363

64+
func ensureContainers(modified *bool, existing *[]corev1.Container, required []corev1.Container) {
65+
var existingCurr *corev1.Container
66+
for i, existingContainer := range *existing {
67+
for _, requiredContainer := range required {
68+
if existingContainer.Name == requiredContainer.Name {
69+
existingCurr = &(*existing)[i]
70+
ensureContainer(modified, existingCurr, requiredContainer)
71+
break
72+
}
73+
}
74+
if existingCurr == nil {
75+
*modified = true
76+
*existing = append((*existing)[:i], (*existing)[i+1:]...)
77+
}
78+
}
79+
80+
for _, requiredContainer := range required {
81+
match := false
82+
for _, existingContainer := range *existing {
83+
if existingContainer.Name == requiredContainer.Name {
84+
match = true
85+
break
86+
}
87+
}
88+
if !match {
89+
*modified = true
90+
*existing = append(*existing, requiredContainer)
91+
}
92+
}
93+
}
94+
9495
func ensureContainer(modified *bool, existing *corev1.Container, required corev1.Container) {
9596
setStringIfSet(modified, &existing.Name, required.Name)
9697
setStringIfSet(modified, &existing.Image, required.Image)

‎lib/resourcemerge/core_test.go

Lines changed: 93 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
corev1 "k8s.io/api/core/v1"
77
"k8s.io/apimachinery/pkg/api/equality"
8+
"k8s.io/apimachinery/pkg/util/diff"
89
"k8s.io/utils/pointer"
910
)
1011

@@ -16,14 +17,98 @@ func TestEnsurePodSpec(t *testing.T) {
1617

1718
expectedModified bool
1819
expected corev1.PodSpec
19-
}{{
20-
name: "empty inputs",
21-
existing: corev1.PodSpec{},
22-
input: corev1.PodSpec{},
20+
}{
21+
{
22+
name: "empty inputs",
23+
existing: corev1.PodSpec{},
24+
input: corev1.PodSpec{},
2325

24-
expectedModified: false,
25-
expected: corev1.PodSpec{},
26-
}}
26+
expectedModified: false,
27+
expected: corev1.PodSpec{},
28+
},
29+
{
30+
name: "remove regular containers from existing",
31+
existing: corev1.PodSpec{
32+
Containers: []corev1.Container{
33+
corev1.Container{Name: "test"}}},
34+
input: corev1.PodSpec{},
35+
36+
expectedModified: true,
37+
expected: corev1.PodSpec{},
38+
},
39+
{
40+
name: "remove regular and init containers from existing",
41+
existing: corev1.PodSpec{
42+
InitContainers: []corev1.Container{
43+
corev1.Container{Name: "test-init"}},
44+
Containers: []corev1.Container{
45+
corev1.Container{Name: "test"}}},
46+
input: corev1.PodSpec{},
47+
48+
expectedModified: true,
49+
expected: corev1.PodSpec{},
50+
},
51+
{
52+
name: "remove init containers from existing",
53+
existing: corev1.PodSpec{
54+
InitContainers: []corev1.Container{
55+
corev1.Container{Name: "test-init"}}},
56+
input: corev1.PodSpec{},
57+
58+
expectedModified: true,
59+
expected: corev1.PodSpec{},
60+
},
61+
{
62+
name: "append regular and init containers",
63+
existing: corev1.PodSpec{
64+
InitContainers: []corev1.Container{
65+
corev1.Container{Name: "test-init-a"}},
66+
Containers: []corev1.Container{
67+
corev1.Container{Name: "test-a"}}},
68+
input: corev1.PodSpec{
69+
InitContainers: []corev1.Container{
70+
corev1.Container{Name: "test-init-a"},
71+
corev1.Container{Name: "test-init-b"},
72+
},
73+
Containers: []corev1.Container{
74+
corev1.Container{Name: "test-a"},
75+
corev1.Container{Name: "test-b"},
76+
},
77+
},
78+
79+
expectedModified: true,
80+
expected: corev1.PodSpec{
81+
InitContainers: []corev1.Container{
82+
corev1.Container{Name: "test-init-a"},
83+
corev1.Container{Name: "test-init-b"},
84+
},
85+
Containers: []corev1.Container{
86+
corev1.Container{Name: "test-a"},
87+
corev1.Container{Name: "test-b"},
88+
},
89+
},
90+
},
91+
{
92+
name: "match regular and init containers",
93+
existing: corev1.PodSpec{
94+
InitContainers: []corev1.Container{
95+
corev1.Container{Name: "test-init"}},
96+
Containers: []corev1.Container{
97+
corev1.Container{Name: "test"}}},
98+
input: corev1.PodSpec{
99+
InitContainers: []corev1.Container{
100+
corev1.Container{Name: "test-init"}},
101+
Containers: []corev1.Container{
102+
corev1.Container{Name: "test"}}},
103+
104+
expectedModified: false,
105+
expected: corev1.PodSpec{
106+
InitContainers: []corev1.Container{
107+
corev1.Container{Name: "test-init"}},
108+
Containers: []corev1.Container{
109+
corev1.Container{Name: "test"}}},
110+
},
111+
}
27112

28113
for _, test := range tests {
29114
t.Run(test.name, func(t *testing.T) {
@@ -34,7 +119,7 @@ func TestEnsurePodSpec(t *testing.T) {
34119
}
35120

36121
if !equality.Semantic.DeepEqual(test.existing, test.expected) {
37-
t.Errorf("mismatch PodSpec got: %v want: %v", test.existing, test.expected)
122+
t.Errorf("unexpected: %s", diff.ObjectReflectDiff(test.expected, test.existing))
38123
}
39124
})
40125
}

0 commit comments

Comments
 (0)