Repository: jenkins-x/lighthouse
Pull request #1323 (2021-06-09 11:40:52)

fix: lets add a nicer default name for LighthouseJobs
so its easier to find the lighthousejob you want for a given repo/context


Patch:
diff --git a/pkg/engines/tekton/utils.go b/pkg/engines/tekton/utils.go
index 0d11be43..b120ec45 100644
--- a/pkg/engines/tekton/utils.go
+++ b/pkg/engines/tekton/utils.go
@@ -3,7 +3,6 @@ package tekton
 import (
 	"bytes"
 	"context"
-	"fmt"
 	"regexp"
 	"strconv"
 	"strings"
@@ -61,10 +60,11 @@ func makePipelineRun(ctx context.Context, lj v1alpha1.LighthouseJob, namespace s
 
 	prLabels, annotations := jobutil.LabelsAndAnnotationsForJob(lj, buildID)
 	specCopy := lj.Spec.PipelineRunSpec.DeepCopy()
+	generateName := jobutil.GenerateName(&lj.Spec)
 	p := tektonv1beta1.PipelineRun{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations:  annotations,
-			GenerateName: fmt.Sprintf("%s-", lj.Spec.Job),
+			GenerateName: generateName,
 			Namespace:    namespace,
 			Labels:       prLabels,
 		},
diff --git a/pkg/jobutil/jobutil.go b/pkg/jobutil/jobutil.go
index 647e3361..479998f8 100644
--- a/pkg/jobutil/jobutil.go
+++ b/pkg/jobutil/jobutil.go
@@ -30,12 +30,15 @@ import (
 	"github.com/jenkins-x/lighthouse/pkg/config/job"
 	"github.com/jenkins-x/lighthouse/pkg/scmprovider"
 	"github.com/jenkins-x/lighthouse/pkg/util"
-	uuid "github.com/satori/go.uuid"
 	"github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 )
 
+const (
+	maxGenerateNamePrefix = 32
+)
+
 // lighthouseClient a minimalistic lighthouse client required by the aborter
 type lighthouseClient interface {
 	Patch(name string, pt types.PatchType, data []byte, subresources ...string) (result *v1alpha1.LighthouseJob, err error)
@@ -44,17 +47,17 @@ type lighthouseClient interface {
 // NewLighthouseJob initializes a LighthouseJob out of a LighthouseJobSpec.
 func NewLighthouseJob(spec v1alpha1.LighthouseJobSpec, extraLabels, extraAnnotations map[string]string) v1alpha1.LighthouseJob {
 	labels, annotations := LabelsAndAnnotationsForSpec(spec, extraLabels, extraAnnotations)
-	newID, _ := uuid.NewV1()
 
+	generateName := GenerateName(&spec)
 	return v1alpha1.LighthouseJob{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "lighthouse.jenkins.io/v1alpha1",
 			Kind:       "LighthouseJob",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        newID.String(),
-			Labels:      labels,
-			Annotations: annotations,
+			GenerateName: generateName,
+			Labels:       labels,
+			Annotations:  annotations,
 		},
 		Spec: spec,
 	}
@@ -186,6 +189,40 @@ func completePrimaryRefs(refs v1alpha1.Refs, jb job.Base) *v1alpha1.Refs {
 	return &refs
 }
 
+// GenerateName generates a meaningful name for the LighthouseJob from the spec
+func GenerateName(spec *v1alpha1.LighthouseJobSpec) string {
+	if spec.Refs == nil {
+		return "missingref"
+	}
+
+	branch := spec.Refs.BaseRef
+	if len(spec.Refs.Pulls) > 0 {
+		branch = "pr-" + strconv.Itoa(spec.Refs.Pulls[0].Number)
+	}
+	name := addNonEmptyParts(spec.Refs.Org, spec.Refs.Repo, branch, spec.Context)
+	name = util.ToValidName(name)
+	if len(name) > maxGenerateNamePrefix {
+		name = name[len(name)-maxGenerateNamePrefix:]
+	}
+	name = strings.TrimPrefix(name, "-")
+	name = util.ToValidName(name)
+
+	if !strings.HasSuffix(name, "-") {
+		name += "-"
+	}
+	return name
+}
+
+func addNonEmptyParts(values ...string) string {
+	var parts []string
+	for _, v := range values {
+		if v != "" {
+			parts = append(parts, v)
+		}
+	}
+	return strings.Join(parts, "-")
+}
+
 // LighthouseJobFields extracts logrus fields from a LighthouseJob useful for logging.
 func LighthouseJobFields(lighthouseJob *v1alpha1.LighthouseJob) logrus.Fields {
 	fields := make(logrus.Fields)
diff --git a/pkg/webhook/create_agent.go b/pkg/webhook/create_agent.go
index 9dcff6ae..ddd052c8 100644
--- a/pkg/webhook/create_agent.go
+++ b/pkg/webhook/create_agent.go
@@ -32,6 +32,9 @@ import (
 func (s *Server) CreateAgent(l *logrus.Entry, owner, repo, ref string) (plugins.Agent, error) {
 	pc := plugins.NewAgent(s.ConfigAgent, s.Plugins, s.ClientAgent, s.ServerURL, l)
 	fullName := scm.Join(owner, repo)
+	if pc.Config == nil {
+		return pc, errors.Errorf("no config available. maybe the ConfigMap got deleted")
+	}
 	if !pc.Config.InRepoConfigEnabled(fullName) {
 		return pc, nil
 	}


Test patch:
diff --git a/pkg/engines/tekton/test_data/controller/start-batch-pullrequest/expected-pr.yml b/pkg/engines/tekton/test_data/controller/start-batch-pullrequest/expected-pr.yml
index 4ea4d6fd..83094e29 100644
--- a/pkg/engines/tekton/test_data/controller/start-batch-pullrequest/expected-pr.yml
+++ b/pkg/engines/tekton/test_data/controller/start-batch-pullrequest/expected-pr.yml
@@ -15,8 +15,8 @@ metadata:
     lighthouse.jenkins-x.io/refs.pull: "813"
     lighthouse.jenkins-x.io/refs.repo: lighthouse
     lighthouse.jenkins-x.io/type: presubmit
-  name: github-mz4c7
-  generateName: github-
+  name: nkins-x-lighthouse-pr-813-github-mz4c7
+  generateName: nkins-x-lighthouse-pr-813-github-
   namespace: jx
   resourceVersion: '1'
   ownerReferences:
diff --git a/pkg/engines/tekton/test_data/controller/start-pullrequest/expected-pr.yml b/pkg/engines/tekton/test_data/controller/start-pullrequest/expected-pr.yml
index 8f551991..ed7ca4d7 100644
--- a/pkg/engines/tekton/test_data/controller/start-pullrequest/expected-pr.yml
+++ b/pkg/engines/tekton/test_data/controller/start-pullrequest/expected-pr.yml
@@ -15,8 +15,8 @@ metadata:
     lighthouse.jenkins-x.io/refs.pull: "813"
     lighthouse.jenkins-x.io/refs.repo: lighthouse
     lighthouse.jenkins-x.io/type: presubmit
-  name: github-mz4c7
-  generateName: github-
+  name: nkins-x-lighthouse-pr-813-github-mz4c7
+  generateName: nkins-x-lighthouse-pr-813-github-
   namespace: jx
   resourceVersion: '1'
   ownerReferences:
diff --git a/pkg/engines/tekton/test_data/controller/start-push/expected-pr.yml b/pkg/engines/tekton/test_data/controller/start-push/expected-pr.yml
index 79608fd0..4cc9f0e8 100644
--- a/pkg/engines/tekton/test_data/controller/start-push/expected-pr.yml
+++ b/pkg/engines/tekton/test_data/controller/start-push/expected-pr.yml
@@ -14,8 +14,8 @@ metadata:
     lighthouse.jenkins-x.io/refs.org: jenkins-x
     lighthouse.jenkins-x.io/refs.repo: lighthouse
     lighthouse.jenkins-x.io/type: postsubmit
-  name: github-mz4c7
-  generateName: github-
+  name: jenkins-x-lighthouse-main-github-mz4c7
+  generateName: jenkins-x-lighthouse-main-github-
   namespace: jx
   resourceVersion: '1'
   ownerReferences:
diff --git a/pkg/jobutil/jobutil_test.go b/pkg/jobutil/jobutil_test.go
index 987e2496..b351ff23 100644
--- a/pkg/jobutil/jobutil_test.go
+++ b/pkg/jobutil/jobutil_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package jobutil
 
 import (
+	"github.com/stretchr/testify/assert"
 	"reflect"
 	"testing"
 
@@ -686,3 +687,73 @@ func TestPartitionActive(t *testing.T) {
 		}
 	}
 }
+
+func TestGenerateName(t *testing.T) {
+	tests := []struct {
+		expected string
+		spec     v1alpha1.LighthouseJobSpec
+	}{
+		{
+			expected: "myorg-myrepo-",
+			spec: v1alpha1.LighthouseJobSpec{
+				Refs: &v1alpha1.Refs{
+					Org:  "myorg",
+					Repo: "myrepo",
+				},
+			},
+		},
+		{
+			expected: "st-organsation-my-repo-",
+			spec: v1alpha1.LighthouseJobSpec{
+				Refs: &v1alpha1.Refs{
+					Org:  "1st.Organsation",
+					Repo: "MY_REPO",
+				},
+			},
+		},
+		{
+			expected: "myorg-myrepo-main-",
+			spec: v1alpha1.LighthouseJobSpec{
+				Refs: &v1alpha1.Refs{
+					Org:     "myorg",
+					Repo:    "myrepo",
+					BaseRef: "main",
+				},
+			},
+		},
+		{
+			expected: "myorg-myrepo-pr-123-",
+			spec: v1alpha1.LighthouseJobSpec{
+				Refs: &v1alpha1.Refs{
+					Org:  "myorg",
+					Repo: "myrepo",
+					Pulls: []v1alpha1.Pull{
+						{
+							Number: 123,
+						},
+					},
+				},
+			},
+		},
+		{
+			expected: "repo-with-very-long-name-pr-123-",
+			spec: v1alpha1.LighthouseJobSpec{
+				Refs: &v1alpha1.Refs{
+					Org:  "organisation-with-long-name",
+					Repo: "repo-with-very-long-name",
+					Pulls: []v1alpha1.Pull{
+						{
+							Number: 123,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		spec := &tc.spec
+		actual := GenerateName(spec)
+		assert.Equal(t, tc.expected, actual, "for spec %#v", spec)
+	}
+}