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 plan marked as completed when template was broken #519

Merged
merged 18 commits into from
Jul 12, 2019

Conversation

alenkacz
Copy link
Contributor

@alenkacz alenkacz commented Jul 8, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:
Fixes behavior when error in template caused plans to be marked as complete instead of error.

Which issue(s) this PR fixes:
Fixes #499

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Fixed a bug where plans were marked as completed instead of error in case of invalid template

@alenkacz alenkacz requested a review from gerred as a code owner July 8, 2019 16:07
restartPolicy: OnFailure
containers:
- name: bb
image: busybox:latest
Copy link
Member

@jbarrick-mesosphere jbarrick-mesosphere Jul 8, 2019

Choose a reason for hiding this comment

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

These pods are (currently) being created against envtest so no actual containers are able to be created. If there's not a way to write this test without requiring containers to start, I'll setup a KIND environment to run our tests like we do for the operator tests (either, a separate suite or just run everything in KIND).

Copy link
Member

Choose a reason for hiding this comment

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

Oh, this shouldn't render at all, I see :)

labels:
controller-tools.k8s.io: "1.0"
name: invalid-v1
namespace: default
Copy link
Member

@jbarrick-mesosphere jbarrick-mesosphere Jul 8, 2019

Choose a reason for hiding this comment

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

Optional: drop the namespace here so it installs cleanly into the test namespace.

@alenkacz alenkacz changed the title WIP: PlanExecution bug Fix plan marked as completed when template was broken Jul 11, 2019
planExecution.Status.State = kudov1alpha1.PhaseStateError
planExecution.Status.Phases[i].State = kudov1alpha1.PhaseStateError
// returning error = nil so that we don't retry since this is non-recoverable
return reconcile.Result{}, nil
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to update the API object here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alenkacz alenkacz merged commit a1e7a05 into master Jul 12, 2019
@alenkacz alenkacz deleted the av/fail-plan-on-missing-param branch October 30, 2019 09:07
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.

planexecution marked as COMPLETE when had an error parsing the template
4 participants