diff --git a/cmd/git/main.go b/cmd/git/main.go index e93f2a9e53..856a215c88 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,6 +100,7 @@ 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 055cfb588b..aa11a7843d 100644 --- a/deploy/crds/shipwright.io_buildruns.yaml +++ b/deploy/crds/shipwright.io_buildruns.yaml @@ -466,9 +466,17 @@ spec: pod: type: string type: object - failure: - description: Failure contains error details that are collected and surfaced from TaskRun + failureDetails: + description: FailureDetails contains error details that are collected and surfaced from TaskRun properties: + location: + description: FailedAt describes the location where the failure happened + properties: + container: + type: string + pod: + type: string + type: object message: type: string reason: diff --git a/pkg/apis/build/v1alpha1/buildrun_types.go b/pkg/apis/build/v1alpha1/buildrun_types.go index 161e60c7d9..1e3424652f 100644 --- a/pkg/apis/build/v1alpha1/buildrun_types.go +++ b/pkg/apis/build/v1alpha1/buildrun_types.go @@ -149,9 +149,9 @@ type BuildRunStatus struct { // +optional FailedAt *FailedAt `json:"failedAt,omitempty"` - // Failure contains error details that are collected and surfaced from TaskRun + // FailureDetails contains error details that are collected and surfaced from TaskRun // +optional - Failure *Failure `json:"failure,omitempty"` + FailureDetails *FailureDetails `json:"failureDetails,omitempty"` } // FailedAt describes the location where the failure happened @@ -160,10 +160,11 @@ type FailedAt struct { Container string `json:"container,omitempty"` } -// Failure describes an error while building images -type Failure struct { - Reason string `json:"reason,omitempty"` - Message string `json:"message,omitempty"` +// FailureDetails describes an error while building images +type FailureDetails struct { + Reason string `json:"reason,omitempty"` + Message string `json:"message,omitempty"` + Location *FailedAt `json:"location,omitempty"` } // BuildRef can be used to refer to a specific instance of a Build. diff --git a/pkg/reconciler/buildrun/buildrun.go b/pkg/reconciler/buildrun/buildrun.go index e286d56ce2..c9573da067 100644 --- a/pkg/reconciler/buildrun/buildrun.go +++ b/pkg/reconciler/buildrun/buildrun.go @@ -308,7 +308,7 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu return reconcile.Result{}, err } - resources.UpdateBuildRunUsingTaskFailures(buildRun, lastTaskRun) + resources.UpdateBuildRunUsingTaskFailures(ctx, r.client, buildRun, lastTaskRun) taskRunStatus := trCondition.Status // check if we should delete the generated service account by checking the build run spec and that the task run is complete diff --git a/pkg/reconciler/buildrun/resources/conditions.go b/pkg/reconciler/buildrun/resources/conditions.go index d20cd9e1c6..61032c727e 100644 --- a/pkg/reconciler/buildrun/resources/conditions.go +++ b/pkg/reconciler/buildrun/resources/conditions.go @@ -11,13 +11,11 @@ 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" - "k8s.io/apimachinery/pkg/types" - - buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" - "github.com/shipwright-io/build/pkg/ctxlog" ) // Common condition strings for reason, kind, etc. @@ -70,8 +68,8 @@ func UpdateBuildRunUsingTaskRunCondition(ctx context.Context, client client.Clie case v1beta1.TaskRunReasonFailed: if taskRun.Status.CompletionTime != nil { - var pod corev1.Pod - if err := client.Get(ctx, types.NamespacedName{Namespace: taskRun.Namespace, Name: taskRun.Status.PodName}, &pod); err != nil { + pod, failedContainer, err := extractFailedPodAndContainer(ctx, client, taskRun) + if err != nil { // when trying to customize the Condition Message field, ensure the Message cover the case // when a Pod is deleted. // Note: this is an edge case, but not doing this prevent a BuildRun from being marked as Failed @@ -85,23 +83,6 @@ func UpdateBuildRunUsingTaskRunCondition(ctx context.Context, client client.Clie buildRun.Status.FailedAt = &buildv1alpha1.FailedAt{Pod: pod.Name} - // Since the container status list is not sorted, as a quick workaround mark all failed containers - var failures = make(map[string]struct{}) - 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 - var failedContainer *corev1.Container - for i, container := range pod.Spec.Containers { - if _, has := failures[container.Name]; has { - failedContainer = &pod.Spec.Containers[i] - break - } - } - if failedContainer != nil { 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", diff --git a/pkg/reconciler/buildrun/resources/failureDetails.go b/pkg/reconciler/buildrun/resources/failureDetails.go new file mode 100644 index 0000000000..05dcf31ec5 --- /dev/null +++ b/pkg/reconciler/buildrun/resources/failureDetails.go @@ -0,0 +1,112 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package resources + +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" +) + +const ( + prefixedResultErrorReason = prefixParamsResultsVolumes + "-" + resultErrorReason + prefixedResultErrorMessage = prefixParamsResultsVolumes + "-" + resultErrorMessage +) + +// UpdateBuildRunUsingTaskFailures is extracting failures from taskRun steps and adding them to buildRun (mutates) +func UpdateBuildRunUsingTaskFailures(ctx context.Context, client client.Client, buildRun *buildv1alpha1.BuildRun, taskRun *v1beta1.TaskRun) { + trCondition := taskRun.Status.GetCondition(apis.ConditionSucceeded) + + // only extract failures when failing condition is present + if trCondition != nil && v1beta1.TaskRunReason(trCondition.Reason) == v1beta1.TaskRunReasonFailed { + buildRun.Status.FailureDetails = extractFailureDetails(ctx, client, taskRun) + } +} + +func extractFailureReasonAndMessage(taskRun *v1beta1.TaskRun) (errorReason string, errorMessage string, hasReasonAndMessage bool) { + for _, step := range taskRun.Status.Steps { + message := step.Terminated.Message + var taskRunResults []v1beta1.PipelineResourceResult + + if err := json.Unmarshal([]byte(message), &taskRunResults); err != nil { + continue + } + + for _, result := range taskRunResults { + if result.Key == prefixedResultErrorMessage { + errorMessage = result.Value + } + + if result.Key == prefixedResultErrorReason { + errorReason = result.Value + } + } + } + + return errorReason, errorMessage, true +} + +func extractFailedPodAndContainer(ctx context.Context, client client.Client, taskRun *v1beta1.TaskRun) (*v1.Pod, *v1.Container, error) { + var pod v1.Pod + if err := client.Get(ctx, types.NamespacedName{Namespace: taskRun.Namespace, Name: taskRun.Status.PodName}, &pod); err != nil { + return nil, nil, err + } + + failures := make(map[string]struct{}) + 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 + var failedContainer *v1.Container + for i, container := range pod.Spec.Containers { + if _, has := failures[container.Name]; has { + failedContainer = &pod.Spec.Containers[i] + break + } + } + + return &pod, failedContainer, nil +} + +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 } + + if reason, message, hasReasonAndMessage := extractFailureReasonAndMessage(taskRun); hasReasonAndMessage { + failure.Reason = reason + failure.Message = message + } + + pod, container, _ := extractFailedPodAndContainer(ctx, client, taskRun) + + if pod != nil && container != nil { + failure.Location.Pod = pod.Name + failure.Location.Container = container.Name + } + + return failure +} + +func getFailureDetailsTaskSpecResults() []v1beta1.TaskResult { + return []v1beta1.TaskResult{ + { + Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorMessage), + Description: "The error description of the task run", + }, + { + Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorReason), + Description: "The error reason of the task run", + }, + } +} diff --git a/pkg/reconciler/buildrun/resources/failures_test.go b/pkg/reconciler/buildrun/resources/failureDetails_test.go similarity index 73% rename from pkg/reconciler/buildrun/resources/failures_test.go rename to pkg/reconciler/buildrun/resources/failureDetails_test.go index 9202871cf7..d66dafd4c5 100644 --- a/pkg/reconciler/buildrun/resources/failures_test.go +++ b/pkg/reconciler/buildrun/resources/failureDetails_test.go @@ -1,8 +1,14 @@ +// 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" @@ -14,6 +20,8 @@ import ( 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{} @@ -37,10 +45,10 @@ var _ = Describe("Surfacing errors", func() { redTaskRun.Status.Steps = append(redTaskRun.Status.Steps, failedStep) redBuild := v1alpha1.BuildRun{} - UpdateBuildRunUsingTaskFailures(&redBuild, &redTaskRun) + UpdateBuildRunUsingTaskFailures(ctx, client, &redBuild, &redTaskRun) - Expect(redBuild.Status.Failure.Message).To(Equal(errorMessageValue)) - Expect(redBuild.Status.Failure.Reason).To(Equal(errorReasonValue)) + 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() { @@ -58,9 +66,10 @@ var _ = Describe("Surfacing errors", func() { redTaskRun.Status.Steps = append(redTaskRun.Status.Steps, failedStep) redBuild := v1alpha1.BuildRun{} - UpdateBuildRunUsingTaskFailures(&redBuild, &redTaskRun) + UpdateBuildRunUsingTaskFailures(ctx, client, &redBuild, &redTaskRun) - Expect(redBuild.Status.Failure).To(BeNil()) + Expect(redBuild.Status.FailureDetails.Reason).To(BeEmpty()) + Expect(redBuild.Status.FailureDetails.Message).To(BeEmpty()) }) It("no errors present in BuildRun for successful TaskRun", func() { @@ -68,9 +77,9 @@ var _ = Describe("Surfacing errors", func() { greenTaskRun.Status.Conditions = append(greenTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionSucceeded}) greenBuildRun := v1alpha1.BuildRun{} - UpdateBuildRunUsingTaskFailures(&greenBuildRun, &greenTaskRun) + UpdateBuildRunUsingTaskFailures(ctx, client, &greenBuildRun, &greenTaskRun) - Expect(greenBuildRun.Status.Failure).To(BeNil()) + Expect(greenBuildRun.Status.FailureDetails).To(BeNil()) }) It("TaskRun has not condition succeeded so nothing to do", func() { @@ -78,8 +87,8 @@ var _ = Describe("Surfacing errors", func() { unfinishedTaskRun.Status.Conditions = append(unfinishedTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionReady}) unfinishedBuildRun := v1alpha1.BuildRun{} - UpdateBuildRunUsingTaskFailures(&unfinishedBuildRun, &unfinishedTaskRun) - Expect(unfinishedBuildRun.Status.Failure).To(BeNil()) + UpdateBuildRunUsingTaskFailures(ctx, client, &unfinishedBuildRun, &unfinishedTaskRun) + Expect(unfinishedBuildRun.Status.FailureDetails).To(BeNil()) }) It("TaskRun has a unknown reason", func() { @@ -87,8 +96,8 @@ var _ = Describe("Surfacing errors", func() { unknownTaskRun.Status.Conditions = append(unknownTaskRun.Status.Conditions, apis.Condition{Type: apis.ConditionSucceeded, Reason: "random"}) unknownBuildRun := v1alpha1.BuildRun{} - UpdateBuildRunUsingTaskFailures(&unknownBuildRun, &unknownTaskRun) - Expect(unknownBuildRun.Status.Failure).To(BeNil()) + UpdateBuildRunUsingTaskFailures(ctx, client, &unknownBuildRun, &unknownTaskRun) + Expect(unknownBuildRun.Status.FailureDetails).To(BeNil()) }) }) }) diff --git a/pkg/reconciler/buildrun/resources/failures.go b/pkg/reconciler/buildrun/resources/failures.go deleted file mode 100644 index 5e7aaa081b..0000000000 --- a/pkg/reconciler/buildrun/resources/failures.go +++ /dev/null @@ -1,52 +0,0 @@ -package resources - -import ( - "encoding/json" - buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - "knative.dev/pkg/apis" -) - -const ( - prefixedResultErrorReason = prefixParamsResultsVolumes + "-" + resultErrorReason - prefixedResultErrorMessage = prefixParamsResultsVolumes + "-" + resultErrorMessage -) - -// UpdateBuildRunUsingTaskFailures is extracting failures from taskRun steps and adding them to buildRun (mutates) -func UpdateBuildRunUsingTaskFailures(buildRun *buildv1alpha1.BuildRun, taskRun *v1beta1.TaskRun) { - trCondition := taskRun.Status.GetCondition(apis.ConditionSucceeded) - - // only extract failures when failing condition is present - if trCondition != nil && v1beta1.TaskRunReason(trCondition.Reason) == v1beta1.TaskRunReasonFailed { - buildRun.Status.Failure = extractErrorFromTaskRun(taskRun) - } -} - -func extractErrorFromTaskRun(taskRun *v1beta1.TaskRun) *buildv1alpha1.Failure { - shipError := buildv1alpha1.Failure{} - - for _, step := range taskRun.Status.Steps { - message := step.Terminated.Message - var taskRunResults []v1beta1.PipelineResourceResult - - if err := json.Unmarshal([]byte(message), &taskRunResults); err != nil { - continue - } - - for _, result := range taskRunResults { - if result.Key == prefixedResultErrorMessage { - shipError.Message = result.Value - } - - if result.Key == prefixedResultErrorReason { - shipError.Reason = result.Value - } - } - } - - if len(shipError.Message) == 0 || len(shipError.Reason) == 0 { - return nil - } - - return &shipError -} diff --git a/pkg/reconciler/buildrun/resources/taskrun.go b/pkg/reconciler/buildrun/resources/taskrun.go index 820f0563c6..b235a223a1 100644 --- a/pkg/reconciler/buildrun/resources/taskrun.go +++ b/pkg/reconciler/buildrun/resources/taskrun.go @@ -108,16 +108,6 @@ func GenerateTaskSpec( Type: taskv1.ParamTypeString, }, }, - Results: []v1beta1.TaskResult{ - { - Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorMessage), - Description: "The error description of the task run", - }, - { - Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, resultErrorReason), - Description: "The error reason of the task run", - }, - }, Workspaces: []v1beta1.WorkspaceDeclaration{ // workspace for the source files { @@ -126,7 +116,7 @@ func GenerateTaskSpec( }, } - generatedTaskSpec.Results = append(generatedTaskSpec.Results, getTaskSpecResults()...) + generatedTaskSpec.Results = append(getTaskSpecResults(), getFailureDetailsTaskSpecResults()...) if build.Spec.Builder != nil { InputBuilder := v1beta1.ParamSpec{