Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #1258 controller updates deployment when headless boolean is up… #102

Merged
merged 5 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 184 additions & 0 deletions controllers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -27,10 +28,14 @@ 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"
)

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
Expand Down Expand Up @@ -73,6 +78,14 @@ func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *re
}
}

updated, err := r.updateDeploymentForHeadlessChange(cr, dep)
if err != nil {
return err
}
if updated {
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)
Expand Down Expand Up @@ -219,3 +232,174 @@ func (r *DevfileRegistryReconciler) deleteOldPVCIfNeeded(ctx context.Context, cr
}
return nil
}

// updateRegistryHeadlessEnv updates or adds the REGISTRY_HEADLESS environment variable
func updateRegistryHeadlessEnv(envVars []corev1.EnvVar, headless bool) []corev1.EnvVar {
found := false
for i, env := range envVars {
if env.Name == "REGISTRY_HEADLESS" {
envVars[i].Value = strconv.FormatBool(headless)
found = true
break
}
}
if !found {
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 != viewerContainerName {
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 == viewerContainerName {
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,
}

// 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: viewerContainerName,
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 == viewerContainerName {
viewerExists = true
break
}
}

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 updated, nil
}
143 changes: 143 additions & 0 deletions controllers/update_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
//
//
// 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 (
"testing"

registryv1alpha1 "github.com/devfile/registry-operator/api/v1alpha1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
)

func TestUpdateDeploymentForHeadlessChange(t *testing.T) {
r := &DevfileRegistryReconciler{}

tests := []struct {
name string
cr *registryv1alpha1.DevfileRegistry
dep *appsv1.Deployment
want bool
wantErr bool
}{
{
name: "Headless true - REGISTRY_HEADLESS set correctly, viewer not present",
cr: &registryv1alpha1.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{
{
Name: "devfile-registry",
Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "true"}},
},
}
return dep
}(),
want: false, // No changes needed, already correct
wantErr: false,
},
{
name: "Headless true - REGISTRY_HEADLESS incorrect, viewer present",
cr: &registryv1alpha1.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{
{
Name: "devfile-registry",
Env: []corev1.EnvVar{{Name: "REGISTRY_HEADLESS", Value: "false"}},
},
{
Name: viewerContainerName,
},
}
return dep
}(),
want: true, // Changes required: update ENV and remove viewer
wantErr: false,
},
{
name: "Headless false - REGISTRY_HEADLESS set correctly, viewer present",
cr: &registryv1alpha1.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: viewerContainerName,
},
}
return dep
}(),
want: false, // No changes needed, already correct
wantErr: false,
},
{
name: "Headless false - REGISTRY_HEADLESS incorrect, viewer missing",
cr: &registryv1alpha1.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: "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) {
// 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)
}
})
}
}
Loading