Repository: kubesphere/ks-devops
Pull request #243 (2021-09-17 14:57:46)

Add support delete job history records once pipelinerun was deleted


Patch:
diff --git a/controllers/jenkins/pipelinerun/pipelinerun_controller.go b/controllers/jenkins/pipelinerun/pipelinerun_controller.go
index 81a7804..bd4c44c 100644
--- a/controllers/jenkins/pipelinerun/pipelinerun_controller.go
+++ b/controllers/jenkins/pipelinerun/pipelinerun_controller.go
@@ -28,12 +28,16 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
+	"k8s.io/klog"
 	prv1alpha3 "kubesphere.io/devops/pkg/api/devops/pipelinerun/v1alpha3"
 	"kubesphere.io/devops/pkg/api/devops/v1alpha3"
 	devopsClient "kubesphere.io/devops/pkg/client/devops"
+	"kubesphere.io/devops/pkg/utils/sliceutil"
 	"reflect"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"strconv"
+	"strings"
 	"time"
 )
 
@@ -58,19 +62,34 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	// get PipelineRun
 	var pr prv1alpha3.PipelineRun
-	if err := r.Client.Get(ctx, req.NamespacedName, &pr); err != nil {
+	var err error
+	if err = r.Client.Get(ctx, req.NamespacedName, &pr); err != nil {
 		log.Error(err, "unable to fetch PipelineRun")
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	// don't modify the cache in other places, like informer cache.
+	pr = *pr.DeepCopy()
+
+	// DeletionTimestamp.IsZero() means copyPipeline has not been deleted.
+	if !pr.ObjectMeta.DeletionTimestamp.IsZero() {
+		if err = r.deleteJenkinsJobHistory(&pr); err != nil {
+			klog.V(4).Infof("failed to delete Jenkins job history from PipelineRun: %s/%s, error: %v",
+				pr.Namespace, pr.Name, err)
+		} else {
+			pr.ObjectMeta.Finalizers = sliceutil.RemoveString(pr.ObjectMeta.Finalizers, func(item string) bool {
+				return item == prv1alpha3.PipelineRunFinalizerName
+			})
+			err = r.Update(context.TODO(), &pr)
+		}
+		return ctrl.Result{}, err
+	}
+
 	// the PipelineRun cannot allow building
 	if !pr.Buildable() {
 		return ctrl.Result{}, nil
 	}
 
-	// don't modify the cache in other places, like informer cache.
-	pr = *pr.DeepCopy()
-
 	// check PipelineRef
 	if pr.Spec.PipelineRef == nil || pr.Spec.PipelineRef.Name == "" {
 		// make the PipelineRun as orphan
@@ -129,6 +148,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		}
 		pr.Annotations[prv1alpha3.JenkinsPipelineRunStatusKey] = string(runResultJSON)
 		pr.Annotations[prv1alpha3.JenkinsPipelineRunStagesStatusKey] = string(prNodesJSON)
+
 		// update PipelineRun
 		if err := r.updateLabelsAndAnnotations(ctx, &pr); err != nil {
 			log.Error(err, "unable to update PipelineRun labels and annotations.")
@@ -186,6 +206,50 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	return ctrl.Result{RequeueAfter: 1 * time.Second}, nil
 }
 
+func (r *Reconciler) deleteJenkinsJobHistory(pipelineRun *prv1alpha3.PipelineRun) (err error) {
+	var buildNum int
+	if buildNum = getJenkinsBuildNumber(pipelineRun); buildNum < 0 {
+		return
+	}
+
+	jenkinsClient := job.Client{
+		JenkinsCore: r.JenkinsCore,
+	}
+	jobName := fmt.Sprintf("job/%s/job/%s", pipelineRun.Namespace, pipelineRun.Spec.PipelineRef.Name)
+	if err = jenkinsClient.DeleteHistory(jobName, buildNum); err != nil {
+		// TODO improve the way to check if the desired build record was deleted
+		if strings.Contains(err.Error(), "not found resources") {
+			err = nil
+		} else {
+			err = fmt.Errorf("failed to delete Jenkins job: %s, build: %d, error: %v", jobName, buildNum, err)
+		}
+	}
+	return
+}
+
+// getJenkinsBuildNumber returns the build number of a Jenkins job build which related with a PipelineRun
+// return a negative value if there is no valid build number
+func getJenkinsBuildNumber(pipelineRun *prv1alpha3.PipelineRun) (num int) {
+	num = -1
+
+	var (
+		buildNum      string
+		buildNumExist bool
+	)
+
+	if buildNum, buildNumExist = pipelineRun.GetPipelineRunID(); !buildNumExist {
+		return
+	}
+
+	var err error
+	if num, err = strconv.Atoi(buildNum); err != nil {
+		num = -1
+		klog.V(7).Infof("found an invalid build number from PipelineRun: %s/%s, err: %v",
+			pipelineRun.Namespace, pipelineRun.Name, err)
+	}
+	return
+}
+
 func (r *Reconciler) triggerJenkinsJob(devopsProjectName, pipelineName string, prSpec *prv1alpha3.PipelineRunSpec) (*job.PipelineBuild, error) {
 	c := job.BlueOceanClient{JenkinsCore: r.JenkinsCore, Organization: "jenkins"}
 
@@ -260,6 +324,10 @@ func (r *Reconciler) updateLabelsAndAnnotations(ctx context.Context, pr *prv1alp
 	prToUpdate = *prToUpdate.DeepCopy()
 	prToUpdate.Labels = pr.Labels
 	prToUpdate.Annotations = pr.Annotations
+	// make sure all PipelineRuns have the finalizer
+	if !sliceutil.HasString(prToUpdate.ObjectMeta.Finalizers, prv1alpha3.PipelineRunFinalizerName) {
+		prToUpdate.ObjectMeta.Finalizers = append(prToUpdate.ObjectMeta.Finalizers, prv1alpha3.PipelineRunFinalizerName)
+	}
 	return r.Update(ctx, &prToUpdate)
 }
 
diff --git a/pkg/api/devops/pipelinerun/v1alpha3/pipelinerun_types.go b/pkg/api/devops/pipelinerun/v1alpha3/pipelinerun_types.go
index c4e7fe0..00a49a9 100644
--- a/pkg/api/devops/pipelinerun/v1alpha3/pipelinerun_types.go
+++ b/pkg/api/devops/pipelinerun/v1alpha3/pipelinerun_types.go
@@ -24,6 +24,9 @@ import (
 	"time"
 )
 
+// PipelineRunFinalizerName is the name of PipelineRun finalizer
+const PipelineRunFinalizerName = "pipelinerun.finalizers.kubesphere.io"
+
 // PipelineRunSpec defines the desired state of PipelineRun
 type PipelineRunSpec struct {
 	// PipelineRef is the Pipeline to which the current PipelineRun belongs
diff --git a/pkg/kapis/devops/v1alpha2/devops.go b/pkg/kapis/devops/v1alpha2/devops.go
index 249c040..d921328 100644
--- a/pkg/kapis/devops/v1alpha2/devops.go
+++ b/pkg/kapis/devops/v1alpha2/devops.go
@@ -127,16 +127,18 @@ func (h *ProjectPipelineHandler) ListPipelines(req *restful.Request, resp *restf
 	// get all pipelines which come from ks
 	pipelineList := &clientDevOps.PipelineList{
 		Total: objs.TotalItems,
-		Items: make([]clientDevOps.Pipeline, 0, len(objs.Items)),
+		Items: make([]clientDevOps.Pipeline, len(objs.Items)),
 	}
-	pipelineMap := make(map[string]int)
-	for i := range objs.Items {
-		if pipeline, ok := objs.Items[i].(*v1alpha3.Pipeline); ok {
+	pipelineMap := make(map[string]int, pipelineList.Total)
+	for i, _ := range objs.Items {
+		if pipeline, ok := objs.Items[i].(*v1alpha3.Pipeline); !ok {
+			continue
+		} else {
 			pipelineMap[pipeline.Name] = i
-			pipelineList.Items = append(pipelineList.Items, clientDevOps.Pipeline{
+			pipelineList.Items[i] = clientDevOps.Pipeline{
 				Name:        pipeline.Name,
 				Annotations: pipeline.Annotations,
-			})
+			}
 		}
 	}
 
@@ -161,7 +163,8 @@ func (h *ProjectPipelineHandler) ListPipelines(req *restful.Request, resp *restf
 		}
 	}
 
-	_ = resp.WriteEntity(pipelineList)
+	resp.Header().Set(restful.HEADER_ContentType, restful.MIME_JSON)
+	resp.WriteAsJson(pipelineList)
 }
 
 func (h *ProjectPipelineHandler) GetPipelineRun(req *restful.Request, resp *restful.Response) {
diff --git a/pkg/models/resources/v1alpha3/interface.go b/pkg/models/resources/v1alpha3/interface.go
index 4421afc..a2f5cdb 100644
--- a/pkg/models/resources/v1alpha3/interface.go
+++ b/pkg/models/resources/v1alpha3/interface.go
@@ -98,9 +98,6 @@ func DefaultList(objects []runtime.Object, q *query.Query, compareFunc CompareFu
 	var filtered []runtime.Object
 	// filter objects
 	for _, object := range objects {
-		if object == nil || reflect.ValueOf(object).IsNil() {
-			continue
-		}
 		selected := true
 		for field, value := range q.Filters {
 			if filterFunc != nil && !filterFunc(object, query.Filter{Field: field, Value: value}) {
@@ -171,7 +168,7 @@ func DefaultFilter() FilterFunc {
 	return func(obj runtime.Object, filter query.Filter) bool {
 		oma, ok := obj.(metav1.ObjectMetaAccessor)
 		if !ok || oma == nil || reflect.ValueOf(oma).IsNil() {
-			return false
+			return true
 		}
 		return DefaultObjectMetaFilter(oma.GetObjectMeta(), filter)
 	}


Test patch:
diff --git a/controllers/jenkins/pipelinerun/pipelinerun_controller_test.go b/controllers/jenkins/pipelinerun/pipelinerun_controller_test.go
index 1101f48..6411d70 100644
--- a/controllers/jenkins/pipelinerun/pipelinerun_controller_test.go
+++ b/controllers/jenkins/pipelinerun/pipelinerun_controller_test.go
@@ -1,8 +1,20 @@
 package pipelinerun
 
 import (
+	"bytes"
+	"errors"
+	"fmt"
+	"github.com/golang/mock/gomock"
+	"github.com/jenkins-zh/jenkins-client/pkg/core"
+	"github.com/jenkins-zh/jenkins-client/pkg/mock/mhttp"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"io/ioutil"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	prv1alpha3 "kubesphere.io/devops/pkg/api/devops/pipelinerun/v1alpha3"
 	"kubesphere.io/devops/pkg/api/devops/v1alpha3"
+	"net/http"
 	"testing"
 )
 
@@ -78,3 +90,204 @@ func Test_getBranch(t *testing.T) {
 		})
 	}
 }
+
+func Test_getJenkinsBuildNumber(t *testing.T) {
+	type args struct {
+		pipelineRun *prv1alpha3.PipelineRun
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantNum int
+	}{{
+		name: "no build number",
+		args: args{
+			pipelineRun: &prv1alpha3.PipelineRun{},
+		},
+		wantNum: -1,
+	}, {
+		name: "invalid build number",
+		args: args{
+			pipelineRun: &prv1alpha3.PipelineRun{
+				ObjectMeta: v1.ObjectMeta{
+					Annotations: map[string]string{
+						prv1alpha3.JenkinsPipelineRunIDKey: "a",
+					},
+				},
+			},
+		},
+		wantNum: -1,
+	}, {
+		name: "valid build number",
+		args: args{
+			pipelineRun: &prv1alpha3.PipelineRun{
+				ObjectMeta: v1.ObjectMeta{
+					Annotations: map[string]string{
+						prv1alpha3.JenkinsPipelineRunIDKey: "2",
+					},
+				},
+			},
+		},
+		wantNum: 2,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if gotNum := getJenkinsBuildNumber(tt.args.pipelineRun); gotNum != tt.wantNum {
+				t.Errorf("getJenkinsBuildNumber() = %v, want %v", gotNum, tt.wantNum)
+			}
+		})
+	}
+}
+
+var _ = Describe("Test deleteJenkinsJobHistory", func() {
+	var (
+		ctrl         *gomock.Controller
+		roundTripper *mhttp.MockRoundTripper
+		reconciler   *Reconciler
+	)
+
+	BeforeEach(func() {
+		ctrl = gomock.NewController(GinkgoT())
+		roundTripper = mhttp.NewMockRoundTripper(ctrl)
+		reconciler = &Reconciler{
+			JenkinsCore: core.JenkinsCore{
+				URL:          "http://localhost",
+				UserName:     "",
+				Token:        "",
+				RoundTripper: roundTripper,
+			},
+		}
+	})
+
+	It("delete an empty PipelineRun", func() {
+		err := reconciler.deleteJenkinsJobHistory(&prv1alpha3.PipelineRun{})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("delete a valid PipelineRun", func() {
+		namespace := "project1"
+		pipelineName := "testPipeline"
+
+		requestCrumb, _ := http.NewRequest(http.MethodGet, "http://localhost/crumbIssuer/api/json", nil)
+		responseCrumb := &http.Response{
+			StatusCode: 200,
+			Proto:      "HTTP/1.1",
+			Request:    requestCrumb,
+			Body: ioutil.NopCloser(bytes.NewBufferString(`
+				{"crumbRequestField":"CrumbRequestField","crumb":"Crumb"}
+				`)),
+		}
+		roundTripper.EXPECT().
+			RoundTrip(core.NewRequestMatcher(requestCrumb)).Return(responseCrumb, nil)
+
+		request, _ := http.NewRequest(http.MethodPost, fmt.Sprintf("http://localhost/job/%s/job/%s/2/doDelete", namespace, pipelineName), nil)
+		request.Header.Set("CrumbRequestField", "Crumb")
+		response := &http.Response{
+			Request:    request,
+			StatusCode: http.StatusOK,
+		}
+		roundTripper.EXPECT().
+			RoundTrip(core.NewRequestMatcher(request)).Return(response, nil)
+
+		err := reconciler.deleteJenkinsJobHistory(&prv1alpha3.PipelineRun{
+			ObjectMeta: v1.ObjectMeta{
+				Namespace: namespace,
+				Annotations: map[string]string{
+					prv1alpha3.JenkinsPipelineRunIDKey: "2",
+				},
+			},
+			Spec: prv1alpha3.PipelineRunSpec{
+				PipelineRef: &corev1.ObjectReference{
+					Name: pipelineName,
+				},
+			},
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("to delete a not exist Jenkins build history", func() {
+		namespace := "project1"
+		pipelineName := "testPipeline"
+
+		requestCrumb, _ := http.NewRequest(http.MethodGet, "http://localhost/crumbIssuer/api/json", nil)
+		responseCrumb := &http.Response{
+			StatusCode: 200,
+			Proto:      "HTTP/1.1",
+			Request:    requestCrumb,
+			Body: ioutil.NopCloser(bytes.NewBufferString(`
+				{"crumbRequestField":"CrumbRequestField","crumb":"Crumb"}
+				`)),
+		}
+		roundTripper.EXPECT().
+			RoundTrip(core.NewRequestMatcher(requestCrumb)).Return(responseCrumb, nil)
+
+		request, _ := http.NewRequest(http.MethodPost, fmt.Sprintf("http://localhost/job/%s/job/%s/2/doDelete", namespace, pipelineName), nil)
+		request.Header.Set("CrumbRequestField", "Crumb")
+		response := &http.Response{
+			Request:    request,
+			StatusCode: http.StatusNotFound,
+		}
+		roundTripper.EXPECT().
+			RoundTrip(core.NewRequestMatcher(request)).Return(response, nil)
+
+		err := reconciler.deleteJenkinsJobHistory(&prv1alpha3.PipelineRun{
+			ObjectMeta: v1.ObjectMeta{
+				Namespace: namespace,
+				Annotations: map[string]string{
+					prv1alpha3.JenkinsPipelineRunIDKey: "2",
+				},
+			},
+			Spec: prv1alpha3.PipelineRunSpec{
+				PipelineRef: &corev1.ObjectReference{
+					Name: pipelineName,
+				},
+			},
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("failed to delete Jenkins build history", func() {
+		namespace := "project1"
+		pipelineName := "testPipeline"
+
+		requestCrumb, _ := http.NewRequest(http.MethodGet, "http://localhost/crumbIssuer/api/json", nil)
+		responseCrumb := &http.Response{
+			StatusCode: 200,
+			Proto:      "HTTP/1.1",
+			Request:    requestCrumb,
+			Body: ioutil.NopCloser(bytes.NewBufferString(`
+				{"crumbRequestField":"CrumbRequestField","crumb":"Crumb"}
+				`)),
+		}
+		roundTripper.EXPECT().
+			RoundTrip(core.NewRequestMatcher(requestCrumb)).Return(responseCrumb, nil)
+
+		request, _ := http.NewRequest(http.MethodPost, fmt.Sprintf("http://localhost/job/%s/job/%s/2/doDelete", namespace, pipelineName), nil)
+		request.Header.Set("CrumbRequestField", "Crumb")
+		response := &http.Response{
+			Request:    request,
+			StatusCode: http.StatusOK,
+		}
+		roundTripper.EXPECT().
+			RoundTrip(core.NewRequestMatcher(request)).Return(response, errors.New("failed"))
+
+		err := reconciler.deleteJenkinsJobHistory(&prv1alpha3.PipelineRun{
+			ObjectMeta: v1.ObjectMeta{
+				Namespace: namespace,
+				Annotations: map[string]string{
+					prv1alpha3.JenkinsPipelineRunIDKey: "2",
+				},
+			},
+			Spec: prv1alpha3.PipelineRunSpec{
+				PipelineRef: &corev1.ObjectReference{
+					Name: pipelineName,
+				},
+			},
+		})
+		Expect(err).To(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		ctrl.Finish()
+	})
+})
diff --git a/controllers/jenkins/pipelinerun/setup_test.go b/controllers/jenkins/pipelinerun/setup_test.go
new file mode 100644
index 0000000..36f6990
--- /dev/null
+++ b/controllers/jenkins/pipelinerun/setup_test.go
@@ -0,0 +1,16 @@
+package pipelinerun
+
+import (
+	"testing"
+
+	"github.com/onsi/ginkgo/reporters"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestPipelineRun(t *testing.T) {
+	RegisterFailHandler(Fail)
+	junitReporter := reporters.NewJUnitReporter("pipelinerun-test.xml")
+	RunSpecsWithDefaultAndCustomReporters(t, "test PipelineRun controller", []Reporter{junitReporter})
+}
diff --git a/pkg/models/resources/v1alpha3/interface_test.go b/pkg/models/resources/v1alpha3/interface_test.go
index 7b33c05..8d23ba1 100644
--- a/pkg/models/resources/v1alpha3/interface_test.go
+++ b/pkg/models/resources/v1alpha3/interface_test.go
@@ -405,39 +405,6 @@ func TestDefaultList(t *testing.T) {
 				},
 			},
 		},
-	}, {
-		name: "Filter nil items",
-		items: []runtime.Object{
-			&v1alpha3.Pipeline{
-				ObjectMeta: v1.ObjectMeta{
-					Name: "pipeline-abcd",
-				},
-			},
-			nil,
-			&v1alpha3.Pipeline{
-				ObjectMeta: v1.ObjectMeta{
-					Name: "pipeline-cdef",
-				},
-			},
-			nil,
-			nil,
-		},
-		filterFunc: DefaultFilter(),
-		expectedResult: api.ListResult{
-			TotalItems: 2,
-			Items: []interface{}{
-				&v1alpha3.Pipeline{
-					ObjectMeta: v1.ObjectMeta{
-						Name: "pipeline-abcd",
-					},
-				},
-				&v1alpha3.Pipeline{
-					ObjectMeta: v1.ObjectMeta{
-						Name: "pipeline-cdef",
-					},
-				},
-			},
-		},
 	}}
 	for _, item := range table {
 		t.Run(item.name, func(t *testing.T) {