Skip to content

Commit 139b350

Browse files
HoriodinoHoriodino
authored
fixes #1258 controller updates deployment when headless boolean is up… (#102)
* fixes #1258 controller updates deployment when headless boolean is updated Signed-off-by: Horiodino <holiodin@gmail.com> * added test case Signed-off-by: Horiodino <holiodin@gmail.com> * fixed:spinning up or removing the viewer as needed, without tracking previous values based on headless Signed-off-by: Horiodino <holiodin@gmail.com> * update go.mod Signed-off-by: Horiodino <holiodin@gmail.com> * add constant viewerContainerName for registry viewer container Signed-off-by: Horiodino <holiodin@gmail.com> --------- Signed-off-by: Horiodino <holiodin@gmail.com>
1 parent 383dd6b commit 139b350

File tree

2 files changed

+327
-0
lines changed

2 files changed

+327
-0
lines changed

‎controllers/update.go

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controllers
1919
import (
2020
"context"
2121
"fmt"
22+
"strconv"
2223

2324
registryv1alpha1 "github.com/devfile/registry-operator/api/v1alpha1"
2425
"github.com/devfile/registry-operator/pkg/registry"
@@ -27,10 +28,14 @@ import (
2728
corev1 "k8s.io/api/core/v1"
2829
networkingv1 "k8s.io/api/networking/v1"
2930
"k8s.io/apimachinery/pkg/api/errors"
31+
"k8s.io/apimachinery/pkg/api/resource"
3032
"k8s.io/apimachinery/pkg/types"
33+
"k8s.io/apimachinery/pkg/util/intstr"
3134
"sigs.k8s.io/controller-runtime/pkg/client"
3235
)
3336

37+
const viewerContainerName = "registry-viewer"
38+
3439
// updateDeployment ensures that a devfile registry deployment exists on the cluster and is up to date with the custom resource
3540
func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, dep *appsv1.Deployment) error {
3641
// Check to see if the existing devfile registry deployment needs to be updated
@@ -73,6 +78,14 @@ func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *re
7378
}
7479
}
7580

81+
updated, err := r.updateDeploymentForHeadlessChange(cr, dep)
82+
if err != nil {
83+
return err
84+
}
85+
if updated {
86+
needsUpdating = true
87+
}
88+
7689
if registry.IsStorageEnabled(cr) {
7790
if dep.Spec.Template.Spec.Volumes[0].PersistentVolumeClaim == nil {
7891
dep.Spec.Template.Spec.Volumes[0].VolumeSource = registry.GetDevfileRegistryVolumeSource(cr)
@@ -219,3 +232,174 @@ func (r *DevfileRegistryReconciler) deleteOldPVCIfNeeded(ctx context.Context, cr
219232
}
220233
return nil
221234
}
235+
236+
// updateRegistryHeadlessEnv updates or adds the REGISTRY_HEADLESS environment variable
237+
func updateRegistryHeadlessEnv(envVars []corev1.EnvVar, headless bool) []corev1.EnvVar {
238+
found := false
239+
for i, env := range envVars {
240+
if env.Name == "REGISTRY_HEADLESS" {
241+
envVars[i].Value = strconv.FormatBool(headless)
242+
found = true
243+
break
244+
}
245+
}
246+
if !found {
247+
envVars = append(envVars, corev1.EnvVar{
248+
Name: "REGISTRY_HEADLESS",
249+
Value: strconv.FormatBool(headless),
250+
})
251+
}
252+
return envVars
253+
}
254+
255+
// removeViewerContainer removes the registry-viewer container from the list of containers
256+
func removeViewerContainer(containers []corev1.Container) []corev1.Container {
257+
var newContainers []corev1.Container
258+
for _, container := range containers {
259+
if container.Name != viewerContainerName {
260+
newContainers = append(newContainers, container)
261+
}
262+
}
263+
return newContainers
264+
}
265+
266+
// updateDeploymentForHeadlessChange updates the deployment based on headless configuration
267+
func (r *DevfileRegistryReconciler) updateDeploymentForHeadlessChange(cr *registryv1alpha1.DevfileRegistry, dep *appsv1.Deployment) (bool, error) {
268+
updated := false
269+
allowPrivilegeEscalation := false
270+
runAsNonRoot := true
271+
localHostname := "localhost"
272+
273+
if !registry.IsHeadlessEnabled(cr) {
274+
// Check if viewer container already exists before adding
275+
viewerExists := false
276+
for _, container := range dep.Spec.Template.Spec.Containers {
277+
if container.Name == viewerContainerName {
278+
viewerExists = true
279+
break
280+
}
281+
}
282+
283+
if !viewerExists {
284+
// Configure StartupProbe
285+
dep.Spec.Template.Spec.Containers[0].StartupProbe = &corev1.Probe{
286+
ProbeHandler: corev1.ProbeHandler{
287+
HTTPGet: &corev1.HTTPGetAction{
288+
Path: "/viewer",
289+
Port: intstr.FromInt(registry.RegistryViewerPort),
290+
Scheme: corev1.URISchemeHTTP,
291+
},
292+
},
293+
InitialDelaySeconds: 30,
294+
PeriodSeconds: 10,
295+
TimeoutSeconds: 20,
296+
}
297+
298+
// Append registry-viewer container
299+
dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, corev1.Container{
300+
Image: registry.GetRegistryViewerImage(cr),
301+
ImagePullPolicy: registry.GetRegistryViewerImagePullPolicy(cr),
302+
Name: viewerContainerName,
303+
SecurityContext: &corev1.SecurityContext{
304+
AllowPrivilegeEscalation: &allowPrivilegeEscalation,
305+
RunAsNonRoot: &runAsNonRoot,
306+
Capabilities: &corev1.Capabilities{
307+
Drop: []corev1.Capability{"ALL"},
308+
},
309+
SeccompProfile: &corev1.SeccompProfile{
310+
Type: "RuntimeDefault",
311+
},
312+
},
313+
Resources: corev1.ResourceRequirements{
314+
Requests: corev1.ResourceList{
315+
corev1.ResourceCPU: resource.MustParse("250m"),
316+
corev1.ResourceMemory: resource.MustParse("64Mi"),
317+
},
318+
Limits: corev1.ResourceList{
319+
corev1.ResourceCPU: resource.MustParse("500m"),
320+
corev1.ResourceMemory: resource.MustParse("256Mi"),
321+
},
322+
},
323+
LivenessProbe: &corev1.Probe{
324+
ProbeHandler: corev1.ProbeHandler{
325+
HTTPGet: &corev1.HTTPGetAction{
326+
Path: "/viewer",
327+
Port: intstr.FromInt(registry.RegistryViewerPort),
328+
Scheme: corev1.URISchemeHTTP,
329+
},
330+
},
331+
InitialDelaySeconds: 15,
332+
PeriodSeconds: 10,
333+
TimeoutSeconds: 20,
334+
},
335+
ReadinessProbe: &corev1.Probe{
336+
ProbeHandler: corev1.ProbeHandler{
337+
HTTPGet: &corev1.HTTPGetAction{
338+
Path: "/viewer",
339+
Port: intstr.FromInt(registry.RegistryViewerPort),
340+
Scheme: corev1.URISchemeHTTP,
341+
},
342+
},
343+
InitialDelaySeconds: 15,
344+
PeriodSeconds: 10,
345+
TimeoutSeconds: 20,
346+
},
347+
Env: []corev1.EnvVar{
348+
{
349+
Name: "NEXT_PUBLIC_ANALYTICS_WRITE_KEY",
350+
Value: cr.Spec.Telemetry.RegistryViewerWriteKey,
351+
},
352+
{
353+
Name: "DEVFILE_REGISTRIES",
354+
Value: fmt.Sprintf(`[
355+
{
356+
"name": "%s",
357+
"url": "http://%s",
358+
"fqdn": "%s"
359+
}
360+
]`, cr.ObjectMeta.Name, localHostname, cr.Status.URL),
361+
},
362+
},
363+
})
364+
updated = true
365+
}
366+
} else {
367+
// Check if REGISTRY_HEADLESS env var needs to be updated
368+
headlessEnvNeedsUpdate := true
369+
for _, env := range dep.Spec.Template.Spec.Containers[0].Env {
370+
if env.Name == "REGISTRY_HEADLESS" && env.Value == strconv.FormatBool(true) {
371+
headlessEnvNeedsUpdate = false
372+
break
373+
}
374+
}
375+
376+
// Check if viewer container needs to be removed
377+
viewerExists := false
378+
for _, container := range dep.Spec.Template.Spec.Containers {
379+
if container.Name == viewerContainerName {
380+
viewerExists = true
381+
break
382+
}
383+
}
384+
385+
if headlessEnvNeedsUpdate || viewerExists {
386+
// Set REGISTRY_HEADLESS environment variable
387+
dep.Spec.Template.Spec.Containers[0].Env = updateRegistryHeadlessEnv(
388+
dep.Spec.Template.Spec.Containers[0].Env,
389+
true,
390+
)
391+
392+
// Remove viewer container
393+
dep.Spec.Template.Spec.Containers = removeViewerContainer(
394+
dep.Spec.Template.Spec.Containers,
395+
)
396+
397+
// Clear startup probe
398+
dep.Spec.Template.Spec.Containers[0].StartupProbe = nil
399+
400+
updated = true
401+
}
402+
}
403+
404+
return updated, nil
405+
}

‎controllers/update_test.go

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
//
2+
//
3+
// Copyright Red Hat
4+
//
5+
// Licensed under the Apache License, Version 2.0 (the "License");
6+
// you may not use this file except in compliance with the License.
7+
// You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing, software
12+
// distributed under the License is distributed on an "AS IS" BASIS,
13+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
// See the License for the specific language governing permissions and
15+
// limitations under the License.
16+
17+
package controllers
18+
19+
import (
20+
"testing"
21+
22+
registryv1alpha1 "github.com/devfile/registry-operator/api/v1alpha1"
23+
appsv1 "k8s.io/api/apps/v1"
24+
corev1 "k8s.io/api/core/v1"
25+
)
26+
27+
func TestUpdateDeploymentForHeadlessChange(t *testing.T) {
28+
r := &DevfileRegistryReconciler{}
29+
30+
tests := []struct {
31+
name string
32+
cr *registryv1alpha1.DevfileRegistry
33+
dep *appsv1.Deployment
34+
want bool
35+
wantErr bool
36+
}{
37+
{
38+
name: "Headless true - REGISTRY_HEADLESS set correctly, viewer not present",
39+
cr: &registryv1alpha1.DevfileRegistry{
40+
Spec: registryv1alpha1.DevfileRegistrySpec{
41+
Headless: func(b bool) *bool { return &b }(true),
42+
},
43+
},
44+
dep: func() *appsv1.Deployment {
45+
dep := &appsv1.Deployment{}
46+
dep.Spec.Template.Spec.Containers = []corev1.Container{
47+
{
48+
Name: "devfile-registry",
49+
Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "true"}},
50+
},
51+
}
52+
return dep
53+
}(),
54+
want: false, // No changes needed, already correct
55+
wantErr: false,
56+
},
57+
{
58+
name: "Headless true - REGISTRY_HEADLESS incorrect, viewer present",
59+
cr: &registryv1alpha1.DevfileRegistry{
60+
Spec: registryv1alpha1.DevfileRegistrySpec{
61+
Headless: func(b bool) *bool { return &b }(true),
62+
},
63+
},
64+
dep: func() *appsv1.Deployment {
65+
dep := &appsv1.Deployment{}
66+
dep.Spec.Template.Spec.Containers = []corev1.Container{
67+
{
68+
Name: "devfile-registry",
69+
Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}},
70+
},
71+
{
72+
Name: viewerContainerName,
73+
},
74+
}
75+
return dep
76+
}(),
77+
want: true, // Changes required: update ENV and remove viewer
78+
wantErr: false,
79+
},
80+
{
81+
name: "Headless false - REGISTRY_HEADLESS set correctly, viewer present",
82+
cr: &registryv1alpha1.DevfileRegistry{
83+
Spec: registryv1alpha1.DevfileRegistrySpec{
84+
Headless: func(b bool) *bool { return &b }(false),
85+
},
86+
},
87+
dep: func() *appsv1.Deployment {
88+
dep := &appsv1.Deployment{}
89+
dep.Spec.Template.Spec.Containers = []corev1.Container{
90+
{
91+
Name: "devfile-registry",
92+
Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}},
93+
},
94+
{
95+
Name: viewerContainerName,
96+
},
97+
}
98+
return dep
99+
}(),
100+
want: false, // No changes needed, already correct
101+
wantErr: false,
102+
},
103+
{
104+
name: "Headless false - REGISTRY_HEADLESS incorrect, viewer missing",
105+
cr: &registryv1alpha1.DevfileRegistry{
106+
Spec: registryv1alpha1.DevfileRegistrySpec{
107+
Headless: func(b bool) *bool { return &b }(false),
108+
},
109+
},
110+
dep: func() *appsv1.Deployment {
111+
dep := &appsv1.Deployment{}
112+
dep.Spec.Template.Spec.Containers = []corev1.Container{
113+
{
114+
Name: "devfile-registry",
115+
Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "true"}},
116+
},
117+
}
118+
return dep
119+
}(),
120+
want: true, // Changes required: update ENV and add viewer
121+
wantErr: false,
122+
},
123+
}
124+
125+
for _, tt := range tests {
126+
t.Run(tt.name, func(t *testing.T) {
127+
// Create a copy of the original cr to check if it's modified
128+
crCopy := tt.cr.DeepCopy()
129+
130+
// Call the method and check for errors
131+
value, err := r.updateDeploymentForHeadlessChange(crCopy, tt.dep)
132+
if (err != nil) != tt.wantErr {
133+
t.Errorf("updateDeploymentForHeadlessChange() error = %v, wantErr %v", err, tt.wantErr)
134+
return
135+
}
136+
137+
// Compare the return value with the expected value
138+
if value != tt.want {
139+
t.Errorf("updateDeploymentForHeadlessChange() = %v, want %v", value, tt.want)
140+
}
141+
})
142+
}
143+
}

0 commit comments

Comments
 (0)