Improve tests

pkg/controller/jenkins/client/jenkins.go

@@ -57,7 +57,6 @@ type Jenkins interface {
 	Poll() (int, error)
 	ExecuteScript(groovyScript string) (logs string, err error)
 	GetNodeSecret(name string) (string, error)
-	IsNotFoundError(err error) bool
 }
 
 type jenkins struct {
@@ -68,7 +67,7 @@ type jenkins struct {
 func (jenkins *jenkins) CreateOrUpdateJob(config, jobName string) (job *gojenkins.Job, created bool, err error) {
 	// create or update
 	job, err = jenkins.GetJob(jobName)
-	if jenkins.IsNotFoundError(err) {
+	if isNotFoundError(err) {
 		job, err = jenkins.CreateJob(config, jobName)
 		created = true
 		return job, true, errors.WithStack(err)
@@ -136,7 +135,7 @@ func New(url, user, passwordOrToken string) (Jenkins, error) {
 	return jenkinsClient, nil
 }
 
-func (jenkins *jenkins) IsNotFoundError(err error) bool {
+func isNotFoundError(err error) bool {
 	if err != nil {
 		return err.Error() == errorNotFound.Error()
 	}

pkg/controller/jenkins/client/mockgen.go

@@ -138,20 +138,6 @@ func (m *MockJenkins) CreateFolder(name string, parents ...string) (*gojenkins.F
 	return ret0, ret1
 }
 
-// IsNotFoundError mocks base method
-func (m *MockJenkins) IsNotFoundError(err error) bool {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "IsNotFoundError", err)
-	ret0, _ := ret[0].(bool)
-	return ret0
-}
-
-// IsNotFoundError indicates an expected call of IsNotFoundError
-func (mr *MockJenkinsMockRecorder) IsNotFoundError(err error) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNotFoundError", reflect.TypeOf((*MockJenkins)(nil).IsNotFoundError), err)
-}
-
 // CreateFolder indicates an expected call of CreateFolder
 func (mr *MockJenkinsMockRecorder) CreateFolder(name interface{}, parents ...interface{}) *gomock.Call {
 	mr.mock.ctrl.T.Helper()

pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go

@@ -227,7 +227,7 @@ func (s SeedJobs) createAgent(jenkinsClient jenkinsclient.Jenkins, k8sClient cli
 	_, err := jenkinsClient.GetNode(agentName)
 
 	// Create node if not exists
-	if jenkinsClient.IsNotFoundError(err) {
+	if err != nil && err.Error() == "No node found" {
 		_, err = jenkinsClient.CreateNode(agentName, 1, "The jenkins-operator generated agent", "/home/jenkins", agentName)
 		if err != nil {
 			return err

pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go

@@ -2,7 +2,6 @@ package seedjobs
 
 import (
 	"context"
-	"fmt"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"testing"
 
@@ -23,100 +22,6 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 )
 
-func TestEnsureSeedJobs(t *testing.T) {
-	t.Run("happy", func(t *testing.T) {
-		// given
-		logger := logf.ZapLogger(false)
-		ctrl := gomock.NewController(t)
-		ctx := context.TODO()
-		defer ctrl.Finish()
-
-		jenkinsClient := jenkinsclient.NewMockJenkins(ctrl)
-		fakeClient := fake.NewFakeClient()
-		err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme)
-		assert.NoError(t, err)
-
-		jenkins := jenkinsCustomResource()
-		err = fakeClient.Create(ctx, jenkins)
-		assert.NoError(t, err)
-
-		agentName := "jnlp"
-		secret := "test-secret"
-		testNode := &gojenkins.Node{
-			Raw: &gojenkins.NodeResponse{
-				DisplayName: agentName,
-			},
-		}
-
-		jobID := jenkins.Spec.SeedJobs[0].ID
-
-		jenkinsClient.EXPECT().IsNotFoundError(nil).AnyTimes()
-		jenkinsClient.EXPECT().GetJob(jobID).AnyTimes()
-		jenkinsClient.EXPECT().GetNodeSecret(agentName).Return(secret, nil).AnyTimes()
-		jenkinsClient.EXPECT().GetAllNodes().Return([]*gojenkins.Node{}, nil).AnyTimes()
-		jenkinsClient.EXPECT().CreateNode(agentName, 1, "The jenkins-operator generated agent", "/home/jenkins", agentName).Return(testNode, nil).AnyTimes()
-		jenkinsClient.EXPECT().GetNode(agentName).Return(testNode, nil).AnyTimes()
-
-		jenkinsClient.EXPECT().ExecuteScript(seedJobCreatingGroovyScript(jenkins.Spec.SeedJobs[0])).AnyTimes()
-
-		seedJobClient := New(jenkinsClient, fakeClient, logger)
-
-		_, err = seedJobClient.EnsureSeedJobs(jenkins)
-		assert.NoError(t, err)
-
-		_, err = jenkinsClient.GetJob(jobID)
-
-		assert.False(t, jenkinsClient.IsNotFoundError(err))
-	})
-
-	t.Run("delete pod when no seed jobs", func(t *testing.T) {
-		// given
-		ctrl := gomock.NewController(t)
-		ctx := context.TODO()
-		defer ctrl.Finish()
-
-		namespace := "test-namespace"
-		agentName := "test-agent"
-		secret := "test-secret"
-		jenkinsCustomRes := jenkinsCustomResource()
-		jenkinsCustomRes.Spec.SeedJobs = []v1alpha2.SeedJob{}
-
-		jenkinsClient := jenkinsclient.NewMockJenkins(ctrl)
-		fakeClient := fake.NewFakeClient()
-		err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme)
-		assert.NoError(t, err)
-
-		jenkinsClient.EXPECT().IsNotFoundError(nil).AnyTimes()
-		jenkinsClient.EXPECT().GetNode(agentName).AnyTimes()
-		jenkinsClient.EXPECT().GetNodeSecret(agentName).Return(secret, nil).AnyTimes()
-		jenkinsClient.EXPECT().GetAllNodes().Return([]*gojenkins.Node{}, nil).AnyTimes()
-		jenkinsClient.EXPECT().CreateNode(agentName, 1, "The jenkins-operator generated agent", "/home/jenkins", agentName).AnyTimes()
-
-		seedJobsClient := New(jenkinsClient, fakeClient, nil)
-
-		err = fakeClient.Create(ctx, &appsv1.Deployment{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      fmt.Sprintf("%s-deployment", agentName),
-				Namespace: namespace,
-			},
-		})
-
-		assert.NoError(t, err)
-
-		// when
-		_, err = seedJobsClient.EnsureSeedJobs(jenkinsCustomRes)
-		assert.NoError(t, err)
-
-		// then
-		var deployment appsv1.Deployment
-		err = fakeClient.Get(ctx, types.NamespacedName{Name: agentName, Namespace: namespace}, &deployment)
-
-		if !errors.IsNotFound(err) {
-			t.Fatal("deployment not removed")
-		}
-	})
-}
-
 func jenkinsCustomResource() *v1alpha2.Jenkins {
 	return &v1alpha2.Jenkins{
 		ObjectMeta: metav1.ObjectMeta{
@@ -157,6 +62,130 @@ func jenkinsCustomResource() *v1alpha2.Jenkins {
 	}
 }
 
+func TestEnsureSeedJobs(t *testing.T) {
+	t.Run("happy", func(t *testing.T) {
+		// given
+		logger := logf.ZapLogger(false)
+		ctrl := gomock.NewController(t)
+		ctx := context.TODO()
+		defer ctrl.Finish()
+
+		jenkinsClient := jenkinsclient.NewMockJenkins(ctrl)
+		fakeClient := fake.NewFakeClient()
+		err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme)
+		assert.NoError(t, err)
+
+		jenkins := jenkinsCustomResource()
+		err = fakeClient.Create(ctx, jenkins)
+		assert.NoError(t, err)
+
+		agentName := "jnlp"
+		agentSecret := "test-secret"
+		testNode := &gojenkins.Node{
+			Raw: &gojenkins.NodeResponse{
+				DisplayName: agentName,
+			},
+		}
+
+		jenkinsClient.EXPECT().GetNode(agentName).Return(nil, nil).AnyTimes()
+		jenkinsClient.EXPECT().CreateNode(agentName, 1, "The jenkins-operator generated agent", "/home/jenkins", agentName).Return(testNode, nil).AnyTimes()
+		jenkinsClient.EXPECT().GetNodeSecret(agentName).Return(agentSecret, nil).AnyTimes()
+		jenkinsClient.EXPECT().ExecuteScript(seedJobCreatingGroovyScript(jenkins.Spec.SeedJobs[0])).AnyTimes()
+
+		seedJobClient := New(jenkinsClient, fakeClient, logger)
+
+		// when
+		_, err = seedJobClient.EnsureSeedJobs(jenkins)
+
+		// then
+		assert.NoError(t, err)
+
+		var agentDeployment appsv1.Deployment
+		err = fakeClient.Get(ctx, types.NamespacedName{Namespace: jenkins.Namespace, Name: agentName}, &agentDeployment)
+		assert.NoError(t, err)
+	})
+
+	t.Run("delete agent deployment when no seed jobs", func(t *testing.T) {
+		// given
+		ctrl := gomock.NewController(t)
+		ctx := context.TODO()
+		defer ctrl.Finish()
+
+		agentName := "test-agent"
+		agentSecret := "test-secret"
+		jenkins := jenkinsCustomResource()
+		jenkins.Spec.SeedJobs = []v1alpha2.SeedJob{}
+
+		jenkinsClient := jenkinsclient.NewMockJenkins(ctrl)
+		fakeClient := fake.NewFakeClient()
+		err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme)
+		assert.NoError(t, err)
+
+		jenkinsClient.EXPECT().GetNode(agentName).AnyTimes()
+		jenkinsClient.EXPECT().CreateNode(agentName, 1, "The jenkins-operator generated agent", "/home/jenkins", agentName).AnyTimes()
+		jenkinsClient.EXPECT().GetNodeSecret(agentName).Return(agentSecret, nil).AnyTimes()
+
+		seedJobsClient := New(jenkinsClient, fakeClient, nil)
+
+		err = fakeClient.Create(ctx, &appsv1.Deployment{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      agentName,
+				Namespace: jenkins.Namespace,
+			},
+		})
+		assert.NoError(t, err)
+
+		// when
+		_, err = seedJobsClient.EnsureSeedJobs(jenkins)
+
+		// then
+		assert.NoError(t, err)
+
+		var deployment appsv1.Deployment
+		err = fakeClient.Get(ctx, types.NamespacedName{Name: agentName, Namespace: jenkins.Namespace}, &deployment)
+
+		assert.False(t, errors.IsNotFound(err), "Agent deployment hasn't been deleted")
+	})
+}
+
+func TestCreateAgent(t *testing.T) {
+	t.Run("don't fail when deployment is already created", func(t *testing.T) {
+		// given
+		ctrl := gomock.NewController(t)
+		ctx := context.TODO()
+		defer ctrl.Finish()
+
+		agentName := "test-agent"
+		agentSecret := "test-secret"
+		jenkins := jenkinsCustomResource()
+
+		jenkinsClient := jenkinsclient.NewMockJenkins(ctrl)
+		fakeClient := fake.NewFakeClient()
+		err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme)
+		assert.NoError(t, err)
+
+		jenkinsClient.EXPECT().GetNode(agentName).AnyTimes()
+		jenkinsClient.EXPECT().CreateNode(agentName, 1, "The jenkins-operator generated agent", "/home/jenkins", agentName).AnyTimes()
+		jenkinsClient.EXPECT().GetNodeSecret(agentName).Return(agentSecret, nil).AnyTimes()
+
+		seedJobsClient := New(jenkinsClient, fakeClient, nil)
+
+		err = fakeClient.Create(ctx, &appsv1.Deployment{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      agentName,
+				Namespace: jenkins.Namespace,
+			},
+		})
+		assert.NoError(t, err)
+
+		// when
+		err = seedJobsClient.createAgent(jenkinsClient, fakeClient, jenkinsCustomResource(), jenkins.Namespace, agentName)
+
+		// then
+		assert.NoError(t, err)
+	})
+}
+
 func TestSeedJobs_isRecreatePodNeeded(t *testing.T) {
 	seedJobsClient := New(nil, nil, nil)
 	t.Run("empty", func(t *testing.T) {
@@ -224,43 +253,3 @@ func TestSeedJobs_isRecreatePodNeeded(t *testing.T) {
 		assert.True(t, got)
 	})
 }
-
-func TestCreateAgent(t *testing.T) {
-	t.Run("not fail when deployment is available", func(t *testing.T) {
-		// given
-		ctrl := gomock.NewController(t)
-		ctx := context.TODO()
-		defer ctrl.Finish()
-
-		namespace := "test-namespace"
-		agentName := "test-agent"
-		secret := "test-secret"
-
-		jenkinsClient := jenkinsclient.NewMockJenkins(ctrl)
-		fakeClient := fake.NewFakeClient()
-		err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme)
-		assert.NoError(t, err)
-
-		jenkinsClient.EXPECT().IsNotFoundError(nil).AnyTimes()
-		jenkinsClient.EXPECT().GetNode(agentName).AnyTimes()
-		jenkinsClient.EXPECT().GetNodeSecret(agentName).Return(secret, nil).AnyTimes()
-		jenkinsClient.EXPECT().GetAllNodes().Return([]*gojenkins.Node{}, nil).AnyTimes()
-		jenkinsClient.EXPECT().CreateNode(agentName, 1, "The jenkins-operator generated agent", "/home/jenkins", agentName).AnyTimes()
-
-		seedJobsClient := New(jenkinsClient, fakeClient, nil)
-
-		// when
-		err = fakeClient.Create(ctx, &appsv1.Deployment{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      fmt.Sprintf("%s-deployment", agentName),
-				Namespace: namespace,
-			},
-		})
-
-		assert.NoError(t, err)
-
-		// then
-		err = seedJobsClient.createAgent(jenkinsClient, fakeClient, jenkinsCustomResource(), namespace, agentName)
-		assert.NoError(t, err)
-	})
-}