diff --git a/cmd/git/main.go b/cmd/git/main.go index 856a215c88..e93f2a9e53 100644 --- a/cmd/git/main.go +++ b/cmd/git/main.go @@ -44,14 +44,14 @@ func (e ExitError) Error() string { type settings struct { help bool - skipValidation bool - depth uint url string revision string + depth uint target string resultFileCommitSha string resultFileCommitAuthor string secretPath string + skipValidation bool } var flagValues settings @@ -100,7 +100,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 aa11a7843d..d176d734bc 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 029a59e928..056e6ef3a9 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 8eaa487238..638a4bc17e 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 1e3424652f..e62737e009 100644 --- a/pkg/apis/build/v1alpha1/buildrun_types.go +++ b/pkg/apis/build/v1alpha1/buildrun_types.go @@ -145,7 +145,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.go b/pkg/reconciler/buildrun/resources/failureDetails.go index 05dcf31ec5..b513d3a8db 100644 --- a/pkg/reconciler/buildrun/resources/failureDetails.go +++ b/pkg/reconciler/buildrun/resources/failureDetails.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 ) @@ -80,8 +83,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/failureDetails_test.go b/pkg/reconciler/buildrun/resources/failureDetails_test.go index d66dafd4c5..3c77d5495e 100644 --- a/pkg/reconciler/buildrun/resources/failureDetails_test.go +++ b/pkg/reconciler/buildrun/resources/failureDetails_test.go @@ -8,42 +8,44 @@ 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" + 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 := &fakes.FakeClient{} + client := &buildfakes.FakeClient{} It("surfaces errors to BuildRun in failed TaskRun", func() { - redTaskRun := v1beta1.TaskRun{} + redTaskRun := pipelinev1beta1.TaskRun{} redTaskRun.Status.Conditions = append(redTaskRun.Status.Conditions, - apis.Condition{Type: apis.ConditionSucceeded, Reason: v1beta1.TaskRunReasonFailed.String()}) - failedStep := v1beta1.StepState{} + apis.Condition{Type: apis.ConditionSucceeded, Reason: pipelinev1beta1.TaskRunReasonFailed.String()}) + failedStep := pipelinev1beta1.StepState{} - errorReasonValue := "val1" - errorMessageValue := "val2" + errorReasonValue := "PullBaseImageFailed" + errorMessageValue := "Failed to pull the base image." 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"} + 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([]v1beta1.PipelineResourceResult{errorReason, errorMessage, unrelated}) + message, _ := json.Marshal([]pipelinev1beta1.PipelineResourceResult{errorReason, errorMessage, unrelated}) - failedStep.Terminated = &v1.ContainerStateTerminated{Message: string(message)} + failedStep.Terminated = &corev1.ContainerStateTerminated{Message: string(message)} redTaskRun.Status.Steps = append(redTaskRun.Status.Steps, failedStep) - redBuild := v1alpha1.BuildRun{} + redBuild := buildv1alpha1.BuildRun{} UpdateBuildRunUsingTaskFailures(ctx, client, &redBuild, &redTaskRun) @@ -51,20 +53,20 @@ var _ = Describe("Surfacing errors", func() { Expect(redBuild.Status.FailureDetails.Reason).To(Equal(errorReasonValue)) }) - It("failed TaskRun without error reason and message", func() { - redTaskRun := v1beta1.TaskRun{} + 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: v1beta1.TaskRunReasonFailed.String()}) - failedStep := v1beta1.StepState{} + apis.Condition{Type: apis.ConditionSucceeded, Reason: pipelinev1beta1.TaskRunReasonFailed.String()}) + failedStep := pipelinev1beta1.StepState{} - unrelated := v1beta1.PipelineResourceResult{Key: "unrelated", Value: "unrelated"} + unrelated := pipelinev1beta1.PipelineResourceResult{Key: "unrelated", Value: "unrelated"} - message, _ := json.Marshal([]v1beta1.PipelineResourceResult{unrelated}) + message, _ := json.Marshal([]pipelinev1beta1.PipelineResourceResult{unrelated}) - failedStep.Terminated = &v1.ContainerStateTerminated{Message: string(message)} + failedStep.Terminated = &corev1.ContainerStateTerminated{Message: string(message)} redTaskRun.Status.Steps = append(redTaskRun.Status.Steps, failedStep) - redBuild := v1alpha1.BuildRun{} + redBuild := buildv1alpha1.BuildRun{} UpdateBuildRunUsingTaskFailures(ctx, client, &redBuild, &redTaskRun) @@ -72,29 +74,29 @@ var _ = Describe("Surfacing errors", func() { Expect(redBuild.Status.FailureDetails.Message).To(BeEmpty()) }) - It("no errors present in BuildRun for successful TaskRun", func() { - greenTaskRun := v1beta1.TaskRun{} + 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 := v1alpha1.BuildRun{} + greenBuildRun := buildv1alpha1.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{} + 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 := v1alpha1.BuildRun{} + unfinishedBuildRun := buildv1alpha1.BuildRun{} UpdateBuildRunUsingTaskFailures(ctx, client, &unfinishedBuildRun, &unfinishedTaskRun) Expect(unfinishedBuildRun.Status.FailureDetails).To(BeNil()) }) - It("TaskRun has a unknown reason", func() { - unknownTaskRun := v1beta1.TaskRun{} + 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 := v1alpha1.BuildRun{} + 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 b235a223a1..e7a33192b0 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"