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

overwrite only backstage and init-dynamic-plugins image. #295

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion controllers/backstage_controller_test.go
Expand Up @@ -1354,7 +1354,7 @@ spec:
}, time.Minute, time.Second).Should(Succeed())

By("Checking that the image was set on all containers in the Pod Spec")
model.VisitContainers(&found.Spec.Template.Spec, func(container *corev1.Container) {
VisitContainers(&found.Spec.Template.Spec, func(container *corev1.Container) {
By(fmt.Sprintf("Checking Image in the Backstage Deployment - container: %q", container.Name), func() {
Expect(container.Image).Should(Equal(imageName))
})
Expand Down Expand Up @@ -1572,3 +1572,19 @@ func getSecretName(containers []corev1.Container, name string, secretName string
}
return ""
}

// ContainerVisitor is called with each container
type ContainerVisitor func(container *corev1.Container)

// visitContainers invokes the visitor function for every container in the given pod template spec
func VisitContainers(podTemplateSpec *corev1.PodSpec, visitor ContainerVisitor) {
for i := range podTemplateSpec.InitContainers {
visitor(&podTemplateSpec.InitContainers[i])
}
for i := range podTemplateSpec.Containers {
visitor(&podTemplateSpec.Containers[i])
}
for i := range podTemplateSpec.EphemeralContainers {
visitor((*corev1.Container)(&podTemplateSpec.EphemeralContainers[i].EphemeralContainerCommon))
}
}
39 changes: 12 additions & 27 deletions pkg/model/deployment.go
Expand Up @@ -18,6 +18,8 @@ import (
"fmt"
"os"

"k8s.io/utils/ptr"

corev1 "k8s.io/api/core/v1"

bsv1alpha1 "redhat-developer/red-hat-developer-hub-operator/api/v1alpha1"
Expand Down Expand Up @@ -77,12 +79,7 @@ func (b *BackstageDeployment) addToModel(model *BackstageModel, _ bsv1alpha1.Bac
// override image with env var
// [GA] TODO Do we need this feature?
if os.Getenv(BackstageImageEnvVar) != "" {
b.deployment.Spec.Template.Spec.Containers[0].Image = os.Getenv(BackstageImageEnvVar)
// exactly the same image for initContainer and want it to be overriden
// the same way as Backstage's one
for i := range b.deployment.Spec.Template.Spec.InitContainers {
b.deployment.Spec.Template.Spec.InitContainers[i].Image = os.Getenv(BackstageImageEnvVar)
}
b.setImage(ptr.To(os.Getenv(BackstageImageEnvVar)))
}

return true, nil
Expand Down Expand Up @@ -155,13 +152,17 @@ func (b *BackstageDeployment) setReplicas(replicas *int32) {
// sets container image name of Backstage Container
func (b *BackstageDeployment) setImage(image *string) {
if image != nil {
// TODO this is a workaround for RHDH/Janus configuration
b.container().Image = *image
// this is a workaround for RHDH/Janus configuration
// it is not a fact that all the containers should be updated
// in general case need something smarter (probably annotation based)
// in general case need something smarter
// to mark/recognize containers for update
VisitContainers(b.podSpec(), func(container *corev1.Container) {
container.Image = *image
})
if len(b.podSpec().InitContainers) > 0 {
i, ic := dynamicPluginsInitContainer(b.podSpec().InitContainers)
if ic != nil {
b.podSpec().InitContainers[i].Image = *image
}
}
}
}

Expand Down Expand Up @@ -190,19 +191,3 @@ func (b *BackstageDeployment) setImagePullSecrets(pullSecrets []string) {
corev1.LocalObjectReference{Name: ps})
}
}

// ContainerVisitor is called with each container
type ContainerVisitor func(container *corev1.Container)

// visitContainers invokes the visitor function for every container in the given pod template spec
func VisitContainers(podTemplateSpec *corev1.PodSpec, visitor ContainerVisitor) {
for i := range podTemplateSpec.InitContainers {
visitor(&podTemplateSpec.InitContainers[i])
}
for i := range podTemplateSpec.Containers {
visitor(&podTemplateSpec.Containers[i])
}
for i := range podTemplateSpec.EphemeralContainers {
visitor((*corev1.Container)(&podTemplateSpec.EphemeralContainers[i].EphemeralContainerCommon))
}
}
28 changes: 21 additions & 7 deletions pkg/model/deployment_test.go
Expand Up @@ -16,7 +16,6 @@ package model

import (
"context"
"os"
"testing"

"k8s.io/utils/ptr"
Expand Down Expand Up @@ -60,23 +59,38 @@ func TestSpecs(t *testing.T) {

}

func TestSpecImagePullSecrets(t *testing.T) {
bs := *deploymentTestBackstage.DeepCopy()

bs.Spec.Application.ImagePullSecrets = nil //[]string{}

testObj := createBackstageTest(bs).withDefaultConfig(true).
addToDefaultConfig("deployment.yaml", "janus-deployment.yaml")

model, err := InitObjects(context.TODO(), bs, testObj.externalConfig, true, true, testObj.scheme)
assert.NoError(t, err)

assert.Equal(t, 0, len(model.backstageDeployment.deployment.Spec.Template.Spec.ImagePullSecrets))
//assert.Equal(t, "my-secret", model.backstageDeployment.deployment.Spec.Template.Spec.ImagePullSecrets[0].Name)

}

// It tests the overriding image feature
// [GA] if we need this (and like this) feature
// we need to think about simple template engine
// for substitution env vars instead.
// Janus image specific
func TestOverrideBackstageImage(t *testing.T) {

bs := *deploymentTestBackstage.DeepCopy()

testObj := createBackstageTest(bs).withDefaultConfig(true).
addToDefaultConfig("deployment.yaml", "janus-deployment.yaml")
addToDefaultConfig("deployment.yaml", "sidecar-deployment.yaml")

_ = os.Setenv(BackstageImageEnvVar, "dummy")
t.Setenv(BackstageImageEnvVar, "dummy")

model, err := InitObjects(context.TODO(), bs, testObj.externalConfig, true, false, testObj.scheme)
assert.NoError(t, err)
gazarenkov marked this conversation as resolved.
Show resolved Hide resolved

assert.Equal(t, 2, len(model.backstageDeployment.podSpec().Containers))
assert.Equal(t, "dummy", model.backstageDeployment.container().Image)
assert.Equal(t, "dummy", model.backstageDeployment.podSpec().InitContainers[0].Image)
gazarenkov marked this conversation as resolved.
Show resolved Hide resolved
assert.Equal(t, "busybox", model.backstageDeployment.podSpec().Containers[1].Image)

}
14 changes: 7 additions & 7 deletions pkg/model/dynamic-plugins.go
Expand Up @@ -54,7 +54,7 @@ func addDynamicPlugins(spec v1alpha1.BackstageSpec, deployment *appsv1.Deploymen
return nil
}

if dynamicPluginsInitContainer(deployment.Spec.Template.Spec.InitContainers) == nil {
if _, ic := dynamicPluginsInitContainer(deployment.Spec.Template.Spec.InitContainers); ic == nil {
return fmt.Errorf("deployment validation failed, dynamic plugin name configured but no InitContainer %s defined", dynamicPluginInitContainerName)
}

Expand Down Expand Up @@ -106,7 +106,7 @@ func (p *DynamicPlugins) updatePod(deployment *appsv1.Deployment) {
//it creates a volume with dynamic-plugins ConfigMap (there should be a key named "dynamic-plugins.yaml")
//and mount it to the dynamic-plugin initContainer's WorkingDir (what if not specified?)

initContainer := dynamicPluginsInitContainer(deployment.Spec.Template.Spec.InitContainers)
_, initContainer := dynamicPluginsInitContainer(deployment.Spec.Template.Spec.InitContainers)
if initContainer == nil {
// it will fail on validate
return
Expand All @@ -121,7 +121,7 @@ func (p *DynamicPlugins) updatePod(deployment *appsv1.Deployment) {
// ConfigMap name must be the same as (deployment.yaml).spec.template.spec.volumes.name.dynamic-plugins-conf.ConfigMap.name
func (p *DynamicPlugins) validate(model *BackstageModel, _ v1alpha1.Backstage) error {

initContainer := dynamicPluginsInitContainer(model.backstageDeployment.deployment.Spec.Template.Spec.InitContainers)
_, initContainer := dynamicPluginsInitContainer(model.backstageDeployment.deployment.Spec.Template.Spec.InitContainers)
if initContainer == nil {
return fmt.Errorf("failed to find initContainer named %s", dynamicPluginInitContainerName)
}
Expand All @@ -142,11 +142,11 @@ func (p *DynamicPlugins) setMetaInfo(backstageName string) {

// returns initContainer supposed to initialize DynamicPlugins
// TODO consider to use a label to identify instead
func dynamicPluginsInitContainer(initContainers []corev1.Container) *corev1.Container {
for _, ic := range initContainers {
func dynamicPluginsInitContainer(initContainers []corev1.Container) (int, *corev1.Container) {
for i, ic := range initContainers {
if ic.Name == dynamicPluginInitContainerName {
return &ic
return i, &ic
}
}
return nil
return -1, nil
}
11 changes: 0 additions & 11 deletions pkg/model/testdata/janus-deployment.yaml
Expand Up @@ -28,14 +28,6 @@ spec:
defaultMode: 420
optional: true
secretName: dynamic-plugins-npmrc
# - name: dynamic-plugins-conf
# configMap:
# name: default-dynamic-plugins
# optional: true
# items:
# - key: dynamic-plugins.yaml
# path: dynamic-plugins.yaml

initContainers:
- command:
- ./install-dynamic-plugins.sh
Expand All @@ -53,9 +45,6 @@ spec:
name: dynamic-plugins-npmrc
readOnly: true
subPath: .npmrc
# - mountPath: /opt/app-root/src/dynamic-plugins.yaml
# subPath: dynamic-plugins.yaml
# name: dynamic-plugins-conf
workingDir: /opt/app-root/src

containers:
Expand Down
25 changes: 25 additions & 0 deletions pkg/model/testdata/sidecar-deployment.yaml
@@ -0,0 +1,25 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: <to_be_replaced> # placeholder for 'backstage-<cr-name>'
spec:
replicas: 1
selector:
matchLabels:
rhdh.redhat.com/app: # placeholder for 'backstage-<cr-name>'
template:
metadata:
labels:
rhdh.redhat.com/app: # placeholder for 'backstage-<cr-name>'
spec:
Dismissed Show dismissed Hide dismissed
initContainers:
- image: 'quay.io/janus-idp/backstage-showcase:next'
name: install-dynamic-plugins
containers:
- name: backstage-backend # placeholder for 'backstage-backend'
Dismissed Show dismissed Hide dismissed
Dismissed Show dismissed Hide dismissed
image: quay.io/janus-idp/backstage-showcase:next
- name: sidecar
Dismissed Show dismissed Hide dismissed
Dismissed Show dismissed Hide dismissed
image: busybox