Skip to content

Commit

Permalink
Merge pull request #284 from MartinBasti/fix-remove-logger-from-getter
Browse files Browse the repository at this point in the history 
fix(STONEINTG-523): refactor get pipeline outcome method
  • Loading branch information
@MartinBasti
MartinBasti authored Sep 4, 2023
2 parents 12da31d + e7a1f96 commit 2c3cd59
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 64 deletions.
38 changes: 34 additions & 4 deletions controllers/integrationpipeline/integrationpipeline_adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,45 @@ func (a *Adapter) determineIfAllIntegrationPipelinesPassed(integrationPipelineRu
allIntegrationPipelineRunsPassed := true
for _, integrationPipelineRun := range *integrationPipelineRuns {
integrationPipelineRun := integrationPipelineRun // G601
pipelineRunOutcome, err := h.CalculateIntegrationPipelineRunOutcome(a.client, a.context, a.logger.Logger, &integrationPipelineRun)
if !h.HasPipelineRunFinished(&integrationPipelineRun) {
a.logger.Info(
fmt.Sprintf("Integration Pipeline Run %s has not finished yet", integrationPipelineRun.Name),
"pipelineRun.Name", integrationPipelineRun.Name,
"pipelineRun.Namespace", integrationPipelineRun.Namespace,
)
allIntegrationPipelineRunsPassed = false
continue
}
pipelineRunOutcome, err := h.GetIntegrationPipelineRunOutcome(a.client, a.context, &integrationPipelineRun)
if err != nil {
a.logger.Error(err, "Failed to get Integration PipelineRun outcome")
a.logger.Error(err, fmt.Sprintf("Failed to get Integration Pipeline Run %s outcome", integrationPipelineRun.Name),
"pipelineRun.Name", integrationPipelineRun.Name,
"pipelineRun.Namespace", integrationPipelineRun.Namespace,
)
return false, err
}
if !pipelineRunOutcome {
a.logger.Info("Integration PipelineRun did not pass all tests")
if !pipelineRunOutcome.HasPipelineRunPassedTesting() {
if !pipelineRunOutcome.HasPipelineRunSucceeded() {
a.logger.Info(
fmt.Sprintf("integration Pipeline Run %s failed without test results of TaskRuns: %s", integrationPipelineRun.Name, h.GetPipelineRunFailedReason(&integrationPipelineRun)),
"pipelineRun.Name", integrationPipelineRun.Name,
"pipelineRun.Namespace", integrationPipelineRun.Namespace,
)

} else {
a.logger.Info(
fmt.Sprintf("Integration Pipeline Run %s did not pass all tests", integrationPipelineRun.Name),
"pipelineRun.Name", integrationPipelineRun.Name,
"pipelineRun.Namespace", integrationPipelineRun.Namespace,
)
}
allIntegrationPipelineRunsPassed = false
}
if a.pipelineRun.Name == integrationPipelineRun.Name {
// log only results of current pipeline run, other pipelines will be logged in their reconciliations
// prevent mess in logs
pipelineRunOutcome.LogResults(a.logger.Logger)
}
}
return allIntegrationPipelineRunsPassed, nil
}
Expand Down
96 changes: 67 additions & 29 deletions helpers/integration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,14 @@ var testResultSchema = `{

// TaskRun is an integration specific wrapper around the status of a Tekton TaskRun.
type TaskRun struct {
logger logr.Logger
pipelineTaskName string
trStatus *tektonv1beta1.TaskRunStatus
testResult *AppStudioTestResult
}

// NewTaskRunFromTektonTaskRun creates and returns am integration TaskRun from the TaskRunStatus.
func NewTaskRunFromTektonTaskRun(logger logr.Logger, pipelineTaskName string, status *tektonv1beta1.TaskRunStatus) *TaskRun {
return &TaskRun{logger: logger, pipelineTaskName: pipelineTaskName, trStatus: status}
func NewTaskRunFromTektonTaskRun(pipelineTaskName string, status *tektonv1beta1.TaskRunStatus) *TaskRun {
return &TaskRun{pipelineTaskName: pipelineTaskName, trStatus: status}
}

// GetPipelineTaskName returns the name of the PipelineTask.
Expand Down Expand Up @@ -154,7 +153,6 @@ func (t *TaskRun) GetTestResult() (*AppStudioTestResult, error) {
if err = sch.Validate(v); err != nil {
return nil, fmt.Errorf("error validating schema of results from taskRun %s: %w", taskRunResult.Name, err)
}
t.logger.Info("Found a AppStudio test result", "Result", result)
t.testResult = &result
return &result, nil
}
Expand All @@ -180,63 +178,103 @@ func (s SortTaskRunsByStartTime) Less(i int, j int) bool {
return s[i].GetStartTime().Before(s[j].GetStartTime())
}

// CalculateIntegrationPipelineRunOutcome checks the Tekton results for a given PipelineRun and calculates the overall outcome.
// IntegrationPipelineRunOutcome is struct for pipeline outcome metadata
type IntegrationPipelineRunOutcome struct {
pipelineRunSucceeded bool
pipelineRun *tektonv1beta1.PipelineRun
// map: task name to results
results map[string]*AppStudioTestResult
}

// HasPipelineRunSucceeded returns true when pipeline in outcome succeeded
func (ipro *IntegrationPipelineRunOutcome) HasPipelineRunSucceeded() bool {
return ipro.pipelineRunSucceeded
}

// HasPipelineRunPassedTesting returns general outcome
// If any of the tasks with the TEST_OUTPUT result don't have the `result` field set to SUCCESS or SKIPPED, it returns false.
func CalculateIntegrationPipelineRunOutcome(adapterClient client.Client, ctx context.Context, logger logr.Logger, pipelineRun *tektonv1beta1.PipelineRun) (bool, error) {
var results []*AppStudioTestResult
var err error
// Check if the pipelineRun finished from the condition of status
if !HasPipelineRunFinished(pipelineRun) {
logger.Info(fmt.Sprintf("PipelineRun %s in namespace %s has not finished", pipelineRun.Name, pipelineRun.Namespace))
return false, nil
func (ipro *IntegrationPipelineRunOutcome) HasPipelineRunPassedTesting() bool {
if !ipro.HasPipelineRunSucceeded() {
return false
}
for _, result := range ipro.results {
if result.Result != AppStudioTestOutputSuccess && result.Result != AppStudioTestOutputSkipped {
return false
}
}
return true
}

// LogResults writes tasks names with results into given logger, each task on separate line
func (ipro *IntegrationPipelineRunOutcome) LogResults(logger logr.Logger) {
for k, v := range ipro.results {
logger.Info(fmt.Sprintf("Found task results for pipeline run %s", ipro.pipelineRun.Name),
"pipelineRun.Name", ipro.pipelineRun.Name,
"pipelineRun.Namespace", ipro.pipelineRun.Namespace,
"task.Name", k, "task.Result", v)
}
}

// GetIntegrationPipelineRunOutcome returns the IntegrationPipelineRunOutcome which can be used for further inspection of
// the results and general outcome
// This function must be called on the finished pipeline
func GetIntegrationPipelineRunOutcome(adapterClient client.Client, ctx context.Context, pipelineRun *tektonv1beta1.PipelineRun) (*IntegrationPipelineRunOutcome, error) {

// Check if the pipelineRun failed from the conditions of status
if !HasPipelineRunSucceeded(pipelineRun) {
logger.Error(fmt.Errorf("PipelineRun %s in namespace %s failed for %s", pipelineRun.Name, pipelineRun.Namespace, GetPipelineRunFailedReason(pipelineRun)), "PipelineRun failed without test results of TaskRuns")
return false, nil
return &IntegrationPipelineRunOutcome{
pipelineRunSucceeded: false,
pipelineRun: pipelineRun,
results: map[string]*AppStudioTestResult{},
}, nil
}
// Check if the pipelineRun.Status contains the childReferences to TaskRuns
if !reflect.ValueOf(pipelineRun.Status.ChildReferences).IsZero() {
// If the pipelineRun.Status contains the childReferences, parse them in the new way by querying for TaskRuns
results, err = GetAppStudioTestResultsFromPipelineRunWithChildReferences(adapterClient, ctx, logger, pipelineRun)
results, err := GetAppStudioTestResultsFromPipelineRunWithChildReferences(adapterClient, ctx, pipelineRun)
if err != nil {
return false, fmt.Errorf("error while getting test results from pipelineRun %s: %w", pipelineRun.Name, err)
}
}

for _, result := range results {
if result.Result != AppStudioTestOutputSuccess && result.Result != AppStudioTestOutputSkipped {
return false, nil
return nil, fmt.Errorf("error while getting test results from pipelineRun %s: %w", pipelineRun.Name, err)
}
return &IntegrationPipelineRunOutcome{
pipelineRunSucceeded: true,
pipelineRun: pipelineRun,
results: results,
}, nil
}

return true, nil
// PLR passed but no results were found
return &IntegrationPipelineRunOutcome{
pipelineRunSucceeded: true,
pipelineRun: pipelineRun,
results: map[string]*AppStudioTestResult{},
}, nil
}

// GetAppStudioTestResultsFromPipelineRunWithChildReferences finds all TaskRuns from childReferences of the PipelineRun
// that also contain a TEST_OUTPUT result and returns the parsed data
func GetAppStudioTestResultsFromPipelineRunWithChildReferences(adapterClient client.Client, ctx context.Context, logger logr.Logger, pipelineRun *tektonv1beta1.PipelineRun) ([]*AppStudioTestResult, error) {
taskRuns, err := GetAllChildTaskRunsForPipelineRun(adapterClient, ctx, logger, pipelineRun)
// returns map taskName: result
func GetAppStudioTestResultsFromPipelineRunWithChildReferences(adapterClient client.Client, ctx context.Context, pipelineRun *tektonv1beta1.PipelineRun) (map[string]*AppStudioTestResult, error) {
taskRuns, err := GetAllChildTaskRunsForPipelineRun(adapterClient, ctx, pipelineRun)
if err != nil {
return nil, err
}

results := []*AppStudioTestResult{}
results := map[string]*AppStudioTestResult{}
for _, tr := range taskRuns {
r, err := tr.GetTestResult()
if err != nil {
return nil, err
}
if r != nil {
results = append(results, r)
results[tr.GetPipelineTaskName()] = r
}
}
return results, nil
}

// GetAllChildTaskRunsForPipelineRun finds all Child TaskRuns for a given PipelineRun and
// returns integration TaskRun wrappers for them sorted by start time.
func GetAllChildTaskRunsForPipelineRun(adapterClient client.Client, ctx context.Context, logger logr.Logger, pipelineRun *tektonv1beta1.PipelineRun) ([]*TaskRun, error) {
func GetAllChildTaskRunsForPipelineRun(adapterClient client.Client, ctx context.Context, pipelineRun *tektonv1beta1.PipelineRun) ([]*TaskRun, error) {
taskRuns := []*TaskRun{}
// If there are no childReferences, skip trying to get tasks
if reflect.ValueOf(pipelineRun.Status.ChildReferences).IsZero() {
Expand All @@ -252,7 +290,7 @@ func GetAllChildTaskRunsForPipelineRun(adapterClient client.Client, ctx context.
return nil, fmt.Errorf("error while getting the child taskRun %s from pipelineRun: %w", childReference.Name, err)
}

integrationTaskRun := NewTaskRunFromTektonTaskRun(logger, childReference.PipelineTaskName, &pipelineTaskRun.Status)
integrationTaskRun := NewTaskRunFromTektonTaskRun(childReference.PipelineTaskName, &pipelineTaskRun.Status)
taskRuns = append(taskRuns, integrationTaskRun)
}
sort.Sort(SortTaskRunsByStartTime(taskRuns))
Expand Down
67 changes: 47 additions & 20 deletions helpers/integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"time"

"github.com/go-logr/logr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/redhat-appstudio/integration-service/api/v1beta1"
Expand All @@ -18,7 +17,6 @@ import (
"k8s.io/apimachinery/pkg/types"
"knative.dev/pkg/apis"
v1 "knative.dev/pkg/apis/duck/v1"
logf "sigs.k8s.io/controller-runtime/pkg/log"
)

var _ = Describe("Pipeline Adapter", Ordered, func() {
Expand All @@ -42,13 +40,11 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
hasComp *applicationapiv1alpha1.Component
hasApp *applicationapiv1alpha1.Application
hasSnapshot *applicationapiv1alpha1.Snapshot
logger logr.Logger
integrationTestScenario *v1beta1.IntegrationTestScenario
sample_image string
)

BeforeAll(func() {
logger = logf.Log.WithName("helpers_test")
now = time.Now()

hasApp = &applicationapiv1alpha1.Application{
Expand Down Expand Up @@ -506,13 +502,13 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
})

It("can create an accurate Integration TaskRun from the given TaskRun status", func() {
integrationTaskRun := helpers.NewTaskRunFromTektonTaskRun(logger, "task-success", &successfulTaskRun.Status)
integrationTaskRun := helpers.NewTaskRunFromTektonTaskRun("task-success", &successfulTaskRun.Status)
Expect(integrationTaskRun).NotTo(BeNil())
Expect(integrationTaskRun.GetPipelineTaskName()).To(Equal("task-success"))
Expect(integrationTaskRun.GetStartTime().Equal(now))
Expect(integrationTaskRun.GetDuration().Minutes()).To(Equal(5.0))

integrationTaskRun = helpers.NewTaskRunFromTektonTaskRun(logger, "task-instant", &emptyTaskRun.Status)
integrationTaskRun = helpers.NewTaskRunFromTektonTaskRun("task-instant", &emptyTaskRun.Status)
Expect(integrationTaskRun).NotTo(BeNil())
Expect(integrationTaskRun.GetPipelineTaskName()).To(Equal("task-instant"))
Expect(integrationTaskRun.GetDuration().Minutes()).To(Equal(0.0))
Expand Down Expand Up @@ -545,9 +541,9 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
}
Expect(k8sClient.Status().Update(ctx, integrationPipelineRun)).Should(Succeed())

pipelineRunOutcome, err := helpers.CalculateIntegrationPipelineRunOutcome(k8sClient, ctx, logger, integrationPipelineRun)
pipelineRunOutcome, err := helpers.GetIntegrationPipelineRunOutcome(k8sClient, ctx, integrationPipelineRun)
Expect(err).To(BeNil())
Expect(pipelineRunOutcome).To(BeTrue())
Expect(pipelineRunOutcome.HasPipelineRunPassedTesting()).To(BeTrue())

gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue())
Expand All @@ -564,9 +560,9 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
})
Expect(k8sClient.Status().Update(ctx, integrationPipelineRun)).Should(Succeed())

pipelineRunOutcome, err := helpers.CalculateIntegrationPipelineRunOutcome(k8sClient, ctx, logger, integrationPipelineRun)
pipelineRunOutcome, err := helpers.GetIntegrationPipelineRunOutcome(k8sClient, ctx, integrationPipelineRun)
Expect(err).To(BeNil())
Expect(pipelineRunOutcome).To(BeFalse())
Expect(pipelineRunOutcome.HasPipelineRunPassedTesting()).To(BeFalse())

gitops.MarkSnapshotAsFailed(k8sClient, ctx, hasSnapshot, "test failed")
Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeFalse())
Expand Down Expand Up @@ -598,14 +594,45 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
}
Expect(k8sClient.Status().Update(ctx, integrationPipelineRun)).Should(Succeed())

pipelineRunOutcome, err := helpers.CalculateIntegrationPipelineRunOutcome(k8sClient, ctx, logger, integrationPipelineRun)
pipelineRunOutcome, err := helpers.GetIntegrationPipelineRunOutcome(k8sClient, ctx, integrationPipelineRun)
Expect(err).To(BeNil())
Expect(pipelineRunOutcome).To(BeFalse())
Expect(pipelineRunOutcome.HasPipelineRunPassedTesting()).To(BeFalse())

gitops.MarkSnapshotAsFailed(k8sClient, ctx, hasSnapshot, "test failed")
Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeFalse())
})

It("no error from pipelinrun when AppStudio Tests failed but pipeline passed", func() {
integrationPipelineRun.Status = tektonv1beta1.PipelineRunStatus{
PipelineRunStatusFields: tektonv1beta1.PipelineRunStatusFields{
ChildReferences: []tektonv1beta1.ChildStatusReference{
{
Name: failedTaskRun.Name,
PipelineTaskName: "pipeline1-task1",
},
{
Name: skippedTaskRun.Name,
PipelineTaskName: "pipeline1-task2",
},
},
},
Status: v1.Status{
Conditions: v1.Conditions{
apis.Condition{
Reason: "Completed",
Status: "True",
Type: apis.ConditionSucceeded,
},
},
},
}
Expect(k8sClient.Status().Update(ctx, integrationPipelineRun)).Should(Succeed())

pipelineRunOutcome, err := helpers.GetIntegrationPipelineRunOutcome(k8sClient, ctx, integrationPipelineRun)
Expect(err).To(BeNil())
Expect(pipelineRunOutcome.HasPipelineRunPassedTesting()).To(BeFalse())
})

It("ensure No Task pipelinerun passed when AppStudio Tests passed", func() {

integrationPipelineRun.Status = tektonv1beta1.PipelineRunStatus{
Expand Down Expand Up @@ -633,9 +660,9 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
}
Expect(k8sClient.Status().Update(ctx, integrationPipelineRun)).Should(Succeed())

pipelineRunOutcome, err := helpers.CalculateIntegrationPipelineRunOutcome(k8sClient, ctx, logger, integrationPipelineRun)
pipelineRunOutcome, err := helpers.GetIntegrationPipelineRunOutcome(k8sClient, ctx, integrationPipelineRun)
Expect(err).To(BeNil())
Expect(pipelineRunOutcome).To(BeFalse())
Expect(pipelineRunOutcome.HasPipelineRunPassedTesting()).To(BeFalse())

gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed")
Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue())
Expand Down Expand Up @@ -663,9 +690,9 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
}

Expect(k8sClient.Status().Update(ctx, integrationPipelineRun)).Should(Succeed())
result, err := helpers.CalculateIntegrationPipelineRunOutcome(k8sClient, ctx, logr.Discard(), integrationPipelineRun)
result, err := helpers.GetIntegrationPipelineRunOutcome(k8sClient, ctx, integrationPipelineRun)
Expect(err).ToNot(BeNil())
Expect(result).To(BeFalse())
Expect(result).To(BeNil())
})

It("can handle broken json as TEST_OUTPUT result", func() {
Expand All @@ -690,9 +717,9 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
}

Expect(k8sClient.Status().Update(ctx, integrationPipelineRun)).Should(Succeed())
result, err := helpers.CalculateIntegrationPipelineRunOutcome(k8sClient, ctx, logr.Discard(), integrationPipelineRun)
result, err := helpers.GetIntegrationPipelineRunOutcome(k8sClient, ctx, integrationPipelineRun)
Expect(err).ToNot(BeNil())
Expect(result).To(BeFalse())
Expect(result).To(BeNil())
})

It("can get all the TaskRuns for a PipelineRun with childReferences", func() {
Expand Down Expand Up @@ -720,7 +747,7 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
},
}

taskRuns, err := helpers.GetAllChildTaskRunsForPipelineRun(k8sClient, ctx, logr.Discard(), integrationPipelineRun)
taskRuns, err := helpers.GetAllChildTaskRunsForPipelineRun(k8sClient, ctx, integrationPipelineRun)
Expect(err).To(BeNil())
Expect(len(taskRuns)).To(Equal(2))

Expand Down Expand Up @@ -756,7 +783,7 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
PipelineRunStatusFields: tektonv1beta1.PipelineRunStatusFields{},
}

taskRuns, err := helpers.GetAllChildTaskRunsForPipelineRun(k8sClient, ctx, logr.Discard(), integrationPipelineRun)
taskRuns, err := helpers.GetAllChildTaskRunsForPipelineRun(k8sClient, ctx, integrationPipelineRun)
Expect(err).To(BeNil())
Expect(taskRuns).To(BeNil())
})
Expand Down
Loading

0 comments on commit 2c3cd59

Please sign in to comment.