From be7e721c154b988138f1d6e02005bc5b97189c7b Mon Sep 17 00:00:00 2001 From: Horiodino Date: Mon, 11 Nov 2024 21:03:29 +0530 Subject: [PATCH 1/5] fixes #1258 controller updates deployment when headless boolean is updated Signed-off-by: Horiodino --- controllers/update.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/controllers/update.go b/controllers/update.go index 0d13a30..79ebd9c 100644 --- a/controllers/update.go +++ b/controllers/update.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "strconv" registryv1alpha1 "github.com/devfile/registry-operator/api/v1alpha1" "github.com/devfile/registry-operator/pkg/registry" @@ -84,6 +85,19 @@ func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *re needsUpdating = true } } + headlessStatus := dep.Spec.Template.Spec.Containers[0].Env + + for _, env := range headlessStatus { + if env.Name == "REGISTRY_HEADLESS" { + value, err := strconv.ParseBool(env.Value) + if err != nil { + return err + } + if *cr.Spec.Headless != value { + needsUpdating = true + } + } + } if len(dep.Spec.Template.Spec.Containers) > 2 { viewerImage := registry.GetRegistryViewerImage(cr) From d440dd80529a4133824ab8359d7c207a1d212088 Mon Sep 17 00:00:00 2001 From: Horiodino Date: Sun, 24 Nov 2024 23:55:18 +0530 Subject: [PATCH 2/5] added test case Signed-off-by: Horiodino --- controllers/update.go | 50 ++++++++++++++------ controllers/update_test.go | 95 ++++++++++++++++++++++++++++++++++++++ go.mod | 3 +- 3 files changed, 134 insertions(+), 14 deletions(-) create mode 100644 controllers/update_test.go diff --git a/controllers/update.go b/controllers/update.go index 79ebd9c..9fcf025 100644 --- a/controllers/update.go +++ b/controllers/update.go @@ -74,6 +74,14 @@ func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *re } } + headlessStatusOutdated, err := r.isHeadlessStatusOutdated(cr, dep) + if err != nil { + return err + } + if headlessStatusOutdated { + needsUpdating = true + } + if registry.IsStorageEnabled(cr) { if dep.Spec.Template.Spec.Volumes[0].PersistentVolumeClaim == nil { dep.Spec.Template.Spec.Volumes[0].VolumeSource = registry.GetDevfileRegistryVolumeSource(cr) @@ -85,19 +93,6 @@ func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *re needsUpdating = true } } - headlessStatus := dep.Spec.Template.Spec.Containers[0].Env - - for _, env := range headlessStatus { - if env.Name == "REGISTRY_HEADLESS" { - value, err := strconv.ParseBool(env.Value) - if err != nil { - return err - } - if *cr.Spec.Headless != value { - needsUpdating = true - } - } - } if len(dep.Spec.Template.Spec.Containers) > 2 { viewerImage := registry.GetRegistryViewerImage(cr) @@ -233,3 +228,32 @@ func (r *DevfileRegistryReconciler) deleteOldPVCIfNeeded(ctx context.Context, cr } return nil } + +func (r *DevfileRegistryReconciler) isHeadlessStatusOutdated(cr *registryv1alpha1.DevfileRegistry, dep *appsv1.Deployment) (bool, error) { + var existingHeadless bool + found := false + + for _, env := range dep.Spec.Template.Spec.Containers[0].Env { + if env.Name == "REGISTRY_HEADLESS" { + var err error + existingHeadless, err = strconv.ParseBool(env.Value) + if err != nil { + return false, fmt.Errorf("error parsing REGISTRY_HEADLESS value: %w", err) + } + found = true + break + } + } + + if !found { + existingHeadless = false + } + + expectedHeadless := registry.IsHeadlessEnabled(cr) + + if existingHeadless != expectedHeadless { + return true, nil + } + + return false, nil +} diff --git a/controllers/update_test.go b/controllers/update_test.go new file mode 100644 index 0000000..5d64e03 --- /dev/null +++ b/controllers/update_test.go @@ -0,0 +1,95 @@ +package controllers + +import ( + "testing" + + registryv1alpha1 "github.com/devfile/registry-operator/api/v1alpha1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" +) + +func TestHeadlessStatusOutdated(t *testing.T) { + var r *DevfileRegistryReconciler + var cr *registryv1alpha1.DevfileRegistry + + r = &DevfileRegistryReconciler{} + cr = ®istryv1alpha1.DevfileRegistry{ + Spec: registryv1alpha1.DevfileRegistrySpec{ + Headless: func(b bool) *bool { return &b }(true), + }, + } + + type args struct { + cr *registryv1alpha1.DevfileRegistry + dep *appsv1.Deployment + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Cr headless true and deployment env REGISTRY_HEADLESS true", + args: args{ + cr: cr, + dep: func() *appsv1.Deployment { + dep := &appsv1.Deployment{} + dep.Spec.Template.Spec.Containers = []corev1.Container{ + { + Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "true"}}, + }, + } + return dep + }(), + }, + want: false, + }, + { + name: "Cr headless true and deployment env REGISTRY_HEADLESS false", + args: args{ + cr: ®istryv1alpha1.DevfileRegistry{ + Spec: registryv1alpha1.DevfileRegistrySpec{ + Headless: func(b bool) *bool { return &b }(true), + }, + }, + dep: func() *appsv1.Deployment { + dep := &appsv1.Deployment{} + dep.Spec.Template.Spec.Containers = []corev1.Container{ + { + Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}}, + }, + } + return dep + }(), + }, + want: true, + }, + { + name: "Cr headless false and deployment env REGISTRY_HEADLESS false", + args: args{ + cr: ®istryv1alpha1.DevfileRegistry{ + Spec: registryv1alpha1.DevfileRegistrySpec{ + Headless: func(b bool) *bool { return &b }(false), + }, + }, + dep: func() *appsv1.Deployment { + dep := &appsv1.Deployment{} + dep.Spec.Template.Spec.Containers = []corev1.Container{ + { + Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}}, + }, + } + return dep + }(), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got, _ := r.isHeadlessStatusOutdated(tt.args.cr, tt.args.dep); got != tt.want { + t.Errorf("isHeadlessStatusOutdated() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/go.mod b/go.mod index 5a550d1..9048ee6 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( k8s.io/apiextensions-apiserver v0.29.2 k8s.io/apimachinery v0.29.2 k8s.io/client-go v0.29.2 + k8s.io/utils v0.0.0-20230726121419-3b25d923346b sigs.k8s.io/controller-runtime v0.17.5 ) @@ -35,6 +36,7 @@ require ( github.com/docker/go-connections v0.5.0 // indirect github.com/docker/go-metrics v0.0.1 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect + github.com/evanphx/json-patch v4.12.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.8.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect @@ -100,7 +102,6 @@ require ( k8s.io/component-base v0.29.2 // indirect k8s.io/klog/v2 v2.110.1 // indirect k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect - k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect oras.land/oras-go v1.2.5 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect From 5d995abed5626b47377f805e9e2c52795fc0a93b Mon Sep 17 00:00:00 2001 From: Horiodino Date: Sat, 7 Dec 2024 23:31:56 +0530 Subject: [PATCH 3/5] fixed:spinning up or removing the viewer as needed, without tracking previous values based on headless Signed-off-by: Horiodino --- controllers/update.go | 178 +++++++++++++++++++++++++++++++++---- controllers/update_test.go | 158 +++++++++++++++++++------------- 2 files changed, 256 insertions(+), 80 deletions(-) diff --git a/controllers/update.go b/controllers/update.go index 9fcf025..35c5b0a 100644 --- a/controllers/update.go +++ b/controllers/update.go @@ -28,7 +28,9 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -74,11 +76,11 @@ func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *re } } - headlessStatusOutdated, err := r.isHeadlessStatusOutdated(cr, dep) + updated, err := r.updateDeploymentForHeadlessChange(cr, dep) if err != nil { return err } - if headlessStatusOutdated { + if updated { needsUpdating = true } @@ -229,31 +231,173 @@ func (r *DevfileRegistryReconciler) deleteOldPVCIfNeeded(ctx context.Context, cr return nil } -func (r *DevfileRegistryReconciler) isHeadlessStatusOutdated(cr *registryv1alpha1.DevfileRegistry, dep *appsv1.Deployment) (bool, error) { - var existingHeadless bool +// updateRegistryHeadlessEnv updates or adds the REGISTRY_HEADLESS environment variable +func updateRegistryHeadlessEnv(envVars []corev1.EnvVar, headless bool) []corev1.EnvVar { found := false - - for _, env := range dep.Spec.Template.Spec.Containers[0].Env { + for i, env := range envVars { if env.Name == "REGISTRY_HEADLESS" { - var err error - existingHeadless, err = strconv.ParseBool(env.Value) - if err != nil { - return false, fmt.Errorf("error parsing REGISTRY_HEADLESS value: %w", err) - } + envVars[i].Value = strconv.FormatBool(headless) found = true break } } - if !found { - existingHeadless = false + envVars = append(envVars, corev1.EnvVar{ + Name: "REGISTRY_HEADLESS", + Value: strconv.FormatBool(headless), + }) + } + return envVars +} + +// removeViewerContainer removes the registry-viewer container from the list of containers +func removeViewerContainer(containers []corev1.Container) []corev1.Container { + var newContainers []corev1.Container + for _, container := range containers { + if container.Name != "registry-viewer" { + newContainers = append(newContainers, container) + } } + return newContainers +} + +// updateDeploymentForHeadlessChange updates the deployment based on headless configuration +func (r *DevfileRegistryReconciler) updateDeploymentForHeadlessChange(cr *registryv1alpha1.DevfileRegistry, dep *appsv1.Deployment) (bool, error) { + updated := false + allowPrivilegeEscalation := false + runAsNonRoot := true + localHostname := "localhost" + + if !registry.IsHeadlessEnabled(cr) { + // Check if viewer container already exists before adding + viewerExists := false + for _, container := range dep.Spec.Template.Spec.Containers { + if container.Name == "registry-viewer" { + viewerExists = true + break + } + } + + if !viewerExists { + // Configure StartupProbe + dep.Spec.Template.Spec.Containers[0].StartupProbe = &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/viewer", + Port: intstr.FromInt(registry.RegistryViewerPort), + Scheme: corev1.URISchemeHTTP, + }, + }, + InitialDelaySeconds: 30, + PeriodSeconds: 10, + TimeoutSeconds: 20, + } - expectedHeadless := registry.IsHeadlessEnabled(cr) + // Append registry-viewer container + dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, corev1.Container{ + Image: registry.GetRegistryViewerImage(cr), + ImagePullPolicy: registry.GetRegistryViewerImagePullPolicy(cr), + Name: "registry-viewer", + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: &allowPrivilegeEscalation, + RunAsNonRoot: &runAsNonRoot, + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + SeccompProfile: &corev1.SeccompProfile{ + Type: "RuntimeDefault", + }, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("250m"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("500m"), + corev1.ResourceMemory: resource.MustParse("256Mi"), + }, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/viewer", + Port: intstr.FromInt(registry.RegistryViewerPort), + Scheme: corev1.URISchemeHTTP, + }, + }, + InitialDelaySeconds: 15, + PeriodSeconds: 10, + TimeoutSeconds: 20, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/viewer", + Port: intstr.FromInt(registry.RegistryViewerPort), + Scheme: corev1.URISchemeHTTP, + }, + }, + InitialDelaySeconds: 15, + PeriodSeconds: 10, + TimeoutSeconds: 20, + }, + Env: []corev1.EnvVar{ + { + Name: "NEXT_PUBLIC_ANALYTICS_WRITE_KEY", + Value: cr.Spec.Telemetry.RegistryViewerWriteKey, + }, + { + Name: "DEVFILE_REGISTRIES", + Value: fmt.Sprintf(`[ + { + "name": "%s", + "url": "http://%s", + "fqdn": "%s" + } + ]`, cr.ObjectMeta.Name, localHostname, cr.Status.URL), + }, + }, + }) + updated = true + } + } else { + // Check if REGISTRY_HEADLESS env var needs to be updated + headlessEnvNeedsUpdate := true + for _, env := range dep.Spec.Template.Spec.Containers[0].Env { + if env.Name == "REGISTRY_HEADLESS" && env.Value == strconv.FormatBool(true) { + headlessEnvNeedsUpdate = false + break + } + } + + // Check if viewer container needs to be removed + viewerExists := false + for _, container := range dep.Spec.Template.Spec.Containers { + if container.Name == "registry-viewer" { + viewerExists = true + break + } + } - if existingHeadless != expectedHeadless { - return true, nil + if headlessEnvNeedsUpdate || viewerExists { + // Set REGISTRY_HEADLESS environment variable + dep.Spec.Template.Spec.Containers[0].Env = updateRegistryHeadlessEnv( + dep.Spec.Template.Spec.Containers[0].Env, + true, + ) + + // Remove viewer container + dep.Spec.Template.Spec.Containers = removeViewerContainer( + dep.Spec.Template.Spec.Containers, + ) + + // Clear startup probe + dep.Spec.Template.Spec.Containers[0].StartupProbe = nil + + updated = true + } } - return false, nil + return updated, nil } diff --git a/controllers/update_test.go b/controllers/update_test.go index 5d64e03..890d6f6 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -8,87 +8,119 @@ import ( corev1 "k8s.io/api/core/v1" ) -func TestHeadlessStatusOutdated(t *testing.T) { - var r *DevfileRegistryReconciler - var cr *registryv1alpha1.DevfileRegistry +func TestUpdateDeploymentForHeadlessChange(t *testing.T) { + r := &DevfileRegistryReconciler{} - r = &DevfileRegistryReconciler{} - cr = ®istryv1alpha1.DevfileRegistry{ - Spec: registryv1alpha1.DevfileRegistrySpec{ - Headless: func(b bool) *bool { return &b }(true), - }, - } - - type args struct { - cr *registryv1alpha1.DevfileRegistry - dep *appsv1.Deployment - } tests := []struct { - name string - args args - want bool + name string + cr *registryv1alpha1.DevfileRegistry + dep *appsv1.Deployment + want bool + wantErr bool }{ { - name: "Cr headless true and deployment env REGISTRY_HEADLESS true", - args: args{ - cr: cr, - dep: func() *appsv1.Deployment { - dep := &appsv1.Deployment{} - dep.Spec.Template.Spec.Containers = []corev1.Container{ - { - Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "true"}}, - }, - } - return dep - }(), + name: "Headless true - REGISTRY_HEADLESS set correctly, viewer not present", + cr: ®istryv1alpha1.DevfileRegistry{ + Spec: registryv1alpha1.DevfileRegistrySpec{ + Headless: func(b bool) *bool { return &b }(true), + }, }, - want: false, + dep: func() *appsv1.Deployment { + dep := &appsv1.Deployment{} + dep.Spec.Template.Spec.Containers = []corev1.Container{ + { + Name: "devfile-registry", + Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "true"}}, + }, + } + return dep + }(), + want: false, // No changes needed, already correct + wantErr: false, }, { - name: "Cr headless true and deployment env REGISTRY_HEADLESS false", - args: args{ - cr: ®istryv1alpha1.DevfileRegistry{ - Spec: registryv1alpha1.DevfileRegistrySpec{ - Headless: func(b bool) *bool { return &b }(true), - }, + name: "Headless true - REGISTRY_HEADLESS incorrect, viewer present", + cr: ®istryv1alpha1.DevfileRegistry{ + Spec: registryv1alpha1.DevfileRegistrySpec{ + Headless: func(b bool) *bool { return &b }(true), }, - dep: func() *appsv1.Deployment { - dep := &appsv1.Deployment{} - dep.Spec.Template.Spec.Containers = []corev1.Container{ - { - Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}}, - }, - } - return dep - }(), }, - want: true, + dep: func() *appsv1.Deployment { + dep := &appsv1.Deployment{} + dep.Spec.Template.Spec.Containers = []corev1.Container{ + { + Name: "devfile-registry", + Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}}, + }, + { + Name: "registry-viewer", + }, + } + return dep + }(), + want: true, // Changes required: update ENV and remove viewer + wantErr: false, }, { - name: "Cr headless false and deployment env REGISTRY_HEADLESS false", - args: args{ - cr: ®istryv1alpha1.DevfileRegistry{ - Spec: registryv1alpha1.DevfileRegistrySpec{ - Headless: func(b bool) *bool { return &b }(false), + name: "Headless false - REGISTRY_HEADLESS set correctly, viewer present", + cr: ®istryv1alpha1.DevfileRegistry{ + Spec: registryv1alpha1.DevfileRegistrySpec{ + Headless: func(b bool) *bool { return &b }(false), + }, + }, + dep: func() *appsv1.Deployment { + dep := &appsv1.Deployment{} + dep.Spec.Template.Spec.Containers = []corev1.Container{ + { + Name: "devfile-registry", + Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}}, + }, + { + Name: "registry-viewer", }, + } + return dep + }(), + want: false, // No changes needed, already correct + wantErr: false, + }, + { + name: "Headless false - REGISTRY_HEADLESS incorrect, viewer missing", + cr: ®istryv1alpha1.DevfileRegistry{ + Spec: registryv1alpha1.DevfileRegistrySpec{ + Headless: func(b bool) *bool { return &b }(false), }, - dep: func() *appsv1.Deployment { - dep := &appsv1.Deployment{} - dep.Spec.Template.Spec.Containers = []corev1.Container{ - { - Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}}, - }, - } - return dep - }(), }, - want: false, + dep: func() *appsv1.Deployment { + dep := &appsv1.Deployment{} + dep.Spec.Template.Spec.Containers = []corev1.Container{ + { + Name: "devfile-registry", + Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "true"}}, + }, + } + return dep + }(), + want: true, // Changes required: update ENV and add viewer + wantErr: false, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got, _ := r.isHeadlessStatusOutdated(tt.args.cr, tt.args.dep); got != tt.want { - t.Errorf("isHeadlessStatusOutdated() = %v, want %v", got, tt.want) + // Create a copy of the original cr to check if it's modified + crCopy := tt.cr.DeepCopy() + + // Call the method and check for errors + value, err := r.updateDeploymentForHeadlessChange(crCopy, tt.dep) + if (err != nil) != tt.wantErr { + t.Errorf("updateDeploymentForHeadlessChange() error = %v, wantErr %v", err, tt.wantErr) + return + } + + // Compare the return value with the expected value + if value != tt.want { + t.Errorf("updateDeploymentForHeadlessChange() = %v, want %v", value, tt.want) } }) } From 02e3de450b188c5eaf5f70f0b667f327ecc5f5b1 Mon Sep 17 00:00:00 2001 From: Horiodino Date: Wed, 11 Dec 2024 23:29:21 +0530 Subject: [PATCH 4/5] update go.mod Signed-off-by: Horiodino --- controllers/update_test.go | 16 ++++++++++++++++ go.mod | 3 +-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/controllers/update_test.go b/controllers/update_test.go index 890d6f6..481fd28 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -1,3 +1,19 @@ +// +// +// Copyright Red Hat +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package controllers import ( diff --git a/go.mod b/go.mod index 9048ee6..5a550d1 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,6 @@ require ( k8s.io/apiextensions-apiserver v0.29.2 k8s.io/apimachinery v0.29.2 k8s.io/client-go v0.29.2 - k8s.io/utils v0.0.0-20230726121419-3b25d923346b sigs.k8s.io/controller-runtime v0.17.5 ) @@ -36,7 +35,6 @@ require ( github.com/docker/go-connections v0.5.0 // indirect github.com/docker/go-metrics v0.0.1 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect - github.com/evanphx/json-patch v4.12.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.8.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect @@ -102,6 +100,7 @@ require ( k8s.io/component-base v0.29.2 // indirect k8s.io/klog/v2 v2.110.1 // indirect k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect + k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect oras.land/oras-go v1.2.5 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect From 4001349493438e2ba1ed03ab5678944d13653abe Mon Sep 17 00:00:00 2001 From: Horiodino Date: Thu, 19 Dec 2024 22:18:59 +0530 Subject: [PATCH 5/5] add constant viewerContainerName for registry viewer container Signed-off-by: Horiodino --- controllers/update.go | 10 ++++++---- controllers/update_test.go | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/controllers/update.go b/controllers/update.go index 35c5b0a..09ce710 100644 --- a/controllers/update.go +++ b/controllers/update.go @@ -34,6 +34,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +const viewerContainerName = "registry-viewer" + // updateDeployment ensures that a devfile registry deployment exists on the cluster and is up to date with the custom resource func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, dep *appsv1.Deployment) error { // Check to see if the existing devfile registry deployment needs to be updated @@ -254,7 +256,7 @@ func updateRegistryHeadlessEnv(envVars []corev1.EnvVar, headless bool) []corev1. func removeViewerContainer(containers []corev1.Container) []corev1.Container { var newContainers []corev1.Container for _, container := range containers { - if container.Name != "registry-viewer" { + if container.Name != viewerContainerName { newContainers = append(newContainers, container) } } @@ -272,7 +274,7 @@ func (r *DevfileRegistryReconciler) updateDeploymentForHeadlessChange(cr *regist // Check if viewer container already exists before adding viewerExists := false for _, container := range dep.Spec.Template.Spec.Containers { - if container.Name == "registry-viewer" { + if container.Name == viewerContainerName { viewerExists = true break } @@ -297,7 +299,7 @@ func (r *DevfileRegistryReconciler) updateDeploymentForHeadlessChange(cr *regist dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, corev1.Container{ Image: registry.GetRegistryViewerImage(cr), ImagePullPolicy: registry.GetRegistryViewerImagePullPolicy(cr), - Name: "registry-viewer", + Name: viewerContainerName, SecurityContext: &corev1.SecurityContext{ AllowPrivilegeEscalation: &allowPrivilegeEscalation, RunAsNonRoot: &runAsNonRoot, @@ -374,7 +376,7 @@ func (r *DevfileRegistryReconciler) updateDeploymentForHeadlessChange(cr *regist // Check if viewer container needs to be removed viewerExists := false for _, container := range dep.Spec.Template.Spec.Containers { - if container.Name == "registry-viewer" { + if container.Name == viewerContainerName { viewerExists = true break } diff --git a/controllers/update_test.go b/controllers/update_test.go index 481fd28..22bb9e3 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -69,7 +69,7 @@ func TestUpdateDeploymentForHeadlessChange(t *testing.T) { Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}}, }, { - Name: "registry-viewer", + Name: viewerContainerName, }, } return dep @@ -92,7 +92,7 @@ func TestUpdateDeploymentForHeadlessChange(t *testing.T) { Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}}, }, { - Name: "registry-viewer", + Name: viewerContainerName, }, } return dep