Repository: kubesphere/ks-devops
Pull request #969 (2023-07-06 09:03:30)

fix: Solve the problem of failure to download multi-branch-pipeline artifacts
<!-- Thanks for sending a pull request! Here are some tips for you:

1. If you want **faster** PR reviews, read how: https://github.com/kubesphere/community/blob/master/developer-guide/development/the-pr-author-guide-to-getting-through-code-review.md
2. In case you want to know how your PR got reviewed, read: https://github.com/kubesphere/community/blob/master/developer-guide/development/code-review-guide.md
3. Here are some coding conventions followed by the KubeSphere community: https://github.com/kubesphere/community/blob/master/developer-guide/development/coding-conventions.md
4. Additional open-source best practice: https://github.com/LinuxSuRen/open-source-best-practice
-->

### What type of PR is this?
/kind bug
<!-- 
Add one of the following kinds:
/kind bug

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
-->

### What this PR does / why we need it:
Multi-branch pipeline artifacts download failed
jenkins-client pr: https://github.com/jenkins-zh/jenkins-client/pull/64

### Which issue(s) this PR fixes:
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
Please leave it or change # to be None if there is no corresponding issue that exists
-->
Fixes #https://github.com/kubesphere/kubesphere/issues/5821

### Special notes for reviewers:
<!--
You can use the following command to let the DevOps SIG members help you to review your PR.
/cc @kubesphere/sig-devops 
And please avoid cc any individual.
-->
Please check the following list before waiting reviewers:

- [ ] Already committed the CRD files to [the Helm Chart](https://github.com/kubesphere-sigs/ks-devops-helm-chart/) if you created some new CRDs
- [ ] Already [added the permission](https://github.com/kubesphere/ks-installer/blob/9e063b085a0e43fdb3d0d9e3e7f4149146f14b9c/roles/ks-core/prepare/files/ks-init/role-templates.yaml) for the new API
- [ ] Already added the RBAC markers for the new controllers

### Does this PR introduce a user-facing change??
None
<!--
If no, just write "None" in the release-note block below.
If yes, a release note is required:
Enter your extended-release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".

Please keep the note be same as your PR title if you believe it should be in the release notes.
-->
```release-note
fixed the bug with downloading multi-branch-pipeline artifacts

```


ks-ci-bot: @littlejiancc: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our [release note process](https://git.k8s.io/community/contributors/guide/release-notes.md) to remove it.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
ks-ci-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **yudong2015** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @yudong2015` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubesphere%2Fks-devops).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubesphere/ks-devops/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["yudong2015"]} -->
littlejiancc: @chilianyi done
chilianyi: > @chilianyi done

Thanks for your pr. 

Please merge the commit records through git rebase, now there are five commit, it will be better to merge them into one commit. 
littlejiancc: > 
Even if there are 5 commits, but the pr is combined into one.

littlejiancc: > > 
> 
> Even if there are 5 commits, but the pr is combined into one.


littlejiancc: > > @chilianyi done
> 
> Thanks for your pr.
> 
> Please merge the commit records through git rebase, now there are five commit, it will be better to merge them into one commit.

Even if there are 5 commits, but the pr is combined into one.
chilianyi: > Even if there are 5 commits, but the pr is combined into one.

Yes, but this helps to keep the commit history clean and concise, making it clearer when viewing the changelog directly.
Thanks.
littlejiancc: > Yes, but this helps to keep the commit history clean and concise, making it clearer when viewing the changelog directly. Thanks.

Ok, I understand, and done it.
yudong2015: /lgtm
/approve
ks-ci-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubesphere/ks-devops/pull/969#issuecomment-1627756530" title="Approved">yudong2015</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubesphere%2Fks-devops).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubesphere/ks-devops/blob/master/OWNERS)~~ [yudong2015]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/go.mod b/go.mod
index 53919d9..d00ceb9 100644
--- a/go.mod
+++ b/go.mod
@@ -20,7 +20,7 @@ require (
 	github.com/google/go-cmp v0.5.5
 	github.com/h2non/gock v1.0.9
 	github.com/jenkins-x/go-scm v1.11.19
-	github.com/jenkins-zh/jenkins-client v0.0.14-0.20220905100332-0c9041a612a1
+	github.com/jenkins-zh/jenkins-client v0.0.15-0.20230706113353-4db299897849
 	github.com/jenkins-zh/jenkins-client/pkg/k8s v0.0.0-20220905100332-0c9041a612a1
 	github.com/kubesphere/sonargo v0.0.2
 	github.com/onsi/ginkgo v1.16.5
diff --git a/go.sum b/go.sum
index 43d8094..53858b0 100644
--- a/go.sum
+++ b/go.sum
@@ -391,8 +391,8 @@ github.com/jenkins-x/go-scm v1.11.19 h1:H4CzaM/C/0QcCVLDh603Q6Bv4hqU4G3De2yQntWu
 github.com/jenkins-x/go-scm v1.11.19/go.mod h1:eIcty4+tf6E7ycGOg0cUqnaLP+1LH1Z8zncQFQqRa3E=
 github.com/jenkins-zh/jenkins-cli v0.0.32/go.mod h1:uE1mH9PNITrg0sugv6HXuM/CSddg0zxXoYu3w57I3JY=
 github.com/jenkins-zh/jenkins-client v0.0.13/go.mod h1:ICBk7OOoTafVP//f/VfKZ34c0ff8vJwVnOsF9btiMYU=
-github.com/jenkins-zh/jenkins-client v0.0.14-0.20220905100332-0c9041a612a1 h1:FqiUegQ9pu3n4l7KtvX6uOBg6dV4BndXUkJeuDT896c=
-github.com/jenkins-zh/jenkins-client v0.0.14-0.20220905100332-0c9041a612a1/go.mod h1:T8M/g0p7jjx0NBwN6gNqsAAmT1P3B70GBBJJgsX9kLw=
+github.com/jenkins-zh/jenkins-client v0.0.15-0.20230706113353-4db299897849 h1:1KwWQPHTji7gs/0vvFM6w0yvoSRixDOLcZ0bPqKJT3k=
+github.com/jenkins-zh/jenkins-client v0.0.15-0.20230706113353-4db299897849/go.mod h1:T8M/g0p7jjx0NBwN6gNqsAAmT1P3B70GBBJJgsX9kLw=
 github.com/jenkins-zh/jenkins-client/pkg/k8s v0.0.0-20220905100332-0c9041a612a1 h1:ZfmezrO6f1VCUeP+f7skW6U11a5NmdIiFU6giMDqjB0=
 github.com/jenkins-zh/jenkins-client/pkg/k8s v0.0.0-20220905100332-0c9041a612a1/go.mod h1:Gfq/TcOliP3fB+kNBpv/Hrx8XGt0Bz0VPJ7BxdY/u2U=
 github.com/jenkins-zh/jenkins-formulas v0.0.5/go.mod h1:zS8fm8u5L6FcjZM0QznXsLV9T2UtSVK+hT6Sm76iUZ4=
diff --git a/pkg/api/devops/v1alpha3/pipelinerun_types.go b/pkg/api/devops/v1alpha3/pipelinerun_types.go
index 56cb3b4..825d5e8 100644
--- a/pkg/api/devops/v1alpha3/pipelinerun_types.go
+++ b/pkg/api/devops/v1alpha3/pipelinerun_types.go
@@ -163,6 +163,15 @@ func (prSpec *PipelineRunSpec) IsMultiBranchPipeline() bool {
 	return prSpec.PipelineSpec != nil && prSpec.PipelineSpec.Type == MultiBranchPipelineType
 }
 
+// GetRefName get refName
+func (pr *PipelineRun) GetRefName() string {
+	var refName string
+	if pr.Spec.IsMultiBranchPipeline() && pr.Spec.SCM != nil {
+		refName = pr.Spec.SCM.RefName
+	}
+	return refName
+}
+
 // GetPipelineRunID gets ID of PipelineRun.
 func (pr *PipelineRun) GetPipelineRunID() (pipelineRunID string, exist bool) {
 	pipelineRunID, exist = pr.Annotations[JenkinsPipelineRunIDAnnoKey]
diff --git a/pkg/client/devops/fake/fakedevops.go b/pkg/client/devops/fake/fakedevops.go
index 2b30114..2346824 100644
--- a/pkg/client/devops/fake/fakedevops.go
+++ b/pkg/client/devops/fake/fakedevops.go
@@ -194,7 +194,7 @@ func (d *Devops) RunPipeline(projectName, pipelineName string, httpParameters *d
 func (d *Devops) GetArtifacts(projectName, pipelineName, runId string, httpParameters *devops.HttpParameters) ([]devops.Artifacts, error) {
 	return nil, nil
 }
-func (d *Devops) DownloadArtifact(projectName, pipelineName, runId, filename string) (io.ReadCloser, error) {
+func (d *Devops) DownloadArtifact(projectName, pipelineName, runId, filename string, isMultiBranch bool, branchName string) (io.ReadCloser, error) {
 	return nil, nil
 }
 
diff --git a/pkg/client/devops/jclient/pipeline.go b/pkg/client/devops/jclient/pipeline.go
index 2859fc0..818414f 100644
--- a/pkg/client/devops/jclient/pipeline.go
+++ b/pkg/client/devops/jclient/pipeline.go
@@ -23,6 +23,7 @@ import (
 	"strconv"
 
 	"github.com/jenkins-zh/jenkins-client/pkg/artifact"
+
 	"kubesphere.io/devops/pkg/client/devops"
 )
 
@@ -67,12 +68,15 @@ func (j *JenkinsClient) GetArtifacts(projectName, pipelineName, runID string, ht
 }
 
 // DownloadArtifact download an artifact
-func (j *JenkinsClient) DownloadArtifact(projectName, pipelineName, runID, filename string) (io.ReadCloser, error) {
+func (j *JenkinsClient) DownloadArtifact(projectName, pipelineName, runID, filename string, isMultiBranch bool, branchName string) (io.ReadCloser, error) {
 	jobRunID, err := strconv.Atoi(runID)
 	if err != nil {
 		return nil, fmt.Errorf("runId error, not a number: %v", err)
 	}
 	c := artifact.Client{JenkinsCore: j.Core}
+	if isMultiBranch {
+		return c.GetArtifactFromMultiBranchPipeline(projectName, pipelineName, isMultiBranch, branchName, jobRunID, filename)
+	}
 	return c.GetArtifact(projectName, pipelineName, jobRunID, filename)
 }
 
diff --git a/pkg/client/devops/pipeline.go b/pkg/client/devops/pipeline.go
index 0ecdbec..8ba23ae 100644
--- a/pkg/client/devops/pipeline.go
+++ b/pkg/client/devops/pipeline.go
@@ -1136,7 +1136,7 @@ type PipelineOperator interface {
 	ReplayPipeline(projectName, pipelineName, runId string, httpParameters *HttpParameters) (*ReplayPipeline, error)
 	RunPipeline(projectName, pipelineName string, httpParameters *HttpParameters) (*RunPipeline, error)
 	GetArtifacts(projectName, pipelineName, runId string, httpParameters *HttpParameters) ([]Artifacts, error)
-	DownloadArtifact(projectName, pipelineName, runId, filename string) (io.ReadCloser, error)
+	DownloadArtifact(projectName, pipelineName, runId, filename string, isMultiBranch bool, branchName string) (io.ReadCloser, error)
 	GetRunLog(projectName, pipelineName, runId string, httpParameters *HttpParameters) ([]byte, error)
 	GetStepLog(projectName, pipelineName, runId, nodeId, stepId string, httpParameters *HttpParameters) ([]byte, http.Header, error)
 	GetNodeSteps(projectName, pipelineName, runId, nodeId string, httpParameters *HttpParameters) ([]NodeSteps, error)
diff --git a/pkg/kapis/devops/v1alpha3/pipelinerun/handler.go b/pkg/kapis/devops/v1alpha3/pipelinerun/handler.go
index 90b6052..212e044 100644
--- a/pkg/kapis/devops/v1alpha3/pipelinerun/handler.go
+++ b/pkg/kapis/devops/v1alpha3/pipelinerun/handler.go
@@ -30,6 +30,8 @@ import (
 	"kubesphere.io/devops/pkg/kapis"
 
 	"github.com/emicklei/go-restful"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	"kubesphere.io/devops/pkg/api/devops/v1alpha3"
 	"kubesphere.io/devops/pkg/apiserver/query"
 	apiserverrequest "kubesphere.io/devops/pkg/apiserver/request"
@@ -37,7 +39,6 @@ import (
 	devopsClient "kubesphere.io/devops/pkg/client/devops"
 	"kubesphere.io/devops/pkg/models/pipelinerun"
 	resourcesV1alpha3 "kubesphere.io/devops/pkg/models/resources/v1alpha3"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // apiHandlerOption holds some useful tools for API handler.
@@ -233,9 +234,11 @@ func (h *apiHandler) downloadArtifact(request *restful.Request, response *restfu
 		return
 	}
 	pipelineName := pr.Labels[v1alpha3.PipelineNameLabelKey]
+	isMultiBranch := pr.Spec.IsMultiBranchPipeline()
+	branchName := pr.GetRefName()
 
 	// request the Jenkins API to download artifact
-	body, err := h.devopsClient.DownloadArtifact(namespaceName, pipelineName, buildID, filename)
+	body, err := h.devopsClient.DownloadArtifact(namespaceName, pipelineName, buildID, filename, isMultiBranch, branchName)
 	if err != nil {
 		kapis.HandleError(request, response, err)
 		return


Test patch:
diff --git a/pkg/api/devops/v1alpha3/pipelinerun_types_test.go b/pkg/api/devops/v1alpha3/pipelinerun_types_test.go
index f1b5dca..9faf818 100644
--- a/pkg/api/devops/v1alpha3/pipelinerun_types_test.go
+++ b/pkg/api/devops/v1alpha3/pipelinerun_types_test.go
@@ -240,3 +240,78 @@ func TestBuildPipelineRunIdentifier(t *testing.T) {
 		})
 	}
 }
+
+func TestPipelineRun_GetRefName(t *testing.T) {
+	type fields struct {
+		TypeMeta   v1.TypeMeta
+		ObjectMeta v1.ObjectMeta
+		Spec       PipelineRunSpec
+		Status     PipelineRunStatus
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   string
+	}{
+		{
+			name: "test with nil scm",
+			fields: fields{
+				TypeMeta:   v1.TypeMeta{},
+				ObjectMeta: v1.ObjectMeta{},
+				Spec: PipelineRunSpec{
+					PipelineSpec: &PipelineSpec{
+						Type: MultiBranchPipelineType,
+					},
+					SCM: nil,
+				},
+				Status: PipelineRunStatus{},
+			},
+			want: "",
+		},
+		{
+			name: "test with noScmPipelineType",
+			fields: fields{
+				TypeMeta:   v1.TypeMeta{},
+				ObjectMeta: v1.ObjectMeta{},
+				Spec: PipelineRunSpec{
+					PipelineSpec: &PipelineSpec{
+						Type: NoScmPipelineType,
+					},
+				},
+				Status: PipelineRunStatus{},
+			},
+			want: "",
+		},
+		{
+			name: "test with multiBranchPipelineType",
+			fields: fields{
+				TypeMeta:   v1.TypeMeta{},
+				ObjectMeta: v1.ObjectMeta{},
+				Spec: PipelineRunSpec{
+					PipelineSpec: &PipelineSpec{
+						Type: MultiBranchPipelineType,
+					},
+					SCM: &SCM{
+						RefType: "",
+						RefName: "main",
+					},
+				},
+				Status: PipelineRunStatus{},
+			},
+			want: "main",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			pr := &PipelineRun{
+				TypeMeta:   tt.fields.TypeMeta,
+				ObjectMeta: tt.fields.ObjectMeta,
+				Spec:       tt.fields.Spec,
+				Status:     tt.fields.Status,
+			}
+			if got := pr.GetRefName(); got != tt.want {
+				t.Errorf("GetRefName() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/pkg/client/devops/fake/fakedevops_test.go b/pkg/client/devops/fake/fakedevops_test.go
index 5982640..2fe4854 100644
--- a/pkg/client/devops/fake/fakedevops_test.go
+++ b/pkg/client/devops/fake/fakedevops_test.go
@@ -21,6 +21,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
+
 	devopsv1alpha3 "kubesphere.io/devops/pkg/api/devops/v1alpha3"
 )
 
@@ -190,7 +191,7 @@ func TestNotImplement(t *testing.T) {
 	assertNils(t, o1, o2)
 	o1, o2 = client.GetArtifacts("", "", "", nil)
 	assertNils(t, o1, o2)
-	o1, o2 = client.DownloadArtifact("", "", "", "")
+	o1, o2 = client.DownloadArtifact("", "", "", "", false, "")
 	assertNils(t, o1, o2)
 	o1, o2 = client.GetRunLog("", "", "", nil)
 	assertNils(t, o1, o2)