Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Returns error if build does not start successfully with kp create image --wait #1309

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

fcaroline2020
Copy link
Contributor

@fcaroline2020 fcaroline2020 commented Aug 25, 2023

@fcaroline2020 fcaroline2020 requested a review from a team as a code owner August 25, 2023 11:42
@tomkennedy513 tomkennedy513 merged commit 330d4ca into main Aug 29, 2023
3 checks passed
@tomkennedy513 tomkennedy513 deleted the kpcli-issue-194-create-image-wait-error branch August 29, 2023 17:58
Comment on lines +102 to +103
fmt.Fprintf(writer, "Build failed to start: %s", err)
return "", nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should toss the error back up the chain instead of trying to handle it here

Suggested change
fmt.Fprintf(writer, "Build failed to start: %s", err)
return "", nil
return "", fmt.Errorf("build failed to start: %s", err)

Comment on lines +101 to +115
if err != nil {
fmt.Fprintf(writer, "Build failed to start: %s", err)
return "", nil
} else {
doneChan := make(chan struct{})
defer func() { <-doneChan }()

go func() { // tail logs
defer close(doneChan)
err := w.logTailer.TailBuildName(ctx, writer, namespace, buildName)
if err != nil {
fmt.Fprintf(writer, "error tailing logs %s", err)
}
}()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if a {
  ...
  return
} else {
  bar()
}

is better written as

if a {
  ...
  return
}
bar()


condition := build.Status.GetCondition(corev1alpha1.ConditionSucceeded)
if condition.IsFalse() {
return errors.New(condition.Message)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return errors.New(condition.Message)
return buildFailure(condition.Message)

func buildFailure(statusMessage string) error {
errMsg := "build failed"
if statusMessage != "" {
errMsg = fmt.Sprintf("%s: %s", errMsg, statusMessage)
}
return errors.New(errMsg)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants