feat(STONEINTG-613): remove status report from PLR controller

* remove status report from integrationPLR controller * create comment in statusreport controller for github webhook * add test result to report of statusreport Signed-off-by: Hongwei Liu <hongliu@redhat.com>
konflux-ci · Oct 11, 2023 · 5e280b9 · 5e280b9
1 parent f3e4d23
commit 5e280b9
Show file tree

Hide file tree

Showing 11 changed files with 120 additions and 549 deletions.
diff --git a/controllers/integrationpipeline/integrationpipeline_adapter.go b/controllers/integrationpipeline/integrationpipeline_adapter.go
@@ -171,24 +171,6 @@ func (a *Adapter) EnsureSnapshotPassedAllTests() (controller.OperationResult, er
 	return controller.ContinueProcessing()
 }
 
-// EnsureStatusReported will ensure that integration PipelineRun status is reported to the git provider
-// which (indirectly) triggered its execution.
-func (a *Adapter) EnsureStatusReported() (controller.OperationResult, error) {
-	reporters, err := a.status.GetReporters(a.pipelineRun)
-
-	if err != nil {
-		return controller.RequeueWithError(err)
-	}
-
-	for _, reporter := range reporters {
-		if err := reporter.ReportStatus(a.client, a.context, a.pipelineRun); err != nil {
-			return controller.RequeueWithError(err)
-		}
-	}
-
-	return controller.ContinueProcessing()
-}
-
 // EnsureStatusReportedInSnapshot will ensure that status of the integration test pipelines is reported to snapshot
 // to be consumed by user
 func (a *Adapter) EnsureStatusReportedInSnapshot() (controller.OperationResult, error) {

diff --git a/controllers/integrationpipeline/integrationpipeline_adapter_test.go b/controllers/integrationpipeline/integrationpipeline_adapter_test.go
@@ -18,8 +18,6 @@ package integrationpipeline
 
 import (
 	"bytes"
-	"context"
-	"errors"
 	"fmt"
 	"reflect"
 	"time"
@@ -41,44 +39,16 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	applicationapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
-	"github.com/redhat-appstudio/integration-service/status"
 	tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tonglil/buflogr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-type MockStatusAdapter struct {
-	Reporter          *MockStatusReporter
-	GetReportersError error
-}
-
-type MockStatusReporter struct {
-	Called            bool
-	ReportStatusError error
-}
-
-func (r *MockStatusReporter) ReportStatus(client.Client, context.Context, *tektonv1beta1.PipelineRun) error {
-	r.Called = true
-	return r.ReportStatusError
-}
-
-func (r *MockStatusReporter) ReportStatusForSnapshot(client.Client, context.Context, *helpers.IntegrationLogger, *applicationapiv1alpha1.Snapshot) error {
-	r.Called = true
-	return r.ReportStatusError
-}
-
-func (a *MockStatusAdapter) GetReporters(object client.Object) ([]status.Reporter, error) {
-	return []status.Reporter{a.Reporter}, a.GetReportersError
-}
-
 var _ = Describe("Pipeline Adapter", Ordered, func() {
 	var (
-		adapter        *Adapter
-		createAdapter  func() *Adapter
-		logger         helpers.IntegrationLogger
-		statusAdapter  *MockStatusAdapter
-		statusReporter *MockStatusReporter
+		adapter       *Adapter
+		createAdapter func() *Adapter
+		logger        helpers.IntegrationLogger
 
 		successfulTaskRun                     *tektonv1beta1.TaskRun
 		failedTaskRun                         *tektonv1beta1.TaskRun
@@ -884,98 +854,6 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
 
 	})
 
-	When("EnsureStatusReported is called", func() {
-		It("ensures status is reported for integration PipelineRuns", func() {
-			adapter = createAdapter()
-			adapter.context = loader.GetMockedContext(ctx, []loader.MockData{
-				{
-					ContextKey: loader.ApplicationContextKey,
-					Resource:   hasApp,
-				},
-				{
-					ContextKey: loader.ComponentContextKey,
-					Resource:   hasComp,
-				},
-				{
-					ContextKey: loader.SnapshotContextKey,
-					Resource:   hasSnapshot,
-				},
-				{
-					ContextKey: loader.TaskRunContextKey,
-					Resource:   successfulTaskRun,
-				},
-				{
-					ContextKey: loader.EnvironmentContextKey,
-					Resource:   hasEnv,
-				},
-				{
-					ContextKey: loader.PipelineRunsContextKey,
-					Resource:   []tektonv1beta1.PipelineRun{*integrationPipelineRunComponent},
-				},
-				{
-					ContextKey: loader.RequiredIntegrationTestScenariosContextKey,
-					Resource:   []v1beta1.IntegrationTestScenario{*integrationTestScenario},
-				},
-			})
-
-			adapter.pipelineRun = &tektonv1beta1.PipelineRun{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "pipelinerun-status-sample",
-					Namespace: "default",
-					Labels: map[string]string{
-						"appstudio.openshift.io/application":              "test-application",
-						"appstudio.openshift.io/component":                "devfile-sample-go-basic",
-						"appstudio.openshift.io/snapshot":                 "test-application-s8tnj",
-						"test.appstudio.openshift.io/scenario":            "example-pass",
-						"pac.test.appstudio.openshift.io/state":           "started",
-						"pac.test.appstudio.openshift.io/sender":          "foo",
-						"pac.test.appstudio.openshift.io/check-run-id":    "9058825284",
-						"pac.test.appstudio.openshift.io/branch":          "main",
-						"pac.test.appstudio.openshift.io/url-org":         "devfile-sample",
-						"pac.test.appstudio.openshift.io/original-prname": "devfile-sample-go-basic-on-pull-request",
-						"pac.test.appstudio.openshift.io/url-repository":  "devfile-sample-go-basic",
-						"pac.test.appstudio.openshift.io/repository":      "devfile-sample-go-basic",
-						"pac.test.appstudio.openshift.io/sha":             "12a4a35ccd08194595179815e4646c3a6c08bb77",
-						"pac.test.appstudio.openshift.io/git-provider":    "github",
-						"pac.test.appstudio.openshift.io/event-type":      "pull_request",
-						"pipelines.appstudio.openshift.io/type":           "test",
-					},
-					Annotations: map[string]string{
-						"pac.test.appstudio.openshift.io/on-target-branch": "[main,master]",
-						"pac.test.appstudio.openshift.io/repo-url":         "https://github.com/devfile-samples/devfile-sample-go-basic",
-						"pac.test.appstudio.openshift.io/sha-title":        "Appstudio update devfile-sample-go-basic",
-						"pac.test.appstudio.openshift.io/git-auth-secret":  "pac-gitauth-zjib",
-						"pac.test.appstudio.openshift.io/pull-request":     "16",
-						"pac.test.appstudio.openshift.io/on-event":         "[pull_request]",
-						"pac.test.appstudio.openshift.io/installation-id":  "30353543",
-					},
-				},
-				Spec: tektonv1beta1.PipelineRunSpec{
-					PipelineRef: &tektonv1beta1.PipelineRef{
-						Name:   "component-pipeline-pass",
-						Bundle: "quay.io/kpavic/test-bundle:component-pipeline-pass",
-					},
-				},
-			}
-
-			result, err := adapter.EnsureStatusReported()
-			Expect(!result.CancelRequest && err == nil).To(BeTrue())
-
-			Expect(statusReporter.Called).To(BeTrue())
-
-			statusAdapter.GetReportersError = errors.New("GetReportersError")
-
-			result, err = adapter.EnsureStatusReported()
-			Expect(result.RequeueRequest && err != nil && err.Error() == "GetReportersError").To(BeTrue())
-
-			statusAdapter.GetReportersError = nil
-			statusReporter.ReportStatusError = errors.New("ReportStatusError")
-
-			result, err = adapter.EnsureStatusReported()
-			Expect(result.RequeueRequest && err != nil && err.Error() == "ReportStatusError").To(BeTrue())
-		})
-	})
-
 	When("EnsureEphemeralEnvironmentsCleanedUp is called", func() {
 		BeforeEach(func() {
 			deploymentTargetClass = &applicationapiv1alpha1.DeploymentTargetClass{
@@ -1095,9 +973,6 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
 
 	createAdapter = func() *Adapter {
 		adapter = NewAdapter(integrationPipelineRunComponent, hasComp, hasApp, logger, loader.NewMockLoader(), k8sClient, ctx)
-		statusReporter = &MockStatusReporter{}
-		statusAdapter = &MockStatusAdapter{Reporter: statusReporter}
-		adapter.status = statusAdapter
 		return adapter
 	}
 })
diff --git a/controllers/integrationpipeline/integrationpipeline_controller.go b/controllers/integrationpipeline/integrationpipeline_controller.go
@@ -110,15 +110,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	return controller.ReconcileHandler([]controller.Operation{
 		adapter.EnsureStatusReportedInSnapshot,
 		adapter.EnsureSnapshotPassedAllTests,
-		adapter.EnsureStatusReported,
 		adapter.EnsureEphemeralEnvironmentsCleanedUp,
 	})
 }
 
 // AdapterInterface is an interface defining all the operations that should be defined in an Integration adapter.
 type AdapterInterface interface {
 	EnsureSnapshotPassedAllTests() (controller.OperationResult, error)
-	EnsureStatusReported() (controller.OperationResult, error)
 	EnsureStatusReportedInSnapshot() (controller.OperationResult, error)
 	EnsureEphemeralEnvironmentsCleanedUp() (controller.OperationResult, error)
 }

diff --git a/controllers/statusreport/statusreport_adapter.go b/controllers/statusreport/statusreport_adapter.go
@@ -18,7 +18,6 @@ package statusreport
 
 import (
 	"context"
-	"os"
 
 	applicationapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
 	"github.com/redhat-appstudio/integration-service/helpers"
@@ -29,8 +28,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-const FeatureFlagStatusReprotingEnabled = "FEATURE_STATUS_REPORTING_ENABLED"
-
 // Adapter holds the objects needed to reconcile a snapshot's test status report.
 type Adapter struct {
 	snapshot    *applicationapiv1alpha1.Snapshot
@@ -59,10 +56,6 @@ func NewAdapter(snapshot *applicationapiv1alpha1.Snapshot, application *applicat
 // EnsureSnapshotTestStatusReported will ensure that integration test status including env provision and snapshotEnvironmentBinding error is reported to the git provider
 // which (indirectly) triggered its execution.
 func (a *Adapter) EnsureSnapshotTestStatusReported() (controller.OperationResult, error) {
-	if !isFeatureEnabled() {
-		return controller.ContinueProcessing()
-	}
-
 	reporters, err := a.status.GetReporters(a.snapshot)
 	if err != nil {
 		return controller.RequeueWithError(err)
@@ -78,11 +71,3 @@ func (a *Adapter) EnsureSnapshotTestStatusReported() (controller.OperationResult
 
 	return controller.ContinueProcessing()
 }
-
-// isFeatureEnabled returns true when the feature flag FEATURE_STATUS_REPORTING_ENABLED has been defined in env vars
-func isFeatureEnabled() bool {
-	if _, found := os.LookupEnv(FeatureFlagStatusReprotingEnabled); found {
-		return true
-	}
-	return false
-}
diff --git a/controllers/statusreport/statusreport_adapter_test.go b/controllers/statusreport/statusreport_adapter_test.go
@@ -19,7 +19,6 @@ package statusreport
 import (
 	"context"
 	"fmt"
-	"os"
 	"reflect"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -28,7 +27,6 @@ import (
 	applicationapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
 	"github.com/redhat-appstudio/integration-service/loader"
 	"github.com/redhat-appstudio/integration-service/status"
-	tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -48,11 +46,6 @@ type MockStatusReporter struct {
 	ReportStatusError error
 }
 
-func (r *MockStatusReporter) ReportStatus(client.Client, context.Context, *tektonv1beta1.PipelineRun) error {
-	r.Called = true
-	return r.ReportStatusError
-}
-
 func (r *MockStatusReporter) ReportStatusForSnapshot(client.Client, context.Context, *helpers.IntegrationLogger, *applicationapiv1alpha1.Snapshot) error {
 	r.Called = true
 	r.ReportStatusError = nil
@@ -131,19 +124,13 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 			},
 		}
 		Expect(k8sClient.Create(ctx, hasSnapshot)).Should(Succeed())
-
-		// enable feature flag for testing
-		err := os.Setenv(FeatureFlagStatusReprotingEnabled, "yes")
-		Expect(err).To(BeNil())
 	})
 
 	AfterAll(func() {
 		err := k8sClient.Delete(ctx, hasSnapshot)
 		Expect(err == nil || errors.IsNotFound(err)).To(BeTrue())
 		err = k8sClient.Delete(ctx, hasApp)
 		Expect(err == nil || errors.IsNotFound(err)).To(BeTrue())
-
-		_ = os.Unsetenv(FeatureFlagStatusReprotingEnabled)
 	})
 
 	When("adapter is created", func() {

diff --git a/status/format.go b/status/format.go
@@ -66,8 +66,8 @@ func FormatSummary(taskRuns []*helpers.TaskRun) (string, error) {
 	return buf.String(), nil
 }
 
-// FormatComment builds a markdown comment for a list of integration TaskRuns.
-func FormatComment(title string, results []*helpers.TaskRun) (string, error) {
+// FormatCommentForFinishedPipelineRun builds a markdown comment for a list of integration TaskRuns.
+func FormatCommentForFinishedPipelineRun(title string, results []*helpers.TaskRun) (string, error) {
 	summary, err := FormatSummary(results)
 	if err != nil {
 		return "", err
@@ -82,6 +82,17 @@ func FormatComment(title string, results []*helpers.TaskRun) (string, error) {
 	return buf.String(), nil
 }
 
+// FormatCommentForDetail build a markdown comment for the details of integrationTestStatusDetail
+func FormatCommentForDetail(title, detail string) (string, error) {
+	buf := bytes.Buffer{}
+	data := CommentTemplateData{Title: title, Summary: detail}
+	t := template.Must(template.New("").Parse(commentTemplate))
+	if err := t.Execute(&buf, data); err != nil {
+		return "", err
+	}
+	return buf.String(), nil
+}
+
 // FormatStatus accepts a TaskRun and returns a Markdown friendly representation of its overall status, if any.
 func FormatStatus(taskRun *helpers.TaskRun) (string, error) {
 	result, err := taskRun.GetTestResult()

diff --git a/status/format_test.go b/status/format_test.go
@@ -147,7 +147,7 @@ var _ = Describe("Formatters", func() {
 	})
 
 	It("can construct a comment", func() {
-		comment, err := status.FormatComment("example-title", taskRuns)
+		comment, err := status.FormatCommentForFinishedPipelineRun("example-title", taskRuns)
 		Expect(err).To(BeNil())
 		Expect(comment).To(ContainSubstring("### example-title"))
 		Expect(comment).To(ContainSubstring(expectedSummary))