Merge pull request #902 from hongweiliu17/STONEINTG-1048

fix(STONEINTG-1048): stop reconciliation for unrecoverable error during group snapshot creation

git/github/github.go

@@ -529,7 +529,8 @@ func (c *Client) CreateCommitStatus(ctx context.Context, owner string, repo stri
 func (c *Client) GetPullRequest(ctx context.Context, owner string, repo string, prID int) (*ghapi.PullRequest, error) {
 	pr, _, err := c.GetPullRequestsService().Get(ctx, owner, repo, prID)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get pull request for owner/repo/pull %s/%s/%d: %w", owner, repo, prID, err)
+		c.logger.Error(err, "failed to get pull request for owner/repo/pull %s/%s/%d", owner, repo, prID)
+		return nil, err
 	}
 
 	return pr, err

helpers/errorhandlers.go

@@ -27,6 +27,7 @@ const (
 	ReasonInvalidImageDigestError       = "InvalidImageDigest"
 	ReasonMissingValidComponentError    = "MissingValidComponentError"
 	ReasonUnknownError                  = "UnknownError"
+	ReasonUnrecoverableMetadataError    = "UnrecoverableMetadataError"
 )
 
 type IntegrationError struct {
@@ -99,3 +100,14 @@ func HandleLoaderError(logger IntegrationLogger, err error, resource, from strin
 	logger.Error(err, fmt.Sprintf("Failed to get %s from the %s", resource, from))
 	return ctrl.Result{}, err
 }
+
+func NewUnrecoverableMetadataError(msg string) error {
+	return &IntegrationError{
+		Reason:  ReasonUnrecoverableMetadataError,
+		Message: fmt.Sprintf("Meeting metadata data error: %s", msg),
+	}
+}
+
+func IsUnrecoverableMetadataError(err error) bool {
+	return getReason(err) == ReasonUnrecoverableMetadataError
+}

helpers/errorhandlers_test.go

@@ -106,5 +106,10 @@ var _ = Describe("Helpers for error handlers", Ordered, func() {
 			Expect(helpers.IsMissingInfoInPipelineRunError(err)).To(BeFalse())
 		})
 
+		It("Can define UnrecoverableMetadataError", func() {
+			err := helpers.NewUnrecoverableMetadataError("undefined annotation")
+			Expect(helpers.IsUnrecoverableMetadataError(err)).To(BeTrue())
+			Expect(err.Error()).To(Equal("Meeting metadata data error: undefined annotation"))
+		})
 	})
 })

internal/controller/snapshot/snapshot_adapter.go

@@ -603,17 +603,16 @@ func (a *Adapter) EnsureGroupSnapshotExist() (controller.OperationResult, error)
 
 	groupSnapshot, componentSnapshotInfos, err := a.prepareGroupSnapshot(a.application, prGroupHash)
 	if err != nil {
-		if strings.Contains(err.Error(), "failed to get pull request for") {
-			// Stop processing in case the repo was removed/moved, and we reach 404 when trying to find the status of pull request
-			return controller.StopProcessing()
-		}
-		a.logger.Error(err, "Failed to prepare group snapshot")
-		// stop reconciliation directly when meeting error before STONEINTG-1048 is resolved
-		err = gitops.AnnotateSnapshot(a.context, a.snapshot, gitops.PRGroupCreationAnnotation, fmt.Sprintf("failed to prepare group snapshot for pr group %s, skipping group snapshot creation", prGroup), a.client)
-		if err != nil {
-			return controller.RequeueWithError(err)
+		a.logger.Error(err, "failed to prepare group snapshot")
+		if h.IsUnrecoverableMetadataError(err) || clienterrors.IsNotFound(err) {
+			err = gitops.AnnotateSnapshot(a.context, a.snapshot, gitops.PRGroupCreationAnnotation, fmt.Sprintf("failed to prepare group snapshot for pr group %s due to error %s, skipping group snapshot creation", prGroup, err.Error()), a.client)
+			if err != nil {
+				return controller.RequeueWithError(err)
+			}
+			return controller.ContinueProcessing()
 		}
-		return controller.StopProcessing()
+		return controller.RequeueWithError(err)
+
 	}
 
 	if groupSnapshot == nil {

internal/controller/snapshot/snapshot_adapter_test.go

@@ -1636,7 +1636,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 			Expect(metadata.HasAnnotation(hasComSnapshot1, gitops.PRGroupCreationAnnotation)).To(BeTrue())
 		})
 
-		It("Calling en when there is running build PLR belonging to the same pr group sha", func() {
+		It("Calling EnsureGroupSnapshotExist when there is running build PLR belonging to the same pr group sha", func() {
 			buildPipelineRun1.Status = tektonv1.PipelineRunStatus{
 				PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{
 					Results: []tektonv1.PipelineRunResult{},
@@ -1679,7 +1679,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 			Expect(metadata.HasAnnotation(hasComSnapshot1, gitops.PRGroupCreationAnnotation)).To(BeTrue())
 		})
 
-		It("Calling en when there is failed build PLR belonging to the same pr group sha", func() {
+		It("Calling EnsureGroupSnapshotExist when there is failed build PLR belonging to the same pr group sha", func() {
 			buildPipelineRun1.Status = tektonv1.PipelineRunStatus{
 				PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{
 					Results: []tektonv1.PipelineRunResult{},
@@ -1764,6 +1764,36 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 			Expect(err).ToNot(HaveOccurred())
 		})
 
+		It("Stop processing when there is no annotationID in snapshot", func() {
+			var buf bytes.Buffer
+			log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}
+			adapter = NewAdapter(ctx, hasComSnapshot1, hasApp, log, loader.NewMockLoader(), k8sClient)
+			adapter.context = toolkit.GetMockedContext(ctx, []toolkit.MockData{
+				{
+					ContextKey: loader.ApplicationContextKey,
+					Resource:   hasApp,
+				},
+				{
+					ContextKey: loader.SnapshotContextKey,
+					Resource:   hasComSnapshot1,
+				},
+				{
+					ContextKey: loader.GetBuildPLRContextKey,
+					Resource:   []tektonv1.PipelineRun{},
+				},
+				{
+					ContextKey: loader.ApplicationComponentsContextKey,
+					Resource:   []applicationapiv1alpha1.Component{*hasCom1, *hasCom3},
+				},
+			})
+
+			result, err := adapter.EnsureGroupSnapshotExist()
+			Expect(result.CancelRequest).To(BeFalse())
+			Expect(result.RequeueRequest).To(BeFalse())
+			Expect(buf.String()).Should(ContainSubstring("failed to get app credentials from Snapshot"))
+			Expect(err).ToNot(HaveOccurred())
+		})
+
 		It("Ensure group snasphot can be created", func() {
 			var buf bytes.Buffer
 			log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}

status/reporter.go

@@ -28,8 +28,10 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/konflux-ci/integration-service/gitops"
+	"github.com/konflux-ci/integration-service/helpers"
 	intgteststat "github.com/konflux-ci/integration-service/pkg/integrationteststatus"
 )
 
@@ -69,18 +71,22 @@ type ReporterInterface interface {
 
 // GetPACGitProviderToken lookup for configured repo and fetch token from namespace
 func GetPACGitProviderToken(ctx context.Context, k8sClient client.Client, snapshot *applicationapiv1alpha1.Snapshot) (string, error) {
-	var err error
+	log := log.FromContext(ctx)
+	var err, unRecoverableError error
 
 	// List all the Repository CRs in the namespace
 	repos := pacv1alpha1.RepositoryList{}
 	if err = k8sClient.List(ctx, &repos, &client.ListOptions{Namespace: snapshot.Namespace}); err != nil {
+		log.Error(err, fmt.Sprintf("failed to get repo from namespace %s", snapshot.Namespace))
 		return "", err
 	}
 
 	// Get the full repo URL
 	url, found := snapshot.GetAnnotations()[gitops.PipelineAsCodeRepoURLAnnotation]
 	if !found {
-		return "", fmt.Errorf("object annotation not found %q", gitops.PipelineAsCodeRepoURLAnnotation)
+		unRecoverableError = helpers.NewUnrecoverableMetadataError(fmt.Sprintf("object annotation not found %q", gitops.PipelineAsCodeRepoURLAnnotation))
+		log.Error(unRecoverableError, fmt.Sprintf("object annotation not found %q", gitops.PipelineAsCodeRepoURLAnnotation))
+		return "", unRecoverableError
 	}
 
 	// Find a Repository CR with a matching URL and get its secret details
@@ -93,20 +99,25 @@ func GetPACGitProviderToken(ctx context.Context, k8sClient client.Client, snapsh
 	}
 
 	if repoSecret == nil {
-		return "", fmt.Errorf("failed to find a Repository matching URL: %q", url)
+		unRecoverableError = helpers.NewUnrecoverableMetadataError(fmt.Sprintf("failed to find a Repository matching URL: %q", url))
+		log.Error(unRecoverableError, fmt.Sprintf("failed to find a Repository matching URL: %q", url))
+		return "", unRecoverableError
 	}
 
 	// Get the pipelines as code secret from the PipelineRun's namespace
 	pacSecret := v1.Secret{}
 	err = k8sClient.Get(ctx, types.NamespacedName{Namespace: snapshot.Namespace, Name: repoSecret.Name}, &pacSecret)
 	if err != nil {
+		log.Error(err, fmt.Sprintf("failed to get secret %s/%s", snapshot.Namespace, repoSecret.Name))
 		return "", err
 	}
 
 	// Get the personal access token from the secret
 	token, found := pacSecret.Data[repoSecret.Key]
 	if !found {
-		return "", fmt.Errorf("failed to find %s secret key", repoSecret.Key)
+		unRecoverableError = helpers.NewUnrecoverableMetadataError(fmt.Sprintf("failed to find %s secret key", repoSecret.Key))
+		log.Error(unRecoverableError, fmt.Sprintf("failed to find %s secret key", repoSecret.Key))
+		return "", unRecoverableError
 	}
 
 	return string(token), nil