From 168110f84d9d5aef757f2a5e29ab3bb4fd4f43ea Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Thu, 25 Jul 2019 13:05:59 +0200 Subject: [PATCH 1/4] Fix groovy script versioning bug --- pkg/controller/jenkins/groovy/groovy.go | 2 +- pkg/controller/jenkins/groovy/groovy_test.go | 103 +++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/pkg/controller/jenkins/groovy/groovy.go b/pkg/controller/jenkins/groovy/groovy.go index ec9823ed..555d5e26 100644 --- a/pkg/controller/jenkins/groovy/groovy.go +++ b/pkg/controller/jenkins/groovy/groovy.go @@ -58,7 +58,7 @@ func (g *Groovy) EnsureSingle(source, name, hash, groovyScript string) (requeue var appliedGroovyScripts []v1alpha2.AppliedGroovyScript for _, ags := range g.jenkins.Status.AppliedGroovyScripts { - if ags.ConfigurationType != g.configurationType && ags.Source != source && ags.Name != name { + if ags.Source != source || ags.Name != name { appliedGroovyScripts = append(appliedGroovyScripts, ags) } } diff --git a/pkg/controller/jenkins/groovy/groovy_test.go b/pkg/controller/jenkins/groovy/groovy_test.go index 1650020a..fd3fe88f 100644 --- a/pkg/controller/jenkins/groovy/groovy_test.go +++ b/pkg/controller/jenkins/groovy/groovy_test.go @@ -177,6 +177,109 @@ func TestGroovy_EnsureSingle(t *testing.T) { assert.Equal(t, source, jenkins.Status.AppliedGroovyScripts[0].Source) assert.Equal(t, groovyScriptName, jenkins.Status.AppliedGroovyScripts[0].Name) }) + t.Run("execute three groovy scripts with another hash", func(t *testing.T) { + // given + firstGroovyScriptName := "testGroovy1" + firstGroovyScriptHash := "testHash1" + + secondGroovyScriptName := "testGroovy2" + secondGroovyScriptHash := "testHash2" + + thirdGroovyScriptName := "testGroovy3" + thirdGroovyScriptHash := "testHash3" + + jenkins := &v1alpha2.Jenkins{ + ObjectMeta: metav1.ObjectMeta{ + Name: jenkinsName, + Namespace: namespace, + }, + } + err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme) + require.NoError(t, err) + fakeClient := fake.NewFakeClient() + err = fakeClient.Create(ctx, jenkins) + require.NoError(t, err) + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) + + jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) + jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) + jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) + + groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + + requeue, err := groovyClient.EnsureSingle(source, firstGroovyScriptName, firstGroovyScriptHash, groovyScript) + + // then + require.NoError(t, err) + assert.True(t, requeue) + err = fakeClient.Get(ctx, types.NamespacedName{Name: jenkins.Name, Namespace: jenkins.Namespace}, jenkins) + require.NoError(t, err) + assert.Equal(t, 1, len(jenkins.Status.AppliedGroovyScripts)) + assert.Equal(t, configurationType, jenkins.Status.AppliedGroovyScripts[0].ConfigurationType) + assert.Equal(t, firstGroovyScriptHash, jenkins.Status.AppliedGroovyScripts[0].Hash) + assert.Equal(t, source, jenkins.Status.AppliedGroovyScripts[0].Source) + assert.Equal(t, firstGroovyScriptName, jenkins.Status.AppliedGroovyScripts[0].Name) + + requeue, err = groovyClient.EnsureSingle(source, secondGroovyScriptName, secondGroovyScriptHash, groovyScript) + // then + require.NoError(t, err) + assert.True(t, requeue) + err = fakeClient.Get(ctx, types.NamespacedName{Name: jenkins.Name, Namespace: jenkins.Namespace}, jenkins) + require.NoError(t, err) + assert.Equal(t, 2, len(jenkins.Status.AppliedGroovyScripts)) + assert.Equal(t, configurationType, jenkins.Status.AppliedGroovyScripts[1].ConfigurationType) + assert.Equal(t, secondGroovyScriptHash, jenkins.Status.AppliedGroovyScripts[1].Hash) + assert.Equal(t, source, jenkins.Status.AppliedGroovyScripts[1].Source) + assert.Equal(t, secondGroovyScriptName, jenkins.Status.AppliedGroovyScripts[1].Name) + + requeue, err = groovyClient.EnsureSingle(source, thirdGroovyScriptName, thirdGroovyScriptHash, groovyScript) + // then + require.NoError(t, err) + assert.True(t, requeue) + err = fakeClient.Get(ctx, types.NamespacedName{Name: jenkins.Name, Namespace: jenkins.Namespace}, jenkins) + require.NoError(t, err) + assert.Equal(t, 3, len(jenkins.Status.AppliedGroovyScripts)) + assert.Equal(t, configurationType, jenkins.Status.AppliedGroovyScripts[2].ConfigurationType) + assert.Equal(t, thirdGroovyScriptHash, jenkins.Status.AppliedGroovyScripts[2].Hash) + assert.Equal(t, source, jenkins.Status.AppliedGroovyScripts[2].Source) + assert.Equal(t, thirdGroovyScriptName, jenkins.Status.AppliedGroovyScripts[2].Name) + + }) + t.Run("execute two groovy scripts with same names in two config maps", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + ObjectMeta: metav1.ObjectMeta{ + Name: jenkinsName, + Namespace: namespace, + }, + } + err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme) + require.NoError(t, err) + fakeClient := fake.NewFakeClient() + err = fakeClient.Create(ctx, jenkins) + require.NoError(t, err) + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) + + jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) + jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) + + groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + + requeue, err := groovyClient.EnsureSingle("test-conf1", "test.groovy", hash, groovyScript) + require.NoError(t, err) + assert.True(t, requeue) + + requeue, err = groovyClient.EnsureSingle("test-conf2", "test.groovy", "anotherHash", groovyScript) + require.NoError(t, err) + assert.True(t, requeue) + + assert.Equal(t, 2, len(jenkins.Status.AppliedGroovyScripts)) + }) t.Run("execute script fails", func(t *testing.T) { // given jenkins := &v1alpha2.Jenkins{ From 7a149c82579fb1c83039f4acb294e15f39fa7e97 Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Thu, 25 Jul 2019 13:20:22 +0200 Subject: [PATCH 2/4] Improve tests --- pkg/controller/jenkins/groovy/groovy_test.go | 34 ++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/pkg/controller/jenkins/groovy/groovy_test.go b/pkg/controller/jenkins/groovy/groovy_test.go index fd3fe88f..b9a7a09b 100644 --- a/pkg/controller/jenkins/groovy/groovy_test.go +++ b/pkg/controller/jenkins/groovy/groovy_test.go @@ -280,6 +280,40 @@ func TestGroovy_EnsureSingle(t *testing.T) { assert.Equal(t, 2, len(jenkins.Status.AppliedGroovyScripts)) }) + t.Run("execute two groovy scripts with different configuration types", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + ObjectMeta: metav1.ObjectMeta{ + Name: jenkinsName, + Namespace: namespace, + }, + } + err := v1alpha2.SchemeBuilder.AddToScheme(scheme.Scheme) + require.NoError(t, err) + fakeClient := fake.NewFakeClient() + err = fakeClient.Create(ctx, jenkins) + require.NoError(t, err) + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) + + jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) + jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) + + groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + + requeue, err := groovyClient.EnsureSingle("test-conf1", "test.groovy", hash, groovyScript) + require.NoError(t, err) + assert.True(t, requeue) + + groovyClient = New(jenkinsClient, fakeClient, log.Log, jenkins, "another-test-configuration-type", emptyCustomization) + + requeue, err = groovyClient.EnsureSingle("test-conf2", "test.groovy", "anotherHash", groovyScript) + require.NoError(t, err) + assert.True(t, requeue) + + assert.Equal(t, 2, len(jenkins.Status.AppliedGroovyScripts)) + }) t.Run("execute script fails", func(t *testing.T) { // given jenkins := &v1alpha2.Jenkins{ From 063f0c074b94932469b1c2971c9d0dac4a9a2e89 Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Wed, 31 Jul 2019 10:51:36 +0200 Subject: [PATCH 3/4] Improve unit tests --- pkg/controller/jenkins/groovy/groovy.go | 2 +- pkg/controller/jenkins/groovy/groovy_test.go | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/controller/jenkins/groovy/groovy.go b/pkg/controller/jenkins/groovy/groovy.go index 555d5e26..15b594f7 100644 --- a/pkg/controller/jenkins/groovy/groovy.go +++ b/pkg/controller/jenkins/groovy/groovy.go @@ -58,7 +58,7 @@ func (g *Groovy) EnsureSingle(source, name, hash, groovyScript string) (requeue var appliedGroovyScripts []v1alpha2.AppliedGroovyScript for _, ags := range g.jenkins.Status.AppliedGroovyScripts { - if ags.Source != source || ags.Name != name { + if g.configurationType != ags.ConfigurationType || ags.Source != source || ags.Name != name { appliedGroovyScripts = append(appliedGroovyScripts, ags) } } diff --git a/pkg/controller/jenkins/groovy/groovy_test.go b/pkg/controller/jenkins/groovy/groovy_test.go index b9a7a09b..3bdd20ec 100644 --- a/pkg/controller/jenkins/groovy/groovy_test.go +++ b/pkg/controller/jenkins/groovy/groovy_test.go @@ -296,19 +296,18 @@ func TestGroovy_EnsureSingle(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) - jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) - requeue, err := groovyClient.EnsureSingle("test-conf1", "test.groovy", hash, groovyScript) + requeue, err := groovyClient.EnsureSingle(source, "test.groovy", hash, groovyScript) require.NoError(t, err) assert.True(t, requeue) groovyClient = New(jenkinsClient, fakeClient, log.Log, jenkins, "another-test-configuration-type", emptyCustomization) - requeue, err = groovyClient.EnsureSingle("test-conf2", "test.groovy", "anotherHash", groovyScript) + requeue, err = groovyClient.EnsureSingle(source, "test.groovy", "anotherHash", groovyScript) require.NoError(t, err) assert.True(t, requeue) From 3a7a4049f3ebede040eab5501bb92455bd7b7da9 Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Wed, 31 Jul 2019 11:11:15 +0200 Subject: [PATCH 4/4] Improve groovy script checker --- pkg/controller/jenkins/groovy/groovy.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/controller/jenkins/groovy/groovy.go b/pkg/controller/jenkins/groovy/groovy.go index 15b594f7..399ec216 100644 --- a/pkg/controller/jenkins/groovy/groovy.go +++ b/pkg/controller/jenkins/groovy/groovy.go @@ -58,9 +58,11 @@ func (g *Groovy) EnsureSingle(source, name, hash, groovyScript string) (requeue var appliedGroovyScripts []v1alpha2.AppliedGroovyScript for _, ags := range g.jenkins.Status.AppliedGroovyScripts { - if g.configurationType != ags.ConfigurationType || ags.Source != source || ags.Name != name { - appliedGroovyScripts = append(appliedGroovyScripts, ags) + if g.configurationType == ags.ConfigurationType && ags.Source == source && ags.Name == name { + continue } + + appliedGroovyScripts = append(appliedGroovyScripts, ags) } appliedGroovyScripts = append(appliedGroovyScripts, v1alpha2.AppliedGroovyScript{ ConfigurationType: g.configurationType,