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.

Sign up for GitHub

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

fix: treat Failed as error during test init #291

Closed
wants to merge 1 commit into from
Closed

fix: treat Failed as error during test init #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion controllers/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"time"
Expand Down Expand Up @@ -46,7 +47,15 @@ func inspectTestRun(ctx context.Context, log logr.Logger, k6 v1alpha1.K6, c clie
}

// there should be only 1 initializer pod
if podList.Items[0].Status.Phase != "Succeeded" {
switch podList.Items[0].Status.Phase {
case "Succeeded":
// nothing to do
case "Failed":
// the init job has failed
returnErr = errors.New("init job has failed")
log.Error(returnErr, "error:")
Comment on lines +53 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the if check for "Failed" phase over here would be sufficient. And return of error as you did. So smth like:

Suggested change
case "Failed":
// the init job has failed
returnErr = errors.New("init job has failed")
log.Error(returnErr, "error:")
if initializerPhase == "Failed" {
// the initializer job has failed
returnErr = errors.New("initializer job has failed")
log.Error(returnErr, "error:")
}

Please use "initializer" so as to distinguish from "init container".

return
default:
log.Info("Waiting for initializing pod to finish")
return
}
Expand Down
6 changes: 5 additions & 1 deletion controllers/k6_initialize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ func RunValidations(ctx context.Context, log logr.Logger, k6 *v1alpha1.K6, r *K6

inspectOutput, inspectReady, err := inspectTestRun(ctx, log, *k6, r.Client)
if err != nil {
// inspectTestRun made a log message already so just return without requeue
// if there is any error, we have to reflect it on the K6 manifest
k6.Status.Stage = "error"
if _, err := r.UpdateStatus(ctx, k6, log); err != nil {
return ctrl.Result{}, err
}
Comment on lines +56 to +59
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should now go a bit later in logic, over here:

k6-operator/controllers/k6_initialize.go

Line 65 in 568d40e

Also, the change of Status is done via k6.GetStatus().Stage = ... for the moment.

return ctrl.Result{}, nil
}
if !inspectReady {
Expand Down
Loading