From ea5ad560127ca15a219093eae2834537a3e44658 Mon Sep 17 00:00:00 2001 From: Sylwia Brant Date: Fri, 9 Oct 2020 15:28:02 +0200 Subject: [PATCH] Fixed targetPort value Made the targetPort in Jenkins pod a fixed value. Previously users encountered issue with changing values. Added test comparing expected and actual targetPort values. --- pkg/configuration/base/reconcile.go | 5 +++-- pkg/configuration/base/resources/service.go | 4 +++- pkg/configuration/base/service.go | 6 +++--- test/e2e/configuration_test.go | 10 ++++++++++ test/e2e/jenkins.go | 15 +++++++++++++++ 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/pkg/configuration/base/reconcile.go b/pkg/configuration/base/reconcile.go index 8ebf5351..2f07becf 100644 --- a/pkg/configuration/base/reconcile.go +++ b/pkg/configuration/base/reconcile.go @@ -13,6 +13,7 @@ import ( jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/client" "github.com/jenkinsci/kubernetes-operator/pkg/configuration" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" + "github.com/jenkinsci/kubernetes-operator/pkg/constants" "github.com/jenkinsci/kubernetes-operator/pkg/groovy" "github.com/jenkinsci/kubernetes-operator/pkg/log" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" @@ -175,12 +176,12 @@ func (r *ReconcileJenkinsBaseConfiguration) ensureResourcesRequiredForJenkinsPod r.logger.V(log.VDebug).Info("Extra role bindings are present") httpServiceName := resources.GetJenkinsHTTPServiceName(r.Configuration.Jenkins) - if err := r.createService(metaObject, httpServiceName, r.Configuration.Jenkins.Spec.Service); err != nil { + if err := r.createService(metaObject, httpServiceName, r.Configuration.Jenkins.Spec.Service, constants.DefaultHTTPPortInt32); err != nil { return err } r.logger.V(log.VDebug).Info("Jenkins HTTP Service is present") - if err := r.createService(metaObject, resources.GetJenkinsSlavesServiceName(r.Configuration.Jenkins), r.Configuration.Jenkins.Spec.SlaveService); err != nil { + if err := r.createService(metaObject, resources.GetJenkinsSlavesServiceName(r.Configuration.Jenkins), r.Configuration.Jenkins.Spec.SlaveService, constants.DefaultSlavePortInt32); err != nil { return err } r.logger.V(log.VDebug).Info("Jenkins slave Service is present") diff --git a/pkg/configuration/base/resources/service.go b/pkg/configuration/base/resources/service.go index 6e9cdc67..0d3201ad 100644 --- a/pkg/configuration/base/resources/service.go +++ b/pkg/configuration/base/resources/service.go @@ -9,6 +9,7 @@ import ( stackerr "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" "net" "strings" @@ -18,7 +19,7 @@ import ( const ServiceKind = "Service" // UpdateService returns new service with override fields from config -func UpdateService(actual corev1.Service, config v1alpha2.Service) corev1.Service { +func UpdateService(actual corev1.Service, config v1alpha2.Service, targetPort int32) corev1.Service { actual.ObjectMeta.Annotations = config.Annotations for key, value := range config.Labels { actual.ObjectMeta.Labels[key] = value @@ -30,6 +31,7 @@ func UpdateService(actual corev1.Service, config v1alpha2.Service) corev1.Servic actual.Spec.Ports = []corev1.ServicePort{{}} } actual.Spec.Ports[0].Port = config.Port + actual.Spec.Ports[0].TargetPort = intstr.IntOrString{IntVal: targetPort, Type: intstr.Int} if config.NodePort != 0 { actual.Spec.Ports[0].NodePort = config.NodePort } diff --git a/pkg/configuration/base/service.go b/pkg/configuration/base/service.go index 524c6a3b..3c9f2130 100644 --- a/pkg/configuration/base/service.go +++ b/pkg/configuration/base/service.go @@ -13,7 +13,7 @@ import ( "k8s.io/apimachinery/pkg/types" ) -func (r *ReconcileJenkinsBaseConfiguration) createService(meta metav1.ObjectMeta, name string, config v1alpha2.Service) error { +func (r *ReconcileJenkinsBaseConfiguration) createService(meta metav1.ObjectMeta, name string, config v1alpha2.Service, targetPort int32) error { service := corev1.Service{} err := r.Client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: meta.Namespace}, &service) if err != nil && apierrors.IsNotFound(err) { @@ -26,7 +26,7 @@ func (r *ReconcileJenkinsBaseConfiguration) createService(meta metav1.ObjectMeta Spec: corev1.ServiceSpec{ Selector: meta.Labels, }, - }, config) + }, config, targetPort) if err = r.CreateResource(&service); err != nil { return stackerr.WithStack(err) } @@ -35,6 +35,6 @@ func (r *ReconcileJenkinsBaseConfiguration) createService(meta metav1.ObjectMeta } service.Spec.Selector = meta.Labels // make sure that user won't break service by hand - service = resources.UpdateService(service, config) + service = resources.UpdateService(service, config, targetPort) return stackerr.WithStack(r.UpdateResource(&service)) } diff --git a/test/e2e/configuration_test.go b/test/e2e/configuration_test.go index f878eece..5d5cfc6b 100644 --- a/test/e2e/configuration_test.go +++ b/test/e2e/configuration_test.go @@ -10,6 +10,7 @@ import ( jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/client" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" + "github.com/jenkinsci/kubernetes-operator/pkg/constants" "github.com/jenkinsci/kubernetes-operator/pkg/groovy" "github.com/jenkinsci/kubernetes-operator/pkg/plugins" @@ -20,6 +21,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) const e2e = "e2e" @@ -92,6 +94,7 @@ func TestConfiguration(t *testing.T) { createKubernetesCredentialsProviderSecret(t, namespace, mySeedJob) waitForJenkinsBaseConfigurationToComplete(t, jenkins) verifyJenkinsMasterPodAttributes(t, jenkins) + verifyServices(t, jenkins) jenkinsClient, cleanUpFunc := verifyJenkinsAPIConnection(t, jenkins, namespace) defer cleanUpFunc() verifyPlugins(t, jenkinsClient, jenkins) @@ -370,6 +373,13 @@ if (!"%s".equals(Jenkins.instance.systemMessage)) { assert.NoError(t, err, logs) } +func verifyServices(t *testing.T, jenkins *v1alpha2.Jenkins) { + jenkinsHTTPService := getJenkinsService(t, jenkins, "http") + jenkinsSlaveService := getJenkinsService(t, jenkins, "slave") + assert.Equal(t, intstr.IntOrString{IntVal: constants.DefaultHTTPPortInt32, Type: intstr.Int}, jenkinsHTTPService.Spec.Ports[0].TargetPort) + assert.Equal(t, intstr.IntOrString{IntVal: constants.DefaultSlavePortInt32, Type: intstr.Int}, jenkinsSlaveService.Spec.Ports[0].TargetPort) +} + func assertMapContainsElementsFromAnotherMap(t *testing.T, expected map[string]string, actual map[string]string) { for key, expectedValue := range expected { actualValue, keyExists := actual[key] diff --git a/test/e2e/jenkins.go b/test/e2e/jenkins.go index 7eaabeb2..b66a8f58 100644 --- a/test/e2e/jenkins.go +++ b/test/e2e/jenkins.go @@ -2,6 +2,7 @@ package e2e import ( "context" + "fmt" "testing" "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" @@ -16,6 +17,7 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" @@ -236,3 +238,16 @@ func restartJenkinsMasterPod(t *testing.T, jenkins *v1alpha2.Jenkins) { } t.Log("Jenkins master pod has been restarted") } + +func getJenkinsService(t *testing.T, jenkins *v1alpha2.Jenkins, serviceKind string) *corev1.Service { + serviceName := constants.OperatorName + "-" + serviceKind + "-" + jenkins.ObjectMeta.Name + lo := metav1.ListOptions{ + FieldSelector: fields.SelectorFromSet(fields.Set{"metadata.name": serviceName}).String(), + } + serviceList, err := framework.Global.KubeClient.CoreV1().Services(jenkins.Namespace).List(lo) + + require.NoError(t, err) + require.Equal(t, 1, len(serviceList.Items), fmt.Sprintf("'%s' service not found", serviceName)) + + return &serviceList.Items[0] +}