Skip to content

Commit c9fe488

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #412 from deads2k/use-internal-lb
update CVO to inject internal loadbalancer for use by the CVO pod
2 parents b2b3add + f0f00ff commit c9fe488

File tree

4 files changed

+282
-10
lines changed

4 files changed

+282
-10
lines changed

‎install/0000_00_cluster-version-operator_03_deployment.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ spec:
4545
name: serving-cert
4646
readOnly: true
4747
env:
48-
- name: KUBERNETES_SERVICE_PORT # allows CVO to communicate with apiserver directly on same host.
48+
- name: KUBERNETES_SERVICE_PORT # allows CVO to communicate with apiserver directly on same host. Is substituted with port from infrastructures.status.apiServerInternalURL if available.
4949
value: "6443"
50-
- name: KUBERNETES_SERVICE_HOST # allows CVO to communicate with apiserver directly on same host.
50+
- name: KUBERNETES_SERVICE_HOST # allows CVO to communicate with apiserver directly on same host. Is substituted with hostname from infrastructures.status.apiServerInternalURL if available.
5151
value: "127.0.0.1"
5252
- name: NODE_NAME
5353
valueFrom:

‎lib/resourcebuilder/apps.go

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package resourcebuilder
33
import (
44
"context"
55
"fmt"
6+
"net"
7+
"net/url"
68
"strings"
79

810
configv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
@@ -22,18 +24,20 @@ import (
2224
)
2325

2426
type deploymentBuilder struct {
25-
client *appsclientv1.AppsV1Client
26-
proxyGetter configv1.ProxiesGetter
27-
raw []byte
28-
modifier MetaV1ObjectModifierFunc
29-
mode Mode
27+
client *appsclientv1.AppsV1Client
28+
proxyGetter configv1.ProxiesGetter
29+
infrastructureGetter configv1.InfrastructuresGetter
30+
raw []byte
31+
modifier MetaV1ObjectModifierFunc
32+
mode Mode
3033
}
3134

3235
func newDeploymentBuilder(config *rest.Config, m lib.Manifest) Interface {
3336
return &deploymentBuilder{
34-
client: appsclientv1.NewForConfigOrDie(withProtobuf(config)),
35-
proxyGetter: configv1.NewForConfigOrDie(config),
36-
raw: m.Raw,
37+
client: appsclientv1.NewForConfigOrDie(withProtobuf(config)),
38+
proxyGetter: configv1.NewForConfigOrDie(config),
39+
infrastructureGetter: configv1.NewForConfigOrDie(config),
40+
raw: m.Raw,
3741
}
3842
}
3943

@@ -72,6 +76,43 @@ func (b *deploymentBuilder) Do(ctx context.Context) error {
7276
}
7377
}
7478

79+
// if we detect the CVO deployment we need to replace the KUBERNETES_SERVICE_HOST env var with the internal load
80+
// balancer to be resilient to kube-apiserver rollouts that cause the localhost server to become non-responsive for
81+
// multiple minutes.
82+
if deployment.Namespace == "openshift-cluster-version" && deployment.Name == "cluster-version-operator" {
83+
infrastructureConfig, err := b.infrastructureGetter.Infrastructures().Get("cluster", metav1.GetOptions{})
84+
// not found just means that we don't have infrastructure configuration yet, so we should tolerate not found and avoid substitution
85+
if err != nil && !errors.IsNotFound(err) {
86+
return err
87+
}
88+
if !errors.IsNotFound(err) {
89+
lbURL, err := url.Parse(infrastructureConfig.Status.APIServerInternalURL)
90+
if err != nil {
91+
return err
92+
}
93+
// if we have any error and have empty strings, substitution below will do nothing and leave the manifest specified value
94+
// errors can happen when the port is not specified, in which case we have a host and we write that into the env vars
95+
lbHost, lbPort, err := net.SplitHostPort(lbURL.Host)
96+
if err != nil {
97+
if strings.Contains(err.Error(), "missing port in address") {
98+
lbHost = lbURL.Host
99+
lbPort = ""
100+
} else {
101+
return err
102+
}
103+
}
104+
err = updatePodSpecWithInternalLoadBalancerKubeService(
105+
&deployment.Spec.Template.Spec,
106+
[]string{"cluster-version-operator"},
107+
lbHost,
108+
lbPort,
109+
)
110+
if err != nil {
111+
return err
112+
}
113+
}
114+
}
115+
75116
_, updated, err := resourceapply.ApplyDeployment(b.client, deployment)
76117
if err != nil {
77118
return err

‎lib/resourcebuilder/podspec.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,78 @@ func updatePodSpecWithProxy(podSpec *corev1.PodSpec, containerNames []string, ht
4545
return nil
4646

4747
}
48+
49+
// updatePodSpecWithInternalLoadBalancerKubeService mutates the input podspec by setting the KUBERNETES_SERVICE_HOST to the internal
50+
// loadbalancer endpoint and the KUBERNETES_SERVICE_PORT to the specified port
51+
func updatePodSpecWithInternalLoadBalancerKubeService(podSpec *corev1.PodSpec, containerNames []string, internalLoadBalancerHost, internalLoadBalancerPort string) error {
52+
hasInternalLoadBalancer := len(internalLoadBalancerHost) > 0
53+
if !hasInternalLoadBalancer {
54+
return nil
55+
}
56+
57+
for _, containerName := range containerNames {
58+
found := false
59+
for i := range podSpec.Containers {
60+
if podSpec.Containers[i].Name != containerName {
61+
continue
62+
}
63+
found = true
64+
65+
podSpec.Containers[i].Env = setKubeServiceValue(podSpec.Containers[i].Env, internalLoadBalancerHost, internalLoadBalancerPort)
66+
}
67+
for i := range podSpec.InitContainers {
68+
if podSpec.InitContainers[i].Name != containerName {
69+
continue
70+
}
71+
found = true
72+
73+
podSpec.InitContainers[i].Env = setKubeServiceValue(podSpec.Containers[i].Env, internalLoadBalancerHost, internalLoadBalancerPort)
74+
}
75+
76+
if !found {
77+
return fmt.Errorf("requested injection for non-existent container: %q", containerName)
78+
}
79+
}
80+
81+
return nil
82+
}
83+
84+
// setKubeServiceValue replaces values if they are present and adds them if they are not
85+
func setKubeServiceValue(in []corev1.EnvVar, internalLoadBalancerHost, internalLoadBalancerPort string) []corev1.EnvVar {
86+
ret := []corev1.EnvVar{}
87+
88+
portVal := "443"
89+
if len(internalLoadBalancerPort) != 0 {
90+
portVal = internalLoadBalancerPort
91+
}
92+
93+
foundPort := false
94+
foundHost := false
95+
for j := range in {
96+
ret = append(ret, *in[j].DeepCopy())
97+
if ret[j].Name == "KUBERNETES_SERVICE_PORT" {
98+
foundPort = true
99+
ret[j].Value = portVal
100+
}
101+
if ret[j].Name == "KUBERNETES_SERVICE_HOST" {
102+
foundHost = true
103+
ret[j].Value = internalLoadBalancerHost
104+
}
105+
}
106+
107+
if !foundPort {
108+
ret = append(ret, corev1.EnvVar{
109+
Name: "KUBERNETES_SERVICE_PORT",
110+
Value: portVal,
111+
})
112+
}
113+
114+
if !foundHost {
115+
ret = append(ret, corev1.EnvVar{
116+
Name: "KUBERNETES_SERVICE_HOST",
117+
Value: internalLoadBalancerHost,
118+
})
119+
}
120+
121+
return ret
122+
}

‎lib/resourcebuilder/podspec_test.go

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,159 @@ func TestUpdatePodSpecWithProxy(t *testing.T) {
110110
})
111111
}
112112
}
113+
114+
func TestUpdatePodSpecWithInternalLoadBalancerKubeService(t *testing.T) {
115+
tests := []struct {
116+
name string
117+
118+
input *corev1.PodSpec
119+
containerNames []string
120+
lbHost, lbPort string
121+
122+
expectedErr string
123+
expected *corev1.PodSpec
124+
}{
125+
{
126+
name: "no lbhost val",
127+
input: &corev1.PodSpec{
128+
Containers: []corev1.Container{
129+
{
130+
Name: "foo",
131+
},
132+
},
133+
},
134+
expected: &corev1.PodSpec{
135+
Containers: []corev1.Container{
136+
{
137+
Name: "foo",
138+
},
139+
},
140+
},
141+
},
142+
{
143+
name: "host and port, add to container and mutate",
144+
containerNames: []string{"foo", "init-foo"},
145+
lbHost: "lbhost-val",
146+
lbPort: "lbport-val",
147+
input: &corev1.PodSpec{
148+
InitContainers: []corev1.Container{
149+
{
150+
Name: "init-foo",
151+
Env: []corev1.EnvVar{
152+
{Name: "KUBERNETES_SERVICE_PORT", Value: "oldport-val"},
153+
{Name: "KUBERNETES_SERVICE_HOST", Value: "oldhost-val"},
154+
},
155+
},
156+
{
157+
Name: "init-bar",
158+
},
159+
},
160+
Containers: []corev1.Container{
161+
{
162+
Name: "foo",
163+
},
164+
{
165+
Name: "bar",
166+
},
167+
},
168+
},
169+
expected: &corev1.PodSpec{
170+
InitContainers: []corev1.Container{
171+
{
172+
Name: "init-foo",
173+
Env: []corev1.EnvVar{
174+
{Name: "KUBERNETES_SERVICE_PORT", Value: "lbport-val"},
175+
{Name: "KUBERNETES_SERVICE_HOST", Value: "lbhost-val"},
176+
},
177+
},
178+
{
179+
Name: "init-bar",
180+
},
181+
},
182+
Containers: []corev1.Container{
183+
{
184+
Name: "foo",
185+
Env: []corev1.EnvVar{
186+
{Name: "KUBERNETES_SERVICE_PORT", Value: "lbport-val"},
187+
{Name: "KUBERNETES_SERVICE_HOST", Value: "lbhost-val"},
188+
},
189+
},
190+
{
191+
Name: "bar",
192+
},
193+
},
194+
},
195+
},
196+
{
197+
name: "lbhost only, add to container and mutate",
198+
containerNames: []string{"foo", "init-foo"},
199+
lbHost: "lbhost-val",
200+
input: &corev1.PodSpec{
201+
InitContainers: []corev1.Container{
202+
{
203+
Name: "init-foo",
204+
Env: []corev1.EnvVar{
205+
{Name: "KUBERNETES_SERVICE_PORT", Value: "oldport-val"},
206+
{Name: "KUBERNETES_SERVICE_HOST", Value: "oldhost-val"},
207+
}},
208+
{
209+
Name: "init-bar",
210+
},
211+
},
212+
Containers: []corev1.Container{
213+
{
214+
Name: "foo",
215+
},
216+
{
217+
Name: "bar",
218+
},
219+
},
220+
},
221+
expected: &corev1.PodSpec{
222+
InitContainers: []corev1.Container{
223+
{
224+
Name: "init-foo",
225+
Env: []corev1.EnvVar{
226+
{Name: "KUBERNETES_SERVICE_PORT", Value: "443"},
227+
{Name: "KUBERNETES_SERVICE_HOST", Value: "lbhost-val"},
228+
},
229+
},
230+
{
231+
Name: "init-bar",
232+
},
233+
},
234+
Containers: []corev1.Container{
235+
{
236+
Name: "foo",
237+
Env: []corev1.EnvVar{
238+
{Name: "KUBERNETES_SERVICE_PORT", Value: "443"},
239+
{Name: "KUBERNETES_SERVICE_HOST", Value: "lbhost-val"},
240+
},
241+
},
242+
{
243+
Name: "bar",
244+
},
245+
},
246+
},
247+
},
248+
}
249+
250+
for _, test := range tests {
251+
t.Run(test.name, func(t *testing.T) {
252+
err := updatePodSpecWithInternalLoadBalancerKubeService(test.input, test.containerNames, test.lbHost, test.lbPort)
253+
switch {
254+
case err == nil && len(test.expectedErr) == 0:
255+
case err != nil && len(test.expectedErr) == 0:
256+
t.Fatal(err)
257+
case err == nil && len(test.expectedErr) != 0:
258+
t.Fatal(err)
259+
case err != nil && len(test.expectedErr) != 0 && err.Error() != test.expectedErr:
260+
t.Fatal(err)
261+
}
262+
263+
if !reflect.DeepEqual(test.input, test.expected) {
264+
t.Error(diff.ObjectDiff(test.input, test.expected))
265+
}
266+
})
267+
}
268+
}

0 commit comments

Comments
 (0)