From af10a972997c296d6ba1bb3877ddbed992fbbff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20S=C4=99k?= Date: Fri, 17 May 2019 16:35:18 +0200 Subject: [PATCH] Allow configure sidecars in Jenkins pod --- pkg/apis/jenkinsio/v1alpha1/jenkins_types.go | 34 +++- .../jenkins/configuration/base/reconcile.go | 141 ++++++++++---- .../configuration/base/reconcile_test.go | 83 +++++++++ .../configuration/base/resources/pod.go | 173 +++++++++++------- .../jenkins/configuration/base/validate.go | 31 +++- .../configuration/base/validate_test.go | 20 +- .../user/seedjobs/seedjobs_test.go | 20 +- pkg/controller/jenkins/jenkins_controller.go | 50 ++++- pkg/controller/jenkins/jobs/jobs_test.go | 20 +- test/e2e/configuration_test.go | 72 +++++--- test/e2e/jenkins.go | 70 +++---- test/e2e/wait.go | 26 ++- 12 files changed, 519 insertions(+), 221 deletions(-) create mode 100644 pkg/controller/jenkins/configuration/base/reconcile_test.go diff --git a/pkg/apis/jenkinsio/v1alpha1/jenkins_types.go b/pkg/apis/jenkinsio/v1alpha1/jenkins_types.go index d2e2faf7..dc4b8288 100644 --- a/pkg/apis/jenkinsio/v1alpha1/jenkins_types.go +++ b/pkg/apis/jenkinsio/v1alpha1/jenkins_types.go @@ -18,17 +18,35 @@ type JenkinsSpec struct { SlaveService Service `json:"slaveService,omitempty"` } +// Container defines Kubernetes container attributes +type Container struct { + Name string `json:"name"` + Image string `json:"image"` + Command []string `json:"command,omitempty"` + Args []string `json:"args,omitempty"` + WorkingDir string `json:"workingDir,omitempty"` + Ports []corev1.ContainerPort `json:"ports,omitempty"` + EnvFrom []corev1.EnvFromSource `json:"envFrom,omitempty"` + Env []corev1.EnvVar `json:"env,omitempty"` + Resources corev1.ResourceRequirements `json:"resources,omitempty"` + VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"` + LivenessProbe *corev1.Probe `json:"livenessProbe,omitempty"` + ReadinessProbe *corev1.Probe `json:"readinessProbe,omitempty"` + Lifecycle *corev1.Lifecycle `json:"lifecycle,omitempty"` + ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"` + SecurityContext *corev1.SecurityContext `json:"securityContext,omitempty"` +} + // JenkinsMaster defines the Jenkins master pod attributes and plugins, // every single change requires Jenkins master pod restart type JenkinsMaster struct { - Image string `json:"image,omitempty"` - ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"` - NodeSelector map[string]string `json:"nodeSelector,omitempty"` - Annotations map[string]string `json:"masterAnnotations,omitempty"` - Resources corev1.ResourceRequirements `json:"resources,omitempty"` - Env []corev1.EnvVar `json:"env,omitempty"` - LivenessProbe *corev1.Probe `json:"livenessProbe,omitempty"` - ReadinessProbe *corev1.Probe `json:"readinessProbe,omitempty"` + Container + + // pod properties + Annotations map[string]string `json:"masterAnnotations,omitempty"` + NodeSelector map[string]string `json:"nodeSelector,omitempty"` + Containers []Container `json:"containers,omitempty"` + // OperatorPlugins contains plugins required by operator OperatorPlugins map[string][]string `json:"basePlugins,omitempty"` // Plugins contains plugins required by user diff --git a/pkg/controller/jenkins/configuration/base/reconcile.go b/pkg/controller/jenkins/configuration/base/reconcile.go index c7151c25..40bee07f 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile.go +++ b/pkg/controller/jenkins/configuration/base/reconcile.go @@ -373,7 +373,7 @@ func isPodTerminating(pod corev1.Pod) bool { func (r *ReconcileJenkinsBaseConfiguration) isRecreatePodNeeded(currentJenkinsMasterPod corev1.Pod) bool { if version.Version != r.jenkins.Status.OperatorVersion { - r.logger.Info(fmt.Sprintf("Jenkins Operator version has changed, actual '%+v' new '%+v' - recreating pod", + r.logger.Info(fmt.Sprintf("Jenkins Operator version has changed, actual '%+v' new '%+v', recreating pod", r.jenkins.Status.OperatorVersion, version.Version)) return true } @@ -385,13 +385,9 @@ func (r *ReconcileJenkinsBaseConfiguration) isRecreatePodNeeded(currentJenkinsMa return true } - if r.jenkins.Spec.Master.Image != currentJenkinsMasterPod.Spec.Containers[0].Image { - r.logger.Info(fmt.Sprintf("Jenkins image has changed to '%+v', recreating pod", r.jenkins.Spec.Master.Image)) - return true - } - - if r.jenkins.Spec.Master.ImagePullPolicy != currentJenkinsMasterPod.Spec.Containers[0].ImagePullPolicy { - r.logger.Info(fmt.Sprintf("Jenkins image pull policy has changed to '%+v', recreating pod", r.jenkins.Spec.Master.ImagePullPolicy)) + if !reflect.DeepEqual(r.jenkins.Spec.Master.NodeSelector, currentJenkinsMasterPod.Spec.NodeSelector) { + r.logger.Info(fmt.Sprintf("Jenkins pod node selector has changed, actual '%+v' required '%+v', recreating pod", + currentJenkinsMasterPod.Spec.NodeSelector, r.jenkins.Spec.Master.NodeSelector)) return true } @@ -401,47 +397,119 @@ func (r *ReconcileJenkinsBaseConfiguration) isRecreatePodNeeded(currentJenkinsMa return true } - if !reflect.DeepEqual(r.jenkins.Spec.Master.Resources, currentJenkinsMasterPod.Spec.Containers[0].Resources) { - r.logger.Info(fmt.Sprintf("Jenkins pod resources have changed, actual '%+v' required '%+v' - recreating pod", - currentJenkinsMasterPod.Spec.Containers[0].Resources, r.jenkins.Spec.Master.Resources)) + if (len(r.jenkins.Spec.Master.Containers) + 1) != len(currentJenkinsMasterPod.Spec.Containers) { + r.logger.Info(fmt.Sprintf("Jenkins amount of containers has changed to '%+v', recreating pod", len(r.jenkins.Spec.Master.Containers)+1)) return true } - if !reflect.DeepEqual(r.jenkins.Spec.Master.ReadinessProbe, currentJenkinsMasterPod.Spec.Containers[0].ReadinessProbe) { - r.logger.Info(fmt.Sprintf("Jenkins pod readinessProbe have changed, actual '%+v' required '%+v' - recreating pod", - currentJenkinsMasterPod.Spec.Containers[0].ReadinessProbe, r.jenkins.Spec.Master.ReadinessProbe)) - return true + for _, actualContainer := range currentJenkinsMasterPod.Spec.Containers { + if actualContainer.Name == resources.JenkinsMasterContainerName { + if changed := r.compareContainers(resources.NewJenkinsMasterContainer(r.jenkins), actualContainer); changed { + return true + } + continue + } + + var expectedContainer *corev1.Container + for _, jenkinsContainer := range r.jenkins.Spec.Master.Containers { + if jenkinsContainer.Name == actualContainer.Name { + tmp := resources.ConvertJenkinsContainerToKubernetesContainer(jenkinsContainer) + expectedContainer = &tmp + } + } + + if expectedContainer == nil { + r.logger.Info(fmt.Sprintf("Container '%+v' not found in pod, recreating pod", actualContainer)) + return true + } + + if changed := r.compareContainers(*expectedContainer, actualContainer); changed { + return true + } } - if !reflect.DeepEqual(r.jenkins.Spec.Master.LivenessProbe, currentJenkinsMasterPod.Spec.Containers[0].LivenessProbe) { - r.logger.Info(fmt.Sprintf("Jenkins pod livenessProbe have changed, actual '%+v' required '%+v' - recreating pod", - currentJenkinsMasterPod.Spec.Containers[0].LivenessProbe, r.jenkins.Spec.Master.LivenessProbe)) + return false +} + +func (r *ReconcileJenkinsBaseConfiguration) compareContainers(expected corev1.Container, actual corev1.Container) bool { + if !reflect.DeepEqual(expected.Args, actual.Args) { + r.logger.Info(fmt.Sprintf("Arguments have changed to '%+v' in container '%s', recreating pod", expected.Args, expected.Name)) return true } - - if !reflect.DeepEqual(r.jenkins.Spec.Master.NodeSelector, currentJenkinsMasterPod.Spec.NodeSelector) { - r.logger.Info(fmt.Sprintf("Jenkins pod node selector has changed, actual '%+v' required '%+v' - recreating pod", - currentJenkinsMasterPod.Spec.NodeSelector, r.jenkins.Spec.Master.NodeSelector)) + if !reflect.DeepEqual(expected.Command, actual.Command) { + r.logger.Info(fmt.Sprintf("Command has changed to '%+v' in container '%s', recreating pod", expected.Command, expected.Name)) return true } - - requiredEnvs := resources.GetJenkinsMasterPodBaseEnvs() - requiredEnvs = append(requiredEnvs, r.jenkins.Spec.Master.Env...) - if !reflect.DeepEqual(requiredEnvs, currentJenkinsMasterPod.Spec.Containers[0].Env) { - r.logger.Info(fmt.Sprintf("Jenkins env have changed, actual '%+v' required '%+v' - recreating pod", - currentJenkinsMasterPod.Spec.Containers[0].Env, requiredEnvs)) + if !reflect.DeepEqual(expected.Env, actual.Env) { + r.logger.Info(fmt.Sprintf("Env has changed to '%+v' in container '%s', recreating pod", expected.Env, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.EnvFrom, actual.EnvFrom) { + r.logger.Info(fmt.Sprintf("EnvFrom has changed to '%+v' in container '%s', recreating pod", expected.EnvFrom, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.Image, actual.Image) { + r.logger.Info(fmt.Sprintf("Image has changed to '%+v' in container '%s', recreating pod", expected.Image, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.ImagePullPolicy, actual.ImagePullPolicy) { + r.logger.Info(fmt.Sprintf("Image pull policy has changed to '%+v' in container '%s', recreating pod", expected.ImagePullPolicy, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.Lifecycle, actual.Lifecycle) { + r.logger.Info(fmt.Sprintf("Lifecycle has changed to '%+v' in container '%s', recreating pod", expected.Lifecycle, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.LivenessProbe, actual.LivenessProbe) { + r.logger.Info(fmt.Sprintf("Liveness probe has changed to '%+v' in container '%s', recreating pod", expected.LivenessProbe, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.Ports, actual.Ports) { + r.logger.Info(fmt.Sprintf("Ports have changed to '%+v' in container '%s', recreating pod", expected.Ports, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.ReadinessProbe, actual.ReadinessProbe) { + r.logger.Info(fmt.Sprintf("Readiness probe has changed to '%+v' in container '%s', recreating pod", expected.ReadinessProbe, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.Resources, actual.Resources) { + r.logger.Info(fmt.Sprintf("Resources have changed to '%+v' in container '%s', recreating pod", expected.Resources, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.SecurityContext, actual.SecurityContext) { + r.logger.Info(fmt.Sprintf("Security context has changed to '%+v' in container '%s', recreating pod", expected.SecurityContext, expected.Name)) + return true + } + if !reflect.DeepEqual(expected.WorkingDir, actual.WorkingDir) { + r.logger.Info(fmt.Sprintf("Working directory has changed to '%+v' in container '%s', recreating pod", expected.WorkingDir, expected.Name)) + return true + } + if !CompareContainerVolumeMounts(expected, actual) { + r.logger.Info(fmt.Sprintf("Volume mounts has changed to '%+v' in container '%s', recreating pod", expected.VolumeMounts, expected.Name)) return true } return false } +// CompareContainerVolumeMounts returns true if two containers volume mounts are the same +func CompareContainerVolumeMounts(expected corev1.Container, actual corev1.Container) bool { + var withoutServiceAccount []corev1.VolumeMount + for _, volumeMount := range actual.VolumeMounts { + if volumeMount.MountPath != "/var/run/secrets/kubernetes.io/serviceaccount" { + withoutServiceAccount = append(withoutServiceAccount, volumeMount) + } + } + + return reflect.DeepEqual(expected.VolumeMounts, withoutServiceAccount) +} + func (r *ReconcileJenkinsBaseConfiguration) restartJenkinsMasterPod(meta metav1.ObjectMeta) error { currentJenkinsMasterPod, err := r.getJenkinsMasterPod(meta) - r.logger.Info(fmt.Sprintf("Terminating Jenkins Master Pod %s/%s", currentJenkinsMasterPod.Namespace, currentJenkinsMasterPod.Name)) if err != nil { return err } + r.logger.Info(fmt.Sprintf("Terminating Jenkins Master Pod %s/%s", currentJenkinsMasterPod.Namespace, currentJenkinsMasterPod.Name)) return stackerr.WithStack(r.k8sClient.Delete(context.TODO(), currentJenkinsMasterPod)) } @@ -461,11 +529,20 @@ func (r *ReconcileJenkinsBaseConfiguration) waitForJenkins(meta metav1.ObjectMet return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil } + containersReadyCount := 0 for _, containerStatus := range jenkinsMasterPodStatus.Status.ContainerStatuses { - if !containerStatus.Ready { - r.logger.V(log.VDebug).Info("Jenkins master pod not ready, readiness probe failed") - return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil + if containerStatus.State.Terminated != nil { + r.logger.Info(fmt.Sprintf("Container '%s' is terminated, status '%+v', recreating pod", containerStatus.Name, containerStatus)) + return reconcile.Result{Requeue: true}, r.restartJenkinsMasterPod(meta) } + if !containerStatus.Ready { + r.logger.V(log.VDebug).Info(fmt.Sprintf("Container '%s' not ready, readiness probe failed", containerStatus.Name)) + } else { + containersReadyCount++ + } + } + if containersReadyCount != len(jenkinsMasterPodStatus.Status.ContainerStatuses) { + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil } return reconcile.Result{}, nil diff --git a/pkg/controller/jenkins/configuration/base/reconcile_test.go b/pkg/controller/jenkins/configuration/base/reconcile_test.go new file mode 100644 index 00000000..1144f55d --- /dev/null +++ b/pkg/controller/jenkins/configuration/base/reconcile_test.go @@ -0,0 +1,83 @@ +package base + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" +) + +func TestCompareContainerVolumeMounts(t *testing.T) { + t.Run("happy with service account", func(t *testing.T) { + expectedContainer := corev1.Container{ + VolumeMounts: []corev1.VolumeMount{ + { + Name: "volume-name", + MountPath: "/mount/path", + }, + }, + } + actualContainer := corev1.Container{ + VolumeMounts: []corev1.VolumeMount{ + { + Name: "volume-name", + MountPath: "/mount/path", + }, + { + Name: "jenkins-operator-example-token-dh4r9", + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + ReadOnly: true, + }, + }, + } + + got := CompareContainerVolumeMounts(expectedContainer, actualContainer) + + assert.True(t, got) + }) + t.Run("happy without service account", func(t *testing.T) { + expectedContainer := corev1.Container{ + VolumeMounts: []corev1.VolumeMount{ + { + Name: "volume-name", + MountPath: "/mount/path", + }, + }, + } + actualContainer := corev1.Container{ + VolumeMounts: []corev1.VolumeMount{ + { + Name: "volume-name", + MountPath: "/mount/path", + }, + }, + } + + got := CompareContainerVolumeMounts(expectedContainer, actualContainer) + + assert.True(t, got) + }) + t.Run("different volume mounts", func(t *testing.T) { + expectedContainer := corev1.Container{ + VolumeMounts: []corev1.VolumeMount{ + { + Name: "volume-name", + MountPath: "/mount/path", + }, + }, + } + actualContainer := corev1.Container{ + VolumeMounts: []corev1.VolumeMount{ + { + Name: "jenkins-operator-example-token-dh4r9", + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + ReadOnly: true, + }, + }, + } + + got := CompareContainerVolumeMounts(expectedContainer, actualContainer) + + assert.False(t, got) + }) +} diff --git a/pkg/controller/jenkins/configuration/base/resources/pod.go b/pkg/controller/jenkins/configuration/base/resources/pod.go index 2319272c..c41fc13e 100644 --- a/pkg/controller/jenkins/configuration/base/resources/pod.go +++ b/pkg/controller/jenkins/configuration/base/resources/pod.go @@ -11,9 +11,11 @@ import ( ) const ( - jenkinsHomeVolumeName = "home" - jenkinsPath = "/var/jenkins" - jenkinsHomePath = jenkinsPath + "/home" + // JenkinsMasterContainerName is the Jenkins master container name in pod + JenkinsMasterContainerName = "jenkins-master" + jenkinsHomeVolumeName = "home" + jenkinsPath = "/var/jenkins" + jenkinsHomePath = jenkinsPath + "/home" jenkinsScriptsVolumeName = "scripts" jenkinsScriptsVolumePath = jenkinsPath + "/scripts" @@ -72,13 +74,111 @@ func GetJenkinsMasterPodBaseEnvs() []corev1.EnvVar { } } +// NewJenkinsMasterContainer returns Jenkins master Kubernetes container +func NewJenkinsMasterContainer(jenkins *v1alpha1.Jenkins) corev1.Container { + envs := GetJenkinsMasterPodBaseEnvs() + envs = append(envs, jenkins.Spec.Master.Env...) + + return corev1.Container{ + Name: JenkinsMasterContainerName, + Image: jenkins.Spec.Master.Image, + ImagePullPolicy: jenkins.Spec.Master.ImagePullPolicy, + Command: []string{ + "bash", + fmt.Sprintf("%s/%s", jenkinsScriptsVolumePath, initScriptName), + }, + LivenessProbe: jenkins.Spec.Master.LivenessProbe, + ReadinessProbe: jenkins.Spec.Master.ReadinessProbe, + Ports: []corev1.ContainerPort{ + { + Name: httpPortName, + ContainerPort: constants.DefaultHTTPPortInt32, + Protocol: corev1.ProtocolTCP, + }, + { + Name: slavePortName, + ContainerPort: constants.DefaultSlavePortInt32, + Protocol: corev1.ProtocolTCP, + }, + }, + Env: envs, + Resources: jenkins.Spec.Master.Resources, + VolumeMounts: []corev1.VolumeMount{ + { + Name: jenkinsHomeVolumeName, + MountPath: jenkinsHomePath, + ReadOnly: false, + }, + { + Name: jenkinsScriptsVolumeName, + MountPath: jenkinsScriptsVolumePath, + ReadOnly: true, + }, + { + Name: jenkinsInitConfigurationVolumeName, + MountPath: jenkinsInitConfigurationVolumePath, + ReadOnly: true, + }, + { + Name: jenkinsBaseConfigurationVolumeName, + MountPath: JenkinsBaseConfigurationVolumePath, + ReadOnly: true, + }, + { + Name: jenkinsUserConfigurationVolumeName, + MountPath: JenkinsUserConfigurationVolumePath, + ReadOnly: true, + }, + { + Name: jenkinsOperatorCredentialsVolumeName, + MountPath: jenkinsOperatorCredentialsVolumePath, + ReadOnly: true, + }, + { + Name: userConfigurationSecretVolumeName, + MountPath: UserConfigurationSecretVolumePath, + ReadOnly: true, + }, + }, + } +} + +// ConvertJenkinsContainerToKubernetesContainer converts Jenkins container to Kubernetes container +func ConvertJenkinsContainerToKubernetesContainer(container v1alpha1.Container) corev1.Container { + return corev1.Container{ + Name: container.Name, + Image: container.Image, + Command: container.Command, + Args: container.Args, + WorkingDir: container.WorkingDir, + Ports: container.Ports, + EnvFrom: container.EnvFrom, + Env: container.Env, + Resources: container.Resources, + VolumeMounts: container.VolumeMounts, + LivenessProbe: container.LivenessProbe, + ReadinessProbe: container.ReadinessProbe, + Lifecycle: container.Lifecycle, + ImagePullPolicy: container.ImagePullPolicy, + SecurityContext: container.SecurityContext, + } +} + +func newContainers(jenkins *v1alpha1.Jenkins) (containers []corev1.Container) { + containers = append(containers, NewJenkinsMasterContainer(jenkins)) + + for _, container := range jenkins.Spec.Master.Containers { + containers = append(containers, ConvertJenkinsContainerToKubernetesContainer(container)) + } + + return +} + // NewJenkinsMasterPod builds Jenkins Master Kubernetes Pod resource func NewJenkinsMasterPod(objectMeta metav1.ObjectMeta, jenkins *v1alpha1.Jenkins) *corev1.Pod { runAsUser := jenkinsUserUID objectMeta.Annotations = jenkins.Spec.Master.Annotations - envs := GetJenkinsMasterPodBaseEnvs() - envs = append(envs, jenkins.Spec.Master.Env...) return &corev1.Pod{ TypeMeta: buildPodTypeMeta(), @@ -91,68 +191,7 @@ func NewJenkinsMasterPod(objectMeta metav1.ObjectMeta, jenkins *v1alpha1.Jenkins RunAsGroup: &runAsUser, }, NodeSelector: jenkins.Spec.Master.NodeSelector, - Containers: []corev1.Container{ - { - Name: "jenkins-master", - Image: jenkins.Spec.Master.Image, - ImagePullPolicy: jenkins.Spec.Master.ImagePullPolicy, - Command: []string{ - "bash", - fmt.Sprintf("%s/%s", jenkinsScriptsVolumePath, initScriptName), - }, - LivenessProbe: jenkins.Spec.Master.LivenessProbe, - ReadinessProbe: jenkins.Spec.Master.ReadinessProbe, - Ports: []corev1.ContainerPort{ - { - Name: httpPortName, - ContainerPort: constants.DefaultHTTPPortInt32, - }, - { - Name: slavePortName, - ContainerPort: constants.DefaultSlavePortInt32, - }, - }, - Env: envs, - Resources: jenkins.Spec.Master.Resources, - VolumeMounts: []corev1.VolumeMount{ - { - Name: jenkinsHomeVolumeName, - MountPath: jenkinsHomePath, - ReadOnly: false, - }, - { - Name: jenkinsScriptsVolumeName, - MountPath: jenkinsScriptsVolumePath, - ReadOnly: true, - }, - { - Name: jenkinsInitConfigurationVolumeName, - MountPath: jenkinsInitConfigurationVolumePath, - ReadOnly: true, - }, - { - Name: jenkinsBaseConfigurationVolumeName, - MountPath: JenkinsBaseConfigurationVolumePath, - ReadOnly: true, - }, - { - Name: jenkinsUserConfigurationVolumeName, - MountPath: JenkinsUserConfigurationVolumePath, - ReadOnly: true, - }, - { - Name: jenkinsOperatorCredentialsVolumeName, - MountPath: jenkinsOperatorCredentialsVolumePath, - ReadOnly: true, - }, - { - Name: userConfigurationSecretVolumeName, - MountPath: UserConfigurationSecretVolumePath, - ReadOnly: true, - }, - }, - }, - }, + Containers: newContainers(jenkins), Volumes: []corev1.Volume{ { Name: jenkinsHomeVolumeName, diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go index bbce249a..2a895d62 100644 --- a/pkg/controller/jenkins/configuration/base/validate.go +++ b/pkg/controller/jenkins/configuration/base/validate.go @@ -18,15 +18,14 @@ var ( // Validate validates Jenkins CR Spec.master section func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *v1alpha1.Jenkins) (bool, error) { - if jenkins.Spec.Master.Image == "" { - r.logger.V(log.VWarn).Info("Image not set") + if !r.validateContainer(jenkins.Spec.Master.Container) { return false, nil } - if !dockerImageRegexp.MatchString(jenkins.Spec.Master.Image) && !docker.ReferenceRegexp.MatchString(jenkins.Spec.Master.Image) { - r.logger.V(log.VWarn).Info("Invalid image") - return false, nil - + for _, container := range jenkins.Spec.Master.Containers { + if !r.validateContainer(container) { + return false, nil + } } if !r.validatePlugins(jenkins.Spec.Master.OperatorPlugins, jenkins.Spec.Master.Plugins) { @@ -40,6 +39,26 @@ func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *v1alpha1.Jenkins) return true, nil } +func (r *ReconcileJenkinsBaseConfiguration) validateContainer(container v1alpha1.Container) bool { + logger := r.logger.WithValues("container", container.Name) + if container.Image == "" { + logger.V(log.VWarn).Info("Image not set") + return false + } + + if !dockerImageRegexp.MatchString(container.Image) && !docker.ReferenceRegexp.MatchString(container.Image) { + r.logger.V(log.VWarn).Info("Invalid image") + return false + } + + if container.ImagePullPolicy == "" { + logger.V(log.VWarn).Info("Image pull policy not set") + return false + } + + return true +} + func (r *ReconcileJenkinsBaseConfiguration) validateJenkinsMasterPodEnvs() bool { baseEnvs := resources.GetJenkinsMasterPodBaseEnvs() baseEnvNames := map[string]string{} diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index 301df06c..ad127755 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/pkg/controller/jenkins/configuration/base/validate_test.go @@ -74,10 +74,12 @@ func TestValidateJenkinsMasterPodEnvs(t *testing.T) { jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ Master: v1alpha1.JenkinsMaster{ - Env: []v1.EnvVar{ - { - Name: "SOME_VALUE", - Value: "", + Container: v1alpha1.Container{ + Env: []v1.EnvVar{ + { + Name: "SOME_VALUE", + Value: "", + }, }, }, }, @@ -92,10 +94,12 @@ func TestValidateJenkinsMasterPodEnvs(t *testing.T) { jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ Master: v1alpha1.JenkinsMaster{ - Env: []v1.EnvVar{ - { - Name: "JENKINS_HOME", - Value: "", + Container: v1alpha1.Container{ + Env: []v1.EnvVar{ + { + Name: "JENKINS_HOME", + Value: "", + }, }, }, }, diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go b/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go index d0eb7466..957ce037 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go +++ b/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go @@ -119,16 +119,18 @@ func jenkinsCustomResource() *v1alpha1.Jenkins { }, Spec: v1alpha1.JenkinsSpec{ Master: v1alpha1.JenkinsMaster{ - Image: "jenkins/jenkins", Annotations: map[string]string{"test": "label"}, - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("300m"), - corev1.ResourceMemory: resource.MustParse("500Mi"), - }, - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("2"), - corev1.ResourceMemory: resource.MustParse("2Gi"), + Container: v1alpha1.Container{ + Image: "jenkins/jenkins", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("300m"), + corev1.ResourceMemory: resource.MustParse("500Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2"), + corev1.ResourceMemory: resource.MustParse("2Gi"), + }, }, }, }, diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index 98070cad..1cff440b 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -252,7 +252,7 @@ func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha1.Jenkins, logger logr.Lo Scheme: corev1.URISchemeHTTP, }, }, - InitialDelaySeconds: int32(30), + InitialDelaySeconds: int32(80), TimeoutSeconds: int32(5), FailureThreshold: int32(12), } @@ -276,12 +276,8 @@ func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha1.Jenkins, logger logr.Lo changed = true jenkins.Spec.Master.Plugins = map[string][]string{"simple-theme-plugin:0.5.1": {}} } - _, requestCPUSet := jenkins.Spec.Master.Resources.Requests[corev1.ResourceCPU] - _, requestMemporySet := jenkins.Spec.Master.Resources.Requests[corev1.ResourceMemory] - _, limitCPUSet := jenkins.Spec.Master.Resources.Limits[corev1.ResourceCPU] - _, limitMemporySet := jenkins.Spec.Master.Resources.Limits[corev1.ResourceMemory] - if !limitCPUSet || !limitMemporySet || !requestCPUSet || !requestMemporySet { - logger.Info("Setting default Jenkins master pod resource requirements") + if isResourceRequirementsNotSet(jenkins.Spec.Master.Resources) { + logger.Info("Setting default Jenkins master container resource requirements") changed = true jenkins.Spec.Master.Resources = corev1.ResourceRequirements{ Requests: corev1.ResourceList{ @@ -318,9 +314,49 @@ func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha1.Jenkins, logger logr.Lo Port: constants.DefaultSlavePortInt32, } } + for i, container := range jenkins.Spec.Master.Containers { + if setDefaultsForContainer(jenkins, i, logger.WithValues("container", container.Name)) { + changed = true + } + } if changed { return errors.WithStack(r.client.Update(context.TODO(), jenkins)) } return nil } + +func setDefaultsForContainer(jenkins *v1alpha1.Jenkins, containerIndex int, logger logr.Logger) bool { + changed := false + + if len(jenkins.Spec.Master.Containers[containerIndex].ImagePullPolicy) == 0 { + logger.Info(fmt.Sprintf("Setting default container image pull policy: %s", corev1.PullAlways)) + changed = true + jenkins.Spec.Master.Containers[containerIndex].ImagePullPolicy = corev1.PullAlways + } + if isResourceRequirementsNotSet(jenkins.Spec.Master.Containers[containerIndex].Resources) { + logger.Info("Setting default container resource requirements") + changed = true + jenkins.Spec.Master.Containers[containerIndex].Resources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + corev1.ResourceMemory: resource.MustParse("50Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("100Mi"), + }, + } + } + + return changed +} + +func isResourceRequirementsNotSet(requirements corev1.ResourceRequirements) bool { + _, requestCPUSet := requirements.Requests[corev1.ResourceCPU] + _, requestMemporySet := requirements.Requests[corev1.ResourceMemory] + _, limitCPUSet := requirements.Limits[corev1.ResourceCPU] + _, limitMemorySet := requirements.Limits[corev1.ResourceMemory] + + return !limitCPUSet || !limitMemorySet || !requestCPUSet || !requestMemporySet +} diff --git a/pkg/controller/jenkins/jobs/jobs_test.go b/pkg/controller/jenkins/jobs/jobs_test.go index 88582259..fe6e43f0 100644 --- a/pkg/controller/jenkins/jobs/jobs_test.go +++ b/pkg/controller/jenkins/jobs/jobs_test.go @@ -387,16 +387,18 @@ func jenkinsCustomResource() *v1alpha1.Jenkins { }, Spec: v1alpha1.JenkinsSpec{ Master: v1alpha1.JenkinsMaster{ - Image: "jenkins/jenkins", Annotations: map[string]string{"test": "label"}, - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("300m"), - corev1.ResourceMemory: resource.MustParse("500Mi"), - }, - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("2"), - corev1.ResourceMemory: resource.MustParse("2Gi"), + Container: v1alpha1.Container{ + Image: "jenkins/jenkins", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("300m"), + corev1.ResourceMemory: resource.MustParse("500Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2"), + corev1.ResourceMemory: resource.MustParse("2Gi"), + }, }, }, }, diff --git a/test/e2e/configuration_test.go b/test/e2e/configuration_test.go index 03255290..1988d4cb 100644 --- a/test/e2e/configuration_test.go +++ b/test/e2e/configuration_test.go @@ -3,11 +3,11 @@ package e2e import ( "context" "fmt" - "reflect" "testing" "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkinsio/v1alpha1" jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/client" + "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/base" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/plugins" @@ -133,43 +133,57 @@ func verifyJenkinsMasterPodAttributes(t *testing.T, jenkins *v1alpha1.Jenkins) { jenkinsPod := getJenkinsMasterPod(t, jenkins) jenkins = getJenkins(t, jenkins.Namespace, jenkins.Name) - for key, value := range jenkins.Spec.Master.Annotations { - if jenkinsPod.ObjectMeta.Annotations[key] != value { - t.Fatalf("Invalid Jenkins pod annotation expected '%+v', actual '%+v'", jenkins.Spec.Master.Annotations, jenkinsPod.ObjectMeta.Annotations) + assert.Equal(t, jenkins.Spec.Master.Annotations, jenkinsPod.ObjectMeta.Annotations) + assert.Equal(t, jenkins.Spec.Master.NodeSelector, jenkinsPod.Spec.NodeSelector) + + assert.Equal(t, resources.JenkinsMasterContainerName, jenkinsPod.Spec.Containers[0].Name) + assert.Equal(t, len(jenkins.Spec.Master.Containers)+1, len(jenkinsPod.Spec.Containers)) + + for _, actualContainer := range jenkinsPod.Spec.Containers { + if actualContainer.Name == resources.JenkinsMasterContainerName { + verifyContainer(t, resources.NewJenkinsMasterContainer(jenkins), actualContainer) + continue } - } - jenkinsContainer := jenkinsPod.Spec.Containers[0] + var expectedContainer *corev1.Container + for _, jenkinsContainer := range jenkins.Spec.Master.Containers { + if jenkinsContainer.Name == actualContainer.Name { + tmp := resources.ConvertJenkinsContainerToKubernetesContainer(jenkinsContainer) + expectedContainer = &tmp + } + } - if jenkinsContainer.Image != jenkins.Spec.Master.Image { - t.Fatalf("Invalid jenkins pod image expected '%s', actual '%s'", jenkins.Spec.Master.Image, jenkinsContainer.Image) - } + if expectedContainer == nil { + t.Errorf("Container '%+v' not found in pod", actualContainer) + continue + } - if !reflect.DeepEqual(jenkinsContainer.Resources, jenkins.Spec.Master.Resources) { - t.Fatalf("Invalid jenkins pod continer resources expected '%+v', actual '%+v'", jenkins.Spec.Master.Resources, jenkinsContainer.Resources) - } - - if !reflect.DeepEqual(jenkinsPod.Spec.NodeSelector, jenkins.Spec.Master.NodeSelector) { - t.Fatalf("Invalid jenkins pod node selector expected '%+v', actual '%+v'", jenkins.Spec.Master.NodeSelector, jenkinsPod.Spec.NodeSelector) - } - - if !reflect.DeepEqual(jenkinsContainer.ReadinessProbe, jenkins.Spec.Master.ReadinessProbe) { - t.Fatalf("Invalid jenkins pod readinessProbe. Expected '%+v', actual '%+v'", jenkins.Spec.Master.ReadinessProbe, jenkinsContainer.ReadinessProbe) - } - - if !reflect.DeepEqual(jenkinsContainer.LivenessProbe, jenkins.Spec.Master.LivenessProbe) { - t.Fatalf("Invalid jenkins pod livenessProbe. Expected '%+v', actual '%+v'", jenkins.Spec.Master.LivenessProbe, jenkinsContainer.LivenessProbe) - } - - requiredEnvs := resources.GetJenkinsMasterPodBaseEnvs() - requiredEnvs = append(requiredEnvs, jenkins.Spec.Master.Env...) - if !reflect.DeepEqual(jenkinsContainer.Env, requiredEnvs) { - t.Fatalf("Invalid jenkins pod continer resources expected '%+v', actual '%+v'", requiredEnvs, jenkinsContainer.Env) + verifyContainer(t, *expectedContainer, actualContainer) } t.Log("Jenkins pod attributes are valid") } +func verifyContainer(t *testing.T, expected corev1.Container, actual corev1.Container) { + assert.Equal(t, expected.Args, actual.Args, expected.Name, expected.Name) + assert.Equal(t, expected.Command, actual.Command, expected.Name) + assert.Equal(t, expected.Env, actual.Env, expected.Name) + assert.Equal(t, expected.EnvFrom, actual.EnvFrom, expected.Name) + assert.Equal(t, expected.Image, actual.Image, expected.Name) + assert.Equal(t, expected.ImagePullPolicy, actual.ImagePullPolicy, expected.Name) + assert.Equal(t, expected.Lifecycle, actual.Lifecycle, expected.Name) + assert.Equal(t, expected.LivenessProbe, actual.LivenessProbe, expected.Name) + assert.Equal(t, expected.Ports, actual.Ports, expected.Name) + assert.Equal(t, expected.ReadinessProbe, actual.ReadinessProbe, expected.Name) + assert.Equal(t, expected.Resources, actual.Resources, expected.Name) + assert.Equal(t, expected.SecurityContext, actual.SecurityContext, expected.Name) + assert.Equal(t, expected.WorkingDir, actual.WorkingDir, expected.Name) + if !base.CompareContainerVolumeMounts(expected, actual) { + t.Errorf("Volume mounts are different in container '%s': expected '%+v', actual '%+v'", + expected.Name, expected.VolumeMounts, actual.VolumeMounts) + } +} + func verifyPlugins(t *testing.T, jenkinsClient jenkinsclient.Jenkins, jenkins *v1alpha1.Jenkins) { installedPlugins, err := jenkinsClient.GetPlugins(1) if err != nil { diff --git a/test/e2e/jenkins.go b/test/e2e/jenkins.go index 6e576ebf..15962069 100644 --- a/test/e2e/jenkins.go +++ b/test/e2e/jenkins.go @@ -73,43 +73,51 @@ func createJenkinsCR(t *testing.T, name, namespace string, seedJob *[]v1alpha1.S }, Spec: v1alpha1.JenkinsSpec{ Master: v1alpha1.JenkinsMaster{ - Image: "jenkins/jenkins", Annotations: map[string]string{"test": "label"}, + Container: v1alpha1.Container{ + Image: "jenkins/jenkins", + Env: []v1.EnvVar{ + { + Name: "TEST_ENV", + Value: "test_env_value", + }, + }, + ReadinessProbe: &corev1.Probe{ + Handler: corev1.Handler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/login", + Port: intstr.FromString("http"), + Scheme: corev1.URISchemeHTTP, + }, + }, + InitialDelaySeconds: int32(80), + TimeoutSeconds: int32(4), + FailureThreshold: int32(10), + }, + LivenessProbe: &corev1.Probe{ + Handler: corev1.Handler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/login", + Port: intstr.FromString("http"), + Scheme: corev1.URISchemeHTTP, + }, + }, + InitialDelaySeconds: int32(80), + TimeoutSeconds: int32(4), + FailureThreshold: int32(10), + }, + }, + Containers: []v1alpha1.Container{ + { + Name: "envoyproxy", + Image: "envoyproxy/envoy-alpine", + }, + }, Plugins: map[string][]string{ "audit-trail:2.4": {}, "simple-theme-plugin:0.5.1": {}, }, NodeSelector: map[string]string{"kubernetes.io/hostname": "minikube"}, - Env: []v1.EnvVar{ - { - Name: "TEST_ENV", - Value: "test_env_value", - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/login", - Port: intstr.FromString("http"), - Scheme: corev1.URISchemeHTTP, - }, - }, - InitialDelaySeconds: int32(35), - TimeoutSeconds: int32(4), - FailureThreshold: int32(10), - }, - LivenessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/login", - Port: intstr.FromString("http"), - Scheme: corev1.URISchemeHTTP, - }, - }, - InitialDelaySeconds: int32(40), - TimeoutSeconds: int32(4), - FailureThreshold: int32(10), - }, }, SeedJobs: seedJobs, }, diff --git a/test/e2e/wait.go b/test/e2e/wait.go index 40299ecb..4f555a0e 100644 --- a/test/e2e/wait.go +++ b/test/e2e/wait.go @@ -2,7 +2,6 @@ package e2e import ( goctx "context" - "fmt" "net/http" "testing" "time" @@ -27,14 +26,14 @@ var ( timeout = time.Second * 60 ) -// checkConditionFunc is used to check if a condition for the jenkins CR is true -type checkConditionFunc func(*v1alpha1.Jenkins) bool +// checkConditionFunc is used to check if a condition for the jenkins CR is set +type checkConditionFunc func(*v1alpha1.Jenkins, error) bool func waitForJenkinsBaseConfigurationToComplete(t *testing.T, jenkins *v1alpha1.Jenkins) { t.Log("Waiting for Jenkins base configuration to complete") - _, err := WaitUntilJenkinsConditionTrue(retryInterval, 150, jenkins, func(jenkins *v1alpha1.Jenkins) bool { - t.Logf("Current Jenkins status '%+v'", jenkins.Status) - return jenkins.Status.BaseConfigurationCompletedTime != nil + _, err := WaitUntilJenkinsConditionSet(retryInterval, 150, jenkins, func(jenkins *v1alpha1.Jenkins, err error) bool { + t.Logf("Current Jenkins status: '%+v', error '%s'", jenkins.Status, err) + return err == nil && jenkins.Status.BaseConfigurationCompletedTime != nil }) assert.NoError(t, err) t.Log("Jenkins pod is running") @@ -68,9 +67,9 @@ func waitForRecreateJenkinsMasterPod(t *testing.T, jenkins *v1alpha1.Jenkins) { func waitForJenkinsUserConfigurationToComplete(t *testing.T, jenkins *v1alpha1.Jenkins) { t.Log("Waiting for Jenkins user configuration to complete") - _, err := WaitUntilJenkinsConditionTrue(retryInterval, 30, jenkins, func(jenkins *v1alpha1.Jenkins) bool { - t.Logf("Current Jenkins status '%+v'", jenkins.Status) - return jenkins.Status.UserConfigurationCompletedTime != nil + _, err := WaitUntilJenkinsConditionSet(retryInterval, 70, jenkins, func(jenkins *v1alpha1.Jenkins, err error) bool { + t.Logf("Current Jenkins status: '%+v', error '%s'", jenkins.Status, err) + return err == nil && jenkins.Status.UserConfigurationCompletedTime != nil }) if err != nil { t.Fatal(err) @@ -92,16 +91,13 @@ func waitForJenkinsSafeRestart(t *testing.T, jenkinsClient jenkinsclient.Jenkins require.NoError(t, err) } -// WaitUntilJenkinsConditionTrue retries until the specified condition check becomes true for the jenkins CR -func WaitUntilJenkinsConditionTrue(retryInterval time.Duration, retries int, jenkins *v1alpha1.Jenkins, checkCondition checkConditionFunc) (*v1alpha1.Jenkins, error) { +// WaitUntilJenkinsConditionSet retries until the specified condition check becomes true for the jenkins CR +func WaitUntilJenkinsConditionSet(retryInterval time.Duration, retries int, jenkins *v1alpha1.Jenkins, checkCondition checkConditionFunc) (*v1alpha1.Jenkins, error) { jenkinsStatus := &v1alpha1.Jenkins{} err := wait.Poll(retryInterval, time.Duration(retries)*retryInterval, func() (bool, error) { namespacedName := types.NamespacedName{Namespace: jenkins.Namespace, Name: jenkins.Name} err := framework.Global.Client.Get(goctx.TODO(), namespacedName, jenkinsStatus) - if err != nil { - return false, fmt.Errorf("failed to get CR: %v", err) - } - return checkCondition(jenkinsStatus), nil + return checkCondition(jenkinsStatus, err), nil }) if err != nil { return nil, err