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

fix: treat Failed as error during test init #291

Closed
wants to merge 1 commit into from

Conversation

JorTurFer
Copy link
Contributor

Fixes #290

@JorTurFer JorTurFer mentioned this pull request Sep 20, 2023
@MrsDaehin
Copy link

is there a way to distinguish Test Errors ( for instance the ones from thresholds ) from Errors due to not being able to launch K6?

I think it would give more insights for troubleshooting.

@JorTurFer
Copy link
Contributor Author

is there a way to distinguish Test Errors ( for instance the ones from thresholds ) from Errors due to not being able to launch K6?

I think it would give more insights for troubleshooting.

I think that we can add another state like init-error or initialization-error or similar to reflect this case. WDYT?

Copy link
Collaborator

@yorugac yorugac left a comment

Choose a reason for hiding this comment

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

It's been awhile so the PR will need to be rebased. But I've left comments to simplify the update :)

Thanks again!

Comment on lines +53 to +56
case "Failed":
// the init job has failed
returnErr = errors.New("init job has failed")
log.Error(returnErr, "error:")
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".

Comment on lines +56 to +59
k6.Status.Stage = "error"
if _, err := r.UpdateStatus(ctx, k6, log); err != nil {
return ctrl.Result{}, err
}
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:

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

@irumaru
Copy link
Contributor

irumaru commented Apr 15, 2024

Hello.
How far along are you now?
Is there anything I can do to help?

@irumaru
Copy link
Contributor

irumaru commented May 10, 2024

Since there is no movement, we have created a new one here.
#401

@yorugac
Copy link
Collaborator

yorugac commented May 23, 2024

Closing this due to PR #401 being merged.

@yorugac yorugac closed this May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

K6 is stuck on stage: initialization if the init job fails
4 participants