diff --git a/cmd/git/main.go b/cmd/git/main.go index 0533e64905..48ab57a04a 100644 --- a/cmd/git/main.go +++ b/cmd/git/main.go @@ -44,9 +44,9 @@ func (e ExitError) Error() string { type settings struct { help bool - depth uint url string revision string + depth uint target string resultFileCommitSha string resultFileCommitAuthor string @@ -104,7 +104,6 @@ func main() { // Execute performs flag parsing, input validation and the Git clone func Execute(ctx context.Context) error { flagValues = settings{depth: 1} - pflag.Parse() if flagValues.help { diff --git a/deploy/crds/shipwright.io_buildruns.yaml b/deploy/crds/shipwright.io_buildruns.yaml index f9c69325fa..c703505227 100644 --- a/deploy/crds/shipwright.io_buildruns.yaml +++ b/deploy/crds/shipwright.io_buildruns.yaml @@ -459,7 +459,7 @@ spec: type: object type: array failedAt: - description: FailedAt points to the resource where the BuildRun failed + description: 'Deprecated: FailedAt points to the resource where the BuildRun failed' properties: container: type: string diff --git a/docs/buildrun.md b/docs/buildrun.md index 1e4d6c952b..143251a6e7 100644 --- a/docs/buildrun.md +++ b/docs/buildrun.md @@ -268,10 +268,29 @@ _Note_: We heavily rely on the Tekton TaskRun [Conditions](https://github.com/te ### Understanding failed BuildRuns -To make it easier for users to understand why did a BuildRun failed, users can infer from the `Status.FailedAt` field, the pod and container where the failure took place. +[DEPRECATED] To make it easier for users to understand why did a BuildRun failed, users can infer from the `Status.FailedAt` field, the pod and container where the failure took place. In addition, the `Status.Conditions` will host under the `Message` field a compacted message containing the `kubectl` command to trigger, in order to retrieve the logs. +Lastly, users can check `Status.FailureDetails` field, which includes the same information available in the `Status.FailedAt` field, +as well as a humanly-readable error message and reason. +The message and reason are only included if the build strategy provides them. + +Example of failed BuildRun: + +```yaml +# [...] +status: + # [...] + failureDetails: + location: + container: step-source-default + pod: baran-build-buildrun-gzmv5-b7wbf-pod-bbpqr + message: The source repository does not exist, or you have insufficient permission + to access it. + reason: GitRemotePrivate +``` + ### Step Results in BuildRun Status After the successful completion of a `BuildRun`, the `.status` field contains the results (`.status.taskResults`) emitted from the `TaskRun` steps generate by the `BuildRun` controller as part of processing the `BuildRun`. These results contain valuable metadata for users, like the _image digest_ or the _commit sha_ of the source code used for building. diff --git a/docs/buildstrategies.md b/docs/buildstrategies.md index a37a45ace5..3e86120ecc 100644 --- a/docs/buildstrategies.md +++ b/docs/buildstrategies.md @@ -296,10 +296,36 @@ status: # [...] output: digest: sha256:07626e3c7fdd28d5328a8d6df8d29cd3da760c7f5e2070b534f9b880ed093a53 - size: "1989004" + size: 1989004 # [...] ``` +Additionally, you can store error details for debugging purposes when a BuildRun fails using your strategy. + +| Result file | Description | +| ---------------------------------- | ----------------------------------------------- | +| `$(results.shp-error-reason.path)` | File to store the error reason. | +| `$(results.shp-error-message.path)` | File to store the error message. | + +Reason is intended to be a one-word, CamelCase classification of the error source, with the first letter capitalized. +Error details are only propagated if the build container terminates with a non-zero exit code. +This information will be available in the `.status.failureDetails` field of the BuildRun. + +```yaml +apiVersion: shipwright.io/v1alpha1 +kind: BuildRun +# [...] +status: + # [...] + failureDetails: + location: + container: step-source-default + pod: baran-build-buildrun-gzmv5-b7wbf-pod-bbpqr + message: The source repository does not exist, or you have insufficient permission + to access it. + reason: GitRemotePrivate +``` + ## Steps Resource Definition All strategies steps can include a definition of resources(_limits and requests_) for CPU, memory and disk. For strategies with more than one step, each step(_container_) could require more resources than others. Strategy admins are free to define the values that they consider the best fit for each step. Also, identical strategies with the same steps that are only different in their name and step resources can be installed on the cluster to allow users to create a build with smaller and larger resource requirements. diff --git a/pkg/apis/build/v1alpha1/buildrun_types.go b/pkg/apis/build/v1alpha1/buildrun_types.go index 49363c9070..d48aa67ecc 100644 --- a/pkg/apis/build/v1alpha1/buildrun_types.go +++ b/pkg/apis/build/v1alpha1/buildrun_types.go @@ -149,7 +149,7 @@ type BuildRunStatus struct { // +optional BuildSpec *BuildSpec `json:"buildSpec,omitempty"` - // FailedAt points to the resource where the BuildRun failed + // Deprecated: FailedAt points to the resource where the BuildRun failed // +optional FailedAt *FailedAt `json:"failedAt,omitempty"` diff --git a/pkg/reconciler/buildrun/resources/conditions.go b/pkg/reconciler/buildrun/resources/conditions.go index 61032c727e..fa090aa655 100644 --- a/pkg/reconciler/buildrun/resources/conditions.go +++ b/pkg/reconciler/buildrun/resources/conditions.go @@ -11,11 +11,12 @@ import ( "knative.dev/pkg/apis" "sigs.k8s.io/controller-runtime/pkg/client" - buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" - "github.com/shipwright-io/build/pkg/ctxlog" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" + "github.com/shipwright-io/build/pkg/ctxlog" ) // Common condition strings for reason, kind, etc. @@ -81,9 +82,11 @@ func UpdateBuildRunUsingTaskRunCondition(ctx context.Context, client client.Clie return err } + //lint:ignore SA1019 we want to give users some time to adopt to failureDetails buildRun.Status.FailedAt = &buildv1alpha1.FailedAt{Pod: pod.Name} if failedContainer != nil { + //lint:ignore SA1019 we want to give users some time to adopt to failureDetails buildRun.Status.FailedAt.Container = failedContainer.Name message = fmt.Sprintf("buildrun step %s failed in pod %s, for detailed information: kubectl --namespace %s logs %s --container=%s", failedContainer.Name, diff --git a/pkg/reconciler/buildrun/resources/failureDetails_test.go b/pkg/reconciler/buildrun/resources/failureDetails_test.go deleted file mode 100644 index d66dafd4c5..0000000000 --- a/pkg/reconciler/buildrun/resources/failureDetails_test.go +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright The Shipwright Contributors -// -// SPDX-License-Identifier: Apache-2.0 - -package resources - -import ( - "context" - "encoding/json" - "fmt" - "github.com/shipwright-io/build/pkg/controller/fakes" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" - v1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - v1 "k8s.io/api/core/v1" - "knative.dev/pkg/apis" -) - -var _ = Describe("Surfacing errors", func() { - Context("resources.UpdateBuildRunUsingTaskFailures", func() { - ctx := context.Background() - client := &fakes.FakeClient{} - - It("surfaces errors to BuildRun in failed TaskRun", func() { - redTaskRun := v1beta1.TaskRun{} - redTaskRun.Status.Conditions = append(redTaskRun.Status.Conditions, - apis.Condition{Type: apis.ConditionSucceeded, Reason: v1beta1.TaskRunReasonFailed.String()}) - failedStep := v1beta1.StepState{} - - errorReasonValue := "val1" - errorMessageValue := "val2" - errorReasonKey := fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorReason) - errorMessageKey := fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorMessage) - - errorReason := v1beta1.PipelineResourceResult{Key: errorReasonKey, Value: errorReasonValue} - errorMessage := v1beta1.PipelineResourceResult{Key: errorMessageKey, Value: errorMessageValue} - unrelated := v1beta1.PipelineResourceResult{Key: "unrelated", Value: "unrelated"} - - message, _ := json.Marshal([]v1beta1.PipelineResourceResult{errorReason, errorMessage, unrelated}) - - failedStep.Terminated = &v1.ContainerStateTerminated{Message: string(message)} - - redTaskRun.Status.Steps = append(redTaskRun.Status.Steps, failedStep) - redBuild := v1alpha1.BuildRun{} - - UpdateBuildRunUsingTaskFailures(ctx, client, &redBuild, &redTaskRun) - - Expect(redBuild.Status.FailureDetails.Message).To(Equal(errorMessageValue)) - Expect(redBuild.Status.FailureDetails.Reason).To(Equal(errorReasonValue)) - }) - - It("failed TaskRun without error reason and message", func() { - redTaskRun := v1beta1.TaskRun{} - redTaskRun.Status.Conditions = append(redTaskRun.Status.Conditions, - apis.Condition{Type: apis.ConditionSucceeded, Reason: v1beta1.TaskRunReasonFailed.String()}) - failedStep := v1beta1.StepState{} - - unrelated := v1beta1.PipelineResourceResult{Key: "unrelated", Value: "unrelated"} - - message, _ := json.Marshal([]v1beta1.PipelineResourceResult{unrelated}) - - failedStep.Terminated = &v1.ContainerStateTerminated{Message: string(message)} - - redTaskRun.Status.Steps = append(redTaskRun.Status.Steps, failedStep) - redBuild := v1alpha1.BuildRun{} - - UpdateBuildRunUsingTaskFailures(ctx, client, &redBuild, &redTaskRun) - - Expect(redBuild.Status.FailureDetails.Reason).To(BeEmpty()) - Expect(redBuild.Status.FailureDetails.Message).To(BeEmpty()) - }) - - It("no errors present in BuildRun for successful TaskRun", func() { - greenTaskRun := v1beta1.TaskRun{} - greenTaskRun.Status.Conditions = append(greenTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionSucceeded}) - greenBuildRun := v1alpha1.BuildRun{} - - UpdateBuildRunUsingTaskFailures(ctx, client, &greenBuildRun, &greenTaskRun) - - Expect(greenBuildRun.Status.FailureDetails).To(BeNil()) - }) - - It("TaskRun has not condition succeeded so nothing to do", func() { - unfinishedTaskRun := v1beta1.TaskRun{} - unfinishedTaskRun.Status.Conditions = append(unfinishedTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionReady}) - unfinishedBuildRun := v1alpha1.BuildRun{} - - UpdateBuildRunUsingTaskFailures(ctx, client, &unfinishedBuildRun, &unfinishedTaskRun) - Expect(unfinishedBuildRun.Status.FailureDetails).To(BeNil()) - }) - - It("TaskRun has a unknown reason", func() { - unknownTaskRun := v1beta1.TaskRun{} - unknownTaskRun.Status.Conditions = append(unknownTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionSucceeded, Reason: "random"}) - unknownBuildRun := v1alpha1.BuildRun{} - - UpdateBuildRunUsingTaskFailures(ctx, client, &unknownBuildRun, &unknownTaskRun) - Expect(unknownBuildRun.Status.FailureDetails).To(BeNil()) - }) - }) -}) diff --git a/pkg/reconciler/buildrun/resources/failureDetails.go b/pkg/reconciler/buildrun/resources/failure_details.go similarity index 91% rename from pkg/reconciler/buildrun/resources/failureDetails.go rename to pkg/reconciler/buildrun/resources/failure_details.go index 05dcf31ec5..e8fcb5e6ba 100644 --- a/pkg/reconciler/buildrun/resources/failureDetails.go +++ b/pkg/reconciler/buildrun/resources/failure_details.go @@ -8,15 +8,18 @@ import ( "context" "encoding/json" "fmt" - buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "knative.dev/pkg/apis" "sigs.k8s.io/controller-runtime/pkg/client" + + buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" ) const ( + resultErrorMessage = "error-message" + resultErrorReason = "error-reason" prefixedResultErrorReason = prefixParamsResultsVolumes + "-" + resultErrorReason prefixedResultErrorMessage = prefixParamsResultsVolumes + "-" + resultErrorMessage ) @@ -61,13 +64,14 @@ func extractFailedPodAndContainer(ctx context.Context, client client.Client, tas } failures := make(map[string]struct{}) + // Find the names of all containers with failure status for _, containerStatus := range pod.Status.ContainerStatuses { if containerStatus.State.Terminated != nil && containerStatus.State.Terminated.ExitCode != 0 { failures[containerStatus.Name] = struct{}{} } } - // Find the first container that failed + // Find the first container that has a failure status var failedContainer *v1.Container for i, container := range pod.Spec.Containers { if _, has := failures[container.Name]; has { @@ -80,8 +84,8 @@ func extractFailedPodAndContainer(ctx context.Context, client client.Client, tas } func extractFailureDetails(ctx context.Context, client client.Client, taskRun *v1beta1.TaskRun) (failure *buildv1alpha1.FailureDetails) { - failure = &buildv1alpha1.FailureDetails{ } - failure.Location = &buildv1alpha1.FailedAt{ Pod: taskRun.Status.PodName } + failure = &buildv1alpha1.FailureDetails{} + failure.Location = &buildv1alpha1.FailedAt{Pod: taskRun.Status.PodName} if reason, message, hasReasonAndMessage := extractFailureReasonAndMessage(taskRun); hasReasonAndMessage { failure.Reason = reason diff --git a/pkg/reconciler/buildrun/resources/failure_details_test.go b/pkg/reconciler/buildrun/resources/failure_details_test.go new file mode 100644 index 0000000000..cc7cf1625a --- /dev/null +++ b/pkg/reconciler/buildrun/resources/failure_details_test.go @@ -0,0 +1,130 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package resources + +import ( + "context" + "encoding/json" + "fmt" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + pipelinev1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + corev1 "k8s.io/api/core/v1" + "knative.dev/pkg/apis" + + + buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" + buildfakes "github.com/shipwright-io/build/pkg/controller/fakes" +) + +var _ = Describe("Surfacing errors", func() { + Context("resources.UpdateBuildRunUsingTaskFailures", func() { + ctx := context.Background() + client := &buildfakes.FakeClient{} + + It("surfaces errors to BuildRun in failed TaskRun", func() { + redTaskRun := pipelinev1beta1.TaskRun{} + redTaskRun.Status.Conditions = append(redTaskRun.Status.Conditions, + apis.Condition{Type: apis.ConditionSucceeded, Reason: pipelinev1beta1.TaskRunReasonFailed.String()}) + failedStep := pipelinev1beta1.StepState{} + + errorReasonValue := "PullBaseImageFailed" + errorMessageValue := "Failed to pull the base image." + errorReasonKey := fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorReason) + errorMessageKey := fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorMessage) + + errorReason := pipelinev1beta1.PipelineResourceResult{Key: errorReasonKey, Value: errorReasonValue} + errorMessage := pipelinev1beta1.PipelineResourceResult{Key: errorMessageKey, Value: errorMessageValue} + unrelated := pipelinev1beta1.PipelineResourceResult{Key: "unrelated-resource-key", Value: "Unrelated resource value"} + + message, _ := json.Marshal([]pipelinev1beta1.PipelineResourceResult{errorReason, errorMessage, unrelated}) + + failedStep.Terminated = &corev1.ContainerStateTerminated{Message: string(message)} + + redTaskRun.Status.Steps = append(redTaskRun.Status.Steps, failedStep) + redBuild := buildv1alpha1.BuildRun{} + + UpdateBuildRunUsingTaskFailures(ctx, client, &redBuild, &redTaskRun) + + Expect(redBuild.Status.FailureDetails.Message).To(Equal(errorMessageValue)) + Expect(redBuild.Status.FailureDetails.Reason).To(Equal(errorReasonValue)) + }) + + It("does not surface unrelated Tekton resources if the TaskRun fails", func() { + redTaskRun := pipelinev1beta1.TaskRun{} + redTaskRun.Status.Conditions = append(redTaskRun.Status.Conditions, + apis.Condition{Type: apis.ConditionSucceeded, Reason: pipelinev1beta1.TaskRunReasonFailed.String()}) + failedStep := pipelinev1beta1.StepState{} + + unrelated := pipelinev1beta1.PipelineResourceResult{Key: "unrelated", Value: "unrelated"} + + message, _ := json.Marshal([]pipelinev1beta1.PipelineResourceResult{unrelated}) + + failedStep.Terminated = &corev1.ContainerStateTerminated{Message: string(message)} + + redTaskRun.Status.Steps = append(redTaskRun.Status.Steps, failedStep) + redBuild := buildv1alpha1.BuildRun{} + + UpdateBuildRunUsingTaskFailures(ctx, client, &redBuild, &redTaskRun) + + Expect(redBuild.Status.FailureDetails.Reason).To(BeEmpty()) + Expect(redBuild.Status.FailureDetails.Message).To(BeEmpty()) + }) + + It("does not surface error results if the container terminated without failure", func() { + greenTaskRun := pipelinev1beta1.TaskRun{} + greenTaskRun.Status.Conditions = append(greenTaskRun.Status.Conditions, + apis.Condition{Type: apis.ConditionSucceeded, Reason: pipelinev1beta1. TaskRunReasonSuccessful.String()}) + failedStep := pipelinev1beta1.StepState{} + + errorReasonValue := "PullBaseImageFailed" + errorMessageValue := "Failed to pull the base image." + errorReasonKey := fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorReason) + errorMessageKey := fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorMessage) + + errorReason := pipelinev1beta1.PipelineResourceResult{Key: errorReasonKey, Value: errorReasonValue} + errorMessage := pipelinev1beta1.PipelineResourceResult{Key: errorMessageKey, Value: errorMessageValue} + message, _ := json.Marshal([]pipelinev1beta1.PipelineResourceResult{ errorReason, errorMessage }) + + failedStep.Terminated = &corev1.ContainerStateTerminated{Message: string(message)} + + greenTaskRun.Status.Steps = append(greenTaskRun.Status.Steps, failedStep) + greenBuildRun := buildv1alpha1.BuildRun{} + + UpdateBuildRunUsingTaskFailures(ctx, client, &greenBuildRun, &greenTaskRun) + + Expect(greenBuildRun.Status.FailureDetails).To(BeNil()) + }) + + It("should not surface errors for a successful TaskRun", func() { + greenTaskRun := pipelinev1beta1.TaskRun{} + greenTaskRun.Status.Conditions = append(greenTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionSucceeded}) + greenBuildRun := buildv1alpha1.BuildRun{} + + UpdateBuildRunUsingTaskFailures(ctx, client, &greenBuildRun, &greenTaskRun) + + Expect(greenBuildRun.Status.FailureDetails).To(BeNil()) + }) + + It("should not surface errors if the TaskRun does not have a Succeeded condition", func() { + unfinishedTaskRun := pipelinev1beta1.TaskRun{} + unfinishedTaskRun.Status.Conditions = append(unfinishedTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionReady}) + unfinishedBuildRun := buildv1alpha1.BuildRun{} + + UpdateBuildRunUsingTaskFailures(ctx, client, &unfinishedBuildRun, &unfinishedTaskRun) + Expect(unfinishedBuildRun.Status.FailureDetails).To(BeNil()) + }) + + It("should not surface errors if the TaskRun is in progress", func() { + unknownTaskRun := pipelinev1beta1.TaskRun{} + unknownTaskRun.Status.Conditions = append(unknownTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionSucceeded, Reason: "random"}) + unknownBuildRun := buildv1alpha1.BuildRun{} + + UpdateBuildRunUsingTaskFailures(ctx, client, &unknownBuildRun, &unknownTaskRun) + Expect(unknownBuildRun.Status.FailureDetails).To(BeNil()) + }) + }) +}) diff --git a/pkg/reconciler/buildrun/resources/taskrun.go b/pkg/reconciler/buildrun/resources/taskrun.go index 1d2afbc0fd..a537b9b7ba 100644 --- a/pkg/reconciler/buildrun/resources/taskrun.go +++ b/pkg/reconciler/buildrun/resources/taskrun.go @@ -28,9 +28,6 @@ const ( paramSourceRoot = "source-root" paramSourceContext = "source-context" - resultErrorMessage = "error-message" - resultErrorReason = "error-reason" - workspaceSource = "source" inputParamBuilder = "BUILDER_IMAGE"