Skip to content

Commit

Permalink
feat(STONEINTG-613): remove status report from PLR controller
Browse files Browse the repository at this point in the history 
* remove status report from integrationPLR controller
* create comment in statusreport controller for github webhook
* add test result to report of statusreport
* update comment when one comment exists for the snapshot and scenario
* update controller diagram

Signed-off-by: Hongwei Liu <hongliu@redhat.com>
  • Loading branch information
@hongweiliu17
hongweiliu17 committed Oct 18, 2023
1 parent 7dd8af7 commit 90684d6
Show file tree
Hide file tree
Showing 15 changed files with 313 additions and 588 deletions.
18 changes: 0 additions & 18 deletions controllers/integrationpipeline/integrationpipeline_adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,6 @@ func NewAdapter(pipelineRun *tektonv1beta1.PipelineRun, component *applicationap
}
}

// 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) {
Expand Down
134 changes: 2 additions & 132 deletions controllers/integrationpipeline/integrationpipeline_adapter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package integrationpipeline

import (
"bytes"
"context"
"errors"
"reflect"
"time"

Expand All @@ -39,44 +37,15 @@ 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
logger helpers.IntegrationLogger

successfulTaskRun *tektonv1beta1.TaskRun
failedTaskRun *tektonv1beta1.TaskRun
Expand Down Expand Up @@ -593,98 +562,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{
Expand Down Expand Up @@ -802,11 +679,4 @@ 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
}
})
2 changes: 0 additions & 2 deletions controllers/integrationpipeline/integrationpipeline_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu

return controller.ReconcileHandler([]controller.Operation{
adapter.EnsureStatusReportedInSnapshot,
adapter.EnsureStatusReported,
adapter.EnsureEphemeralEnvironmentsCleanedUp,
})
}

// AdapterInterface is an interface defining all the operations that should be defined in an Integration adapter.
type AdapterInterface interface {
EnsureStatusReported() (controller.OperationResult, error)
EnsureStatusReportedInSnapshot() (controller.OperationResult, error)
EnsureEphemeralEnvironmentsCleanedUp() (controller.OperationResult, error)
}
Expand Down
17 changes: 3 additions & 14 deletions controllers/statusreport/statusreport_adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/redhat-appstudio/integration-service/gitops"
"github.com/redhat-appstudio/integration-service/metrics"
"github.com/redhat-appstudio/operator-toolkit/metadata"
"os"

applicationapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
"github.com/redhat-appstudio/integration-service/helpers"
Expand All @@ -35,8 +34,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
Expand All @@ -62,10 +59,10 @@ 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
// EnsureSnapshotTestStatusReportedToGitHub 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() || !gitops.IsSnapshotCreatedByPACPullRequestEvent(a.snapshot) {
func (a *Adapter) EnsureSnapshotTestStatusReportedToGitHub() (controller.OperationResult, error) {
if !gitops.IsSnapshotCreatedByPACPullRequestEvent(a.snapshot) {
return controller.ContinueProcessing()
}

Expand All @@ -85,14 +82,6 @@ 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
}

// EnsureSnapshotFinishedAllTests is an operation that will ensure that a pipeline Snapshot
// to the PipelineRun being processed finished and passed all tests for all defined required IntegrationTestScenarios.
// If the Snapshot doesn't have the freshest state of components, a composite Snapshot will be created instead
Expand Down
13 changes: 1 addition & 12 deletions controllers/statusreport/statusreport_adapter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/redhat-appstudio/integration-service/api/v1beta1"
"github.com/tonglil/buflogr"
"k8s.io/apimachinery/pkg/api/meta"
"os"
"reflect"
"time"

Expand All @@ -34,7 +33,6 @@ import (
"github.com/redhat-appstudio/integration-service/loader"
intgteststat "github.com/redhat-appstudio/integration-service/pkg/integrationteststatus"
"github.com/redhat-appstudio/integration-service/status"
tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Expand All @@ -54,11 +52,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
Expand Down Expand Up @@ -260,10 +253,6 @@ 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())
})

AfterEach(func() {
Expand Down Expand Up @@ -293,7 +282,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
Resource: hasPRSnapshot,
},
})
result, err := adapter.EnsureSnapshotTestStatusReported()
result, err := adapter.EnsureSnapshotTestStatusReportedToGitHub()
fmt.Fprintf(GinkgoWriter, "-------err: %v\n", err)
fmt.Fprintf(GinkgoWriter, "-------result: %v\n", result)
Expect(!result.CancelRequest && err == nil).To(BeTrue())
Expand Down
4 changes: 2 additions & 2 deletions controllers/statusreport/statusreport_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu

adapter := NewAdapter(snapshot, application, logger, loader, r.Client, ctx)
return controller.ReconcileHandler([]controller.Operation{
adapter.EnsureSnapshotTestStatusReported,
adapter.EnsureSnapshotTestStatusReportedToGitHub,
adapter.EnsureSnapshotFinishedAllTests,
})
}

// AdapterInterface is an interface defining all the operations that should be defined in an Integration adapter.
type AdapterInterface interface {
EnsureSnapshotTestStatusReported() (controller.OperationResult, error)
EnsureSnapshotTestStatusReportedToGitHub() (controller.OperationResult, error)
EnsureSnapshotFinishedAllTests() (controller.OperationResult, error)
}

Expand Down
13 changes: 10 additions & 3 deletions docs/statusreport-controller.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ flowchart TD
predicate((PREDICATE: <br>Snapshot has annotation <br>test.appstudio.openshift.io/status <br>changed))
%%%%%%%%%%%%%%%%%%%%%%% Drawing EnsureSnapshotTestStatusReported() function
%%%%%%%%%%%%%%%%%%%%%%% Drawing EnsureSnapshotTestStatusReportedToGitHub() function
%% Node definitions
ensure(Process further if: Snapshot has label <br>pac.test.appstudio.openshift.io/git-provider:github <br>defined)
Expand All @@ -30,11 +30,14 @@ flowchart TD
create_commitStatusAdapter(Create commitStatusAdapter according to <br>commit owner, repo, SHA <br>and integration test status)
does_commitStatus_exist{Does commitStatus exist <br>on github already?}
create_new_commitStatus_on_gh(Create new commitStatus on github)
does_comment_exist(Does a comment exist for snapshot and scenario?)
update_existing_comment(Update the existing comment for <br>snapshot and scenario</br>)
create_new_comment(Create a new comment for <br>snapshot and scenario</br>)
continue_processing(Controller continues processing)
%% Node connections
predicate ----> |"EnsureSnapshotTestStatusReported()"|ensure
predicate ----> |"EnsureSnapshotTestStatusReportedToGitHub()"|ensure
ensure --> get_annotation_value
get_annotation_value --> collect_commit_info
collect_commit_info --> is_installation_defined
Expand All @@ -56,7 +59,11 @@ flowchart TD
create_commitStatusAdapter --> does_commitStatus_exist
does_commitStatus_exist --Yes--> continue_processing
does_commitStatus_exist --No--> create_new_commitStatus_on_gh
create_new_commitStatus_on_gh --> continue_processing
create_new_commitStatus_on_gh --> does_comment_exist
does_comment_exist --Yes--> update_existing_comment
does_comment_exist --No--> create_new_comment
update_existing_comment --> continue_processing
create_new_comment --> continue_processing
%%%%%%%%%%%%%%%%%%%%%%% Drawing EnsureSnapshotTestStatusReported() function
Expand Down
Loading

0 comments on commit 90684d6

Please sign in to comment.