From e1579b0279d0d62c1fda154906dec0d0245379e0 Mon Sep 17 00:00:00 2001 From: Hongwei Liu Date: Tue, 10 Sep 2024 17:37:43 +0800 Subject: [PATCH] feat(STONEINTG-999): report test status for group snapshot * Update EnsureSnapshotTestStatusReportedToGitProvider to support group snapshot Signed-off-by: Hongwei Liu --- gitops/snapshot.go | 1 + .../statusreport/statusreport_adapter.go | 11 +- .../statusreport/statusreport_adapter_test.go | 8 +- status/mock_status.go | 8 +- status/status.go | 136 ++++++++++++------ status/status_test.go | 19 +-- 6 files changed, 117 insertions(+), 66 deletions(-) diff --git a/gitops/snapshot.go b/gitops/snapshot.go index 252b27cb7..7667dbb76 100644 --- a/gitops/snapshot.go +++ b/gitops/snapshot.go @@ -931,6 +931,7 @@ func IsComponentSnapshot(snapshot *applicationapiv1alpha1.Snapshot) bool { return metadata.HasLabelWithValue(snapshot, SnapshotTypeLabel, SnapshotComponentType) } +// IsGroupSnapshot returns true if snapshot label 'test.appstudio.openshift.io/type' is 'group' func IsGroupSnapshot(snapshot *applicationapiv1alpha1.Snapshot) bool { return metadata.HasLabelWithValue(snapshot, SnapshotTypeLabel, SnapshotGroupType) } diff --git a/internal/controller/statusreport/statusreport_adapter.go b/internal/controller/statusreport/statusreport_adapter.go index 65e524471..eee61b793 100644 --- a/internal/controller/statusreport/statusreport_adapter.go +++ b/internal/controller/statusreport/statusreport_adapter.go @@ -66,19 +66,14 @@ func NewAdapter(context context.Context, snapshot *applicationapiv1alpha1.Snapsh // EnsureSnapshotTestStatusReportedToGitProvider will ensure that integration test status is reported to the git provider // which (indirectly) triggered its execution. +// The status is reported to git provider if it is a component snapshot +// Or reported to giv proviers which trigger component snapshot included in group snapshot if it is a gorup snapshot func (a *Adapter) EnsureSnapshotTestStatusReportedToGitProvider() (controller.OperationResult, error) { if gitops.IsSnapshotCreatedByPACPushEvent(a.snapshot) { return controller.ContinueProcessing() } - reporter := a.status.GetReporter(a.snapshot) - if reporter == nil { - a.logger.Info("No suitable reporter found, skipping report") - return controller.ContinueProcessing() - } - a.logger.Info(fmt.Sprintf("Detected reporter: %s", reporter.GetReporterName())) - - err := a.status.ReportSnapshotStatus(a.context, reporter, a.snapshot) + err := a.status.ReportSnapshotStatus(a.context, a.snapshot) if err != nil { a.logger.Error(err, "failed to report test status to git provider for snapshot", "snapshot.Namespace", a.snapshot.Namespace, "snapshot.Name", a.snapshot.Name) diff --git a/internal/controller/statusreport/statusreport_adapter_test.go b/internal/controller/statusreport/statusreport_adapter_test.go index d24970fb0..c83b66060 100644 --- a/internal/controller/statusreport/statusreport_adapter_test.go +++ b/internal/controller/statusreport/statusreport_adapter_test.go @@ -242,14 +242,14 @@ var _ = Describe("Snapshot Adapter", Ordered, func() { It("ensures the statusReport is called", func() { ctrl := gomock.NewController(GinkgoT()) - mockReporter := status.NewMockReporterInterface(ctrl) + // mockReporter := status.NewMockReporterInterface(ctrl) mockStatus := status.NewMockStatusInterface(ctrl) - mockReporter.EXPECT().GetReporterName().Return("mocked_reporter") + // mockReporter.EXPECT().GetReporterName().Return("mocked_reporter") - mockStatus.EXPECT().GetReporter(gomock.Any()).Return(mockReporter) + // mockStatus.EXPECT().GetReporter(gomock.Any()).Return(mockReporter) // ReportSnapshotStatus must be called once - mockStatus.EXPECT().ReportSnapshotStatus(gomock.Any(), gomock.Any(), gomock.Any()).Times(1) + mockStatus.EXPECT().ReportSnapshotStatus(gomock.Any(), gomock.Any()).Times(1) mockScenarios := []v1beta2.IntegrationTestScenario{} adapter = NewAdapter(ctx, hasPRSnapshot, hasApp, logger, loader.NewMockLoader(), k8sClient) diff --git a/status/mock_status.go b/status/mock_status.go index cfafcb030..1c02dae3e 100644 --- a/status/mock_status.go +++ b/status/mock_status.go @@ -100,15 +100,15 @@ func (mr *MockStatusInterfaceMockRecorder) IsPRMRInSnapshotOpened(arg0, arg1 any } // ReportSnapshotStatus mocks base method. -func (m *MockStatusInterface) ReportSnapshotStatus(arg0 context.Context, arg1 ReporterInterface, arg2 *v1alpha1.Snapshot) error { +func (m *MockStatusInterface) ReportSnapshotStatus(arg0 context.Context, arg1 *v1alpha1.Snapshot) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ReportSnapshotStatus", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "ReportSnapshotStatus", arg0, arg1) ret0, _ := ret[0].(error) return ret0 } // ReportSnapshotStatus indicates an expected call of ReportSnapshot -func (mr *MockStatusInterfaceMockRecorder) ReportSnapshotStatus(arg0, arg1, arg2 any) *gomock.Call { +func (mr *MockStatusInterfaceMockRecorder) ReportSnapshotStatus(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReportSnapshotStatus", reflect.TypeOf((*MockStatusInterface)(nil).ReportSnapshotStatus), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReportSnapshotStatus", reflect.TypeOf((*MockStatusInterface)(nil).ReportSnapshotStatus), arg0, arg1) } diff --git a/status/status.go b/status/status.go index e2e843ff7..5b941f4e7 100644 --- a/status/status.go +++ b/status/status.go @@ -187,7 +187,7 @@ func MigrateSnapshotToReportStatus(s *applicationapiv1alpha1.Snapshot, testStatu type StatusInterface interface { GetReporter(*applicationapiv1alpha1.Snapshot) ReporterInterface - ReportSnapshotStatus(context.Context, ReporterInterface, *applicationapiv1alpha1.Snapshot) error + ReportSnapshotStatus(context.Context, *applicationapiv1alpha1.Snapshot) error // Check if PR/MR is opened IsPRMRInSnapshotOpened(context.Context, *applicationapiv1alpha1.Snapshot) (bool, error) // Check if github PR is open @@ -227,12 +227,12 @@ func (s *Status) GetReporter(snapshot *applicationapiv1alpha1.Snapshot) Reporter } // ReportSnapshotStatus reports status of all integration tests into Pull Request -func (s *Status) ReportSnapshotStatus(ctx context.Context, reporter ReporterInterface, snapshot *applicationapiv1alpha1.Snapshot) error { +func (s *Status) ReportSnapshotStatus(ctx context.Context, testedSnapshot *applicationapiv1alpha1.Snapshot) error { - statuses, err := gitops.NewSnapshotIntegrationTestStatusesFromSnapshot(snapshot) + statuses, err := gitops.NewSnapshotIntegrationTestStatusesFromSnapshot(testedSnapshot) if err != nil { s.logger.Error(err, "failed to get test status annotations from snapshot", - "snapshot.Namespace", snapshot.Namespace, "snapshot.Name", snapshot.Name) + "snapshot.Namespace", testedSnapshot.Namespace, "snapshot.Name", testedSnapshot.Name) return err } @@ -240,58 +240,83 @@ func (s *Status) ReportSnapshotStatus(ctx context.Context, reporter ReporterInte if len(integrationTestStatusDetails) == 0 { // no tests to report, skip s.logger.Info("No test result to report to GitHub, skipping", - "snapshot.Namespace", snapshot.Namespace, "snapshot.Name", snapshot.Name) + "snapshot.Namespace", testedSnapshot.Namespace, "snapshot.Name", testedSnapshot.Name) return nil } - if err := reporter.Initialize(ctx, snapshot); err != nil { - s.logger.Error(err, "Failed to initialize reporter", "reporter", reporter.GetReporterName()) - return fmt.Errorf("failed to initialize reporter: %w", err) + // get the component snapshot list that include the git provider info the report will be reported to + destinationSnapshots := make([]*applicationapiv1alpha1.Snapshot, 0) + if gitops.IsComponentSnapshot(testedSnapshot) { + destinationSnapshots = append(destinationSnapshots, testedSnapshot) + } else if gitops.IsGroupSnapshot(testedSnapshot) { + destinationSnapshots, err = getComponentSnapshotsFromGroupSnapshot(ctx, s.client, testedSnapshot) + if err != nil { + s.logger.Error(err, "failed to get component snapshot included in group snapshot snapshot", + "snapshot.NameSpace", testedSnapshot.Namespace, "snapshot.Name", testedSnapshot.Name) + } } - s.logger.Info("Reporter initialized", "reporter", reporter.GetReporterName()) - MigrateSnapshotToReportStatus(snapshot, integrationTestStatusDetails) + // Report the integration test status to pr/commit included the tested component snapshot + // or the component snapshot included in group snapshot + for _, destinationComponentSnapshot := range destinationSnapshots { + destinationComponentSnapshot := destinationComponentSnapshot + reporter := s.GetReporter(destinationComponentSnapshot) + if reporter == nil { + s.logger.Info("No suitable reporter found, skipping report") + return nil + } + s.logger.Info(fmt.Sprintf("Detected reporter: %s", reporter.GetReporterName())) - srs, err := NewSnapshotReportStatusFromSnapshot(snapshot) - if err != nil { - s.logger.Error(err, "failed to get latest snapshot write metadata annotation for snapshot", - "snapshot.NameSpace", snapshot.Namespace, "snapshot.Name", snapshot.Name) - srs, _ = NewSnapshotReportStatus("") - } + if err := reporter.Initialize(ctx, destinationComponentSnapshot); err != nil { + s.logger.Error(err, "Failed to initialize reporter", "reporter", reporter.GetReporterName()) + return fmt.Errorf("failed to initialize reporter: %w", err) + } + s.logger.Info("Reporter initialized", "reporter", reporter.GetReporterName()) - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - for _, integrationTestStatusDetail := range integrationTestStatusDetails { - if srs.IsNewer(integrationTestStatusDetail.ScenarioName, integrationTestStatusDetail.LastUpdateTime) { - s.logger.Info("Integration Test contains new status updates", "scenario.Name", integrationTestStatusDetail.ScenarioName) - } else { - //integration test contains no changes - continue - } - testReport, reportErr := s.generateTestReport(ctx, *integrationTestStatusDetail, snapshot) - if reportErr != nil { - if writeErr := WriteSnapshotReportStatus(ctx, s.client, snapshot, srs); writeErr != nil { // try to write what was already written - return fmt.Errorf("failed to generate test report AND write snapshot report status metadata: %w", errors.Join(reportErr, writeErr)) + MigrateSnapshotToReportStatus(testedSnapshot, integrationTestStatusDetails) + + srs, err := NewSnapshotReportStatusFromSnapshot(testedSnapshot) + if err != nil { + s.logger.Error(err, "failed to get latest snapshot write metadata annotation for snapshot", + "snapshot.NameSpace", testedSnapshot.Namespace, "snapshot.Name", testedSnapshot.Name) + srs, _ = NewSnapshotReportStatus("") + } + + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + for _, integrationTestStatusDetail := range integrationTestStatusDetails { + if srs.IsNewer(integrationTestStatusDetail.ScenarioName, integrationTestStatusDetail.LastUpdateTime) { + s.logger.Info("Integration Test contains new status updates", "scenario.Name", integrationTestStatusDetail.ScenarioName) + } else { + //integration test contains no changes + continue } - return fmt.Errorf("failed to generate test report: %w", reportErr) - } - if reportStatusErr := reporter.ReportStatus(ctx, *testReport); reportStatusErr != nil { - if writeErr := WriteSnapshotReportStatus(ctx, s.client, snapshot, srs); writeErr != nil { // try to write what was already written - return fmt.Errorf("failed to report status AND write snapshot report status metadata: %w", errors.Join(reportStatusErr, writeErr)) + testReport, reportErr := s.generateTestReport(ctx, *integrationTestStatusDetail, testedSnapshot) + if reportErr != nil { + if writeErr := WriteSnapshotReportStatus(ctx, s.client, testedSnapshot, srs); writeErr != nil { // try to write what was already written + return fmt.Errorf("failed to generate test report AND write snapshot report status metadata: %w", errors.Join(reportErr, writeErr)) + } + return fmt.Errorf("failed to generate test report: %w", reportErr) } - return fmt.Errorf("failed to update status: %w", reportStatusErr) + if reportStatusErr := reporter.ReportStatus(ctx, *testReport); reportStatusErr != nil { + if writeErr := WriteSnapshotReportStatus(ctx, s.client, testedSnapshot, srs); writeErr != nil { // try to write what was already written + return fmt.Errorf("failed to report status AND write snapshot report status metadata: %w", errors.Join(reportStatusErr, writeErr)) + } + return fmt.Errorf("failed to update status: %w", reportStatusErr) + } + srs.SetLastUpdateTime(integrationTestStatusDetail.ScenarioName, integrationTestStatusDetail.LastUpdateTime) } - srs.SetLastUpdateTime(integrationTestStatusDetail.ScenarioName, integrationTestStatusDetail.LastUpdateTime) - } - if err := WriteSnapshotReportStatus(ctx, s.client, snapshot, srs); err != nil { - return fmt.Errorf("failed to write snapshot report status metadata: %w", err) - } - return err - }) + if err := WriteSnapshotReportStatus(ctx, s.client, testedSnapshot, srs); err != nil { + return fmt.Errorf("failed to write snapshot report status metadata: %w", err) + } + return err + }) + } + if err != nil { return fmt.Errorf("issue occured during generating or updating report status: %w", err) } - s.logger.Info(fmt.Sprintf("Successfully updated the %s annotation", gitops.SnapshotStatusReportAnnotation), "snapshotReporterStatus.value", srs) + s.logger.Info(fmt.Sprintf("Successfully updated the %s annotation", gitops.SnapshotStatusReportAnnotation), "snapshot.Name", testedSnapshot.Name) return nil } @@ -542,3 +567,30 @@ func (s Status) IsPRInSnapshotOpened(ctx context.Context, reporter ReporterInter } return false, err } + +// getComponentSnapshotsFromGroupSnapshot return the component snapshot list which component snapshot is created from +func getComponentSnapshotsFromGroupSnapshot(ctx context.Context, c client.Client, groupSnapshot *applicationapiv1alpha1.Snapshot) ([]*applicationapiv1alpha1.Snapshot, error) { + var componentSnapshotInfos []*gitops.ComponentSnapshotInfo + var componentSnapshots []*applicationapiv1alpha1.Snapshot + var err error + if componentSnapshotInfoString, ok := groupSnapshot.Annotations[gitops.GroupSnapshotInfoAnnotation]; ok { + componentSnapshotInfos, err = gitops.UnmarshalJSON([]byte(componentSnapshotInfoString)) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal JSON string: %w", err) + } + } + + for _, componentSnapshotInfo := range componentSnapshotInfos { + componentSnapshot := &applicationapiv1alpha1.Snapshot{} + err := c.Get(ctx, types.NamespacedName{ + Namespace: groupSnapshot.Namespace, + Name: componentSnapshotInfo.Snapshot, + }, componentSnapshot) + if err != nil { + return nil, fmt.Errorf("failed to find component snapshot %s included in group snapshot %s/%s: %w", componentSnapshotInfo.Snapshot, groupSnapshot.Namespace, groupSnapshot.Name, err) + } + componentSnapshots = append(componentSnapshots, componentSnapshot) + } + return componentSnapshots, nil + +} diff --git a/status/status_test.go b/status/status_test.go index f95e52509..427b60a6a 100644 --- a/status/status_test.go +++ b/status/status_test.go @@ -65,6 +65,7 @@ var _ = Describe("Status Adapter", func() { githubSnapshot *applicationapiv1alpha1.Snapshot hasSnapshot *applicationapiv1alpha1.Snapshot mockReporter *status.MockReporterInterface + mockStatus *status.MockStatusInterface pipelineRun *tektonv1.PipelineRun successfulTaskRun *tektonv1.TaskRun @@ -309,7 +310,9 @@ var _ = Describe("Status Adapter", func() { ctrl := gomock.NewController(GinkgoT()) mockReporter = status.NewMockReporterInterface(ctrl) + mockStatus = status.NewMockStatusInterface(ctrl) mockReporter.EXPECT().GetReporterName().Return("mocked-reporter").AnyTimes() + mockReporter.EXPECT().Initialize(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() os.Setenv("CONSOLE_NAME", "Red Hat Konflux") }) @@ -331,7 +334,7 @@ var _ = Describe("Status Adapter", func() { mockReporter.EXPECT().ReportStatus(gomock.Any(), gomock.Any()).Times(0) // without test results reported shouldn't report status st := status.NewStatus(logr.Discard(), nil) - err := st.ReportSnapshotStatus(context.Background(), mockReporter, githubSnapshot) + err := st.ReportSnapshotStatus(context.Background(), githubSnapshot) Expect(err).NotTo(HaveOccurred()) }) @@ -343,7 +346,7 @@ var _ = Describe("Status Adapter", func() { hasSnapshot.Annotations["test.appstudio.openshift.io/status"] = "[{\"scenario\":\"scenario1\",\"status\":\"InProgress\",\"startTime\":\"2023-07-26T16:57:49+02:00\",\"lastUpdateTime\":\"2023-08-26T17:57:50+02:00\",\"details\":\"Test in progress\"}]" hasSnapshot.Annotations["test.appstudio.openshift.io/git-reporter-status"] = "{\"scenarios\":{\"scenario1\":{\"lastUpdateTime\":\"2023-08-26T17:57:50+02:00\"}}}" st := status.NewStatus(logr.Discard(), mockK8sClient) - err := st.ReportSnapshotStatus(context.Background(), mockReporter, hasSnapshot) + err := st.ReportSnapshotStatus(context.Background(), hasSnapshot) Expect(err).NotTo(HaveOccurred()) }) @@ -355,7 +358,7 @@ var _ = Describe("Status Adapter", func() { hasSnapshot.Annotations["test.appstudio.openshift.io/status"] = "[{\"scenario\":\"scenario1\",\"status\":\"InProgress\",\"startTime\":\"2023-07-26T16:57:49+02:00\",\"lastUpdateTime\":\"2023-08-26T17:57:50+02:00\",\"details\":\"Test in progress\"}]" hasSnapshot.Annotations["test.appstudio.openshift.io/pr-last-update"] = "2023-08-26T17:57:50+02:00" st := status.NewStatus(logr.Discard(), mockK8sClient) - err := st.ReportSnapshotStatus(context.Background(), mockReporter, hasSnapshot) + err := st.ReportSnapshotStatus(context.Background(), hasSnapshot) Expect(err).NotTo(HaveOccurred()) }) @@ -368,7 +371,7 @@ var _ = Describe("Status Adapter", func() { hasSnapshot.Annotations["test.appstudio.openshift.io/git-reporter-status"] = "{\"scenarios\":{\"scenario1\":{\"lastUpdateTime\":\"2023-08-26T17:57:49+02:00\"}}}" hasSnapshot.Annotations["test.appstudio.openshift.io/group-test-info"] = "[{\"namespace\":\"default\",\"component\":\"devfile-sample-java-springboot-basic-8969\",\"buildPipelineRun\":\"build-plr-java-qjfxz\",\"snapshot\":\"app-8969-bbn7d\"},{\"namespace\":\"default\",\"component\":\"devfile-sample-go-basic-8969\",\"buildPipelineRun\":\"build-plr-go-jmsjq\",\"snapshot\":\"app-8969-kzq2l\"}]" st := status.NewStatus(logr.Discard(), mockK8sClient) - err := st.ReportSnapshotStatus(context.Background(), mockReporter, hasSnapshot) + err := st.ReportSnapshotStatus(context.Background(), hasSnapshot) Expect(err).NotTo(HaveOccurred()) }) @@ -380,7 +383,7 @@ var _ = Describe("Status Adapter", func() { hasSnapshot.Annotations["test.appstudio.openshift.io/status"] = "[{\"scenario\":\"scenario1\",\"status\":\"InProgress\",\"startTime\":\"2023-07-26T16:57:49+02:00\",\"lastUpdateTime\":\"2023-08-26T17:57:50+02:00\",\"details\":\"Test in progress\"}]" hasSnapshot.Annotations["test.appstudio.openshift.io/pr-last-update"] = "2023-08-26T17:57:49+02:00" st := status.NewStatus(logr.Discard(), mockK8sClient) - err := st.ReportSnapshotStatus(context.Background(), mockReporter, hasSnapshot) + err := st.ReportSnapshotStatus(context.Background(), hasSnapshot) Expect(err).NotTo(HaveOccurred()) }) @@ -404,7 +407,7 @@ var _ = Describe("Status Adapter", func() { mockReporter.EXPECT().ReportStatus(gomock.Any(), gomock.Eq(expectedTestReport)).Times(1) st := status.NewStatus(logr.Discard(), mockK8sClient) - err = st.ReportSnapshotStatus(context.Background(), mockReporter, hasSnapshot) + err = st.ReportSnapshotStatus(context.Background(), hasSnapshot) Expect(err).NotTo(HaveOccurred()) }) @@ -443,7 +446,7 @@ var _ = Describe("Status Adapter", func() { mockReporter.EXPECT().ReportStatus(gomock.Any(), gomock.Eq(expectedTestReport)).Times(1) st := status.NewStatus(logr.Discard(), mockK8sClient) - err = st.ReportSnapshotStatus(context.Background(), mockReporter, hasSnapshot) + err = st.ReportSnapshotStatus(context.Background(), hasSnapshot) Expect(err).NotTo(HaveOccurred()) }) @@ -459,7 +462,7 @@ var _ = Describe("Status Adapter", func() { mockReporter.EXPECT().ReportStatus(gomock.Any(), HasSummary(expectedSummary)).Times(1) st := status.NewStatus(logr.Discard(), mockK8sClient) - err := st.ReportSnapshotStatus(context.Background(), mockReporter, hasSnapshot) + err := st.ReportSnapshotStatus(context.Background(), hasSnapshot) Expect(err).NotTo(HaveOccurred()) }, Entry("Passed", integrationteststatus.IntegrationTestStatusTestPassed, "has passed"),