Skip to content

Commit 58afe98

Browse files
committed
fix: use image of container in MigratorSpec
Previous to this commit, the operator would inject an waiter container that would poll for the image of the first container in the target Pod rather than the image of the specified container. This is surprising behaviour and also potentially problematic when combined with Istio's `holdApplicationUntilProxyStart` functionality, which injects the Istio sidecar at the beginning of the list of containers. This commit ensures the operator uses the container specified in the MigratorSpec, if supplied and present. If the target container name is specified and not present, it falls back to using the image from the first container in the Pod (the previous behaviour). If the target container name is not present, it also uses the previous behaviour.
1 parent ea796df commit 58afe98

File tree

2 files changed

+116
-1
lines changed

2 files changed

+116
-1
lines changed

webhook/webhook.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,18 @@ func (hook *initInjector) handleInner(ctx context.Context, req admission.Request
9292

9393
// For each migrator, inject an initContainer.
9494
for _, m := range migrators {
95+
// Look for the container named in the migrator and pull the image from that. If no container
96+
// matches, fall back to using the first container.
97+
podIdx := 0
98+
for i, p := range pod.Spec.Containers {
99+
if p.Name == m.Spec.Container {
100+
podIdx = i
101+
}
102+
}
95103
initContainer := corev1.Container{
96104
Name: fmt.Sprintf("migrate-wait-%s", m.Name),
97105
Image: os.Getenv("WAITER_IMAGE"),
98-
Command: []string{"/waiter", pod.Spec.Containers[0].Image, m.Namespace, m.Name, os.Getenv("API_HOSTNAME")},
106+
Command: []string{"/waiter", pod.Spec.Containers[podIdx].Image, m.Namespace, m.Name, os.Getenv("API_HOSTNAME")},
99107
Resources: corev1.ResourceRequirements{
100108
Requests: corev1.ResourceList{
101109
corev1.ResourceMemory: resource.MustParse("16M"),

webhook/webhook_test.go

+107
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,113 @@ var _ = Describe("InitInjector", func() {
9494
Expect(pod.Spec.InitContainers[0].Command).To(Equal([]string{"/waiter", "fake", helper.Namespace, "testing", "migrations-operator.migration-operator.svc"}))
9595
})
9696

97+
It("selects the specified container with a multi-container Pod", func() {
98+
c := helper.TestClient
99+
100+
migrator := &migrationsv1beta1.Migrator{
101+
ObjectMeta: metav1.ObjectMeta{Name: "testing"},
102+
Spec: migrationsv1beta1.MigratorSpec{
103+
Selector: &metav1.LabelSelector{
104+
MatchLabels: map[string]string{"app": "testing"},
105+
},
106+
Container: "second",
107+
},
108+
}
109+
c.Create(migrator)
110+
111+
pod := &corev1.Pod{
112+
ObjectMeta: metav1.ObjectMeta{Name: "testing", Labels: map[string]string{"app": "testing"}},
113+
Spec: corev1.PodSpec{
114+
Containers: []corev1.Container{
115+
{
116+
Name: "main",
117+
Image: "fake",
118+
},
119+
{
120+
Name: "second",
121+
Image: "foo",
122+
},
123+
},
124+
},
125+
}
126+
c.Create(pod)
127+
128+
c.EventuallyGetName("testing", pod)
129+
Expect(pod.Spec.InitContainers).To(HaveLen(1))
130+
Expect(pod.Spec.InitContainers[0].Command).To(Equal([]string{"/waiter", "foo", helper.Namespace, "testing", "migrations-operator.migration-operator.svc"}))
131+
})
132+
133+
It("falls back to the first container if the specified container doesn't exist", func() {
134+
c := helper.TestClient
135+
136+
migrator := &migrationsv1beta1.Migrator{
137+
ObjectMeta: metav1.ObjectMeta{Name: "testing"},
138+
Spec: migrationsv1beta1.MigratorSpec{
139+
Selector: &metav1.LabelSelector{
140+
MatchLabels: map[string]string{"app": "testing"},
141+
},
142+
Container: "third",
143+
},
144+
}
145+
c.Create(migrator)
146+
147+
pod := &corev1.Pod{
148+
ObjectMeta: metav1.ObjectMeta{Name: "testing", Labels: map[string]string{"app": "testing"}},
149+
Spec: corev1.PodSpec{
150+
Containers: []corev1.Container{
151+
{
152+
Name: "main",
153+
Image: "bar",
154+
},
155+
{
156+
Name: "second",
157+
Image: "foo",
158+
},
159+
},
160+
},
161+
}
162+
c.Create(pod)
163+
164+
c.EventuallyGetName("testing", pod)
165+
Expect(pod.Spec.InitContainers).To(HaveLen(1))
166+
Expect(pod.Spec.InitContainers[0].Command).To(Equal([]string{"/waiter", "bar", helper.Namespace, "testing", "migrations-operator.migration-operator.svc"}))
167+
})
168+
169+
It("uses the first container image if no container name is supplied with a multi-container Pod", func() {
170+
c := helper.TestClient
171+
172+
migrator := &migrationsv1beta1.Migrator{
173+
ObjectMeta: metav1.ObjectMeta{Name: "testing"},
174+
Spec: migrationsv1beta1.MigratorSpec{
175+
Selector: &metav1.LabelSelector{
176+
MatchLabels: map[string]string{"app": "testing"},
177+
},
178+
},
179+
}
180+
c.Create(migrator)
181+
182+
pod := &corev1.Pod{
183+
ObjectMeta: metav1.ObjectMeta{Name: "testing", Labels: map[string]string{"app": "testing"}},
184+
Spec: corev1.PodSpec{
185+
Containers: []corev1.Container{
186+
{
187+
Name: "main",
188+
Image: "bar",
189+
},
190+
{
191+
Name: "second",
192+
Image: "foo",
193+
},
194+
},
195+
},
196+
}
197+
c.Create(pod)
198+
199+
c.EventuallyGetName("testing", pod)
200+
Expect(pod.Spec.InitContainers).To(HaveLen(1))
201+
Expect(pod.Spec.InitContainers[0].Command).To(Equal([]string{"/waiter", "bar", helper.Namespace, "testing", "migrations-operator.migration-operator.svc"}))
202+
})
203+
97204
It("doesn't inject with a non-matching migrator", func() {
98205
c := helper.TestClient
99206

0 commit comments

Comments
 (0)