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

Add support for non restored state on job submission #38

Merged
merged 6 commits into from
Jul 3, 2019
Merged

Add support for non restored state on job submission #38

merged 6 commits into from
Jul 3, 2019

Conversation

YuvalItzchakov
Copy link
Contributor

This PR adds support for providing the --allowNonRestoredState flag to a submit job request. If, for some reason, that state isn't persistent between versions of a job, the deployment will fail entirely.

The addition here adds a allowNonRestoredState boolean flag to the crd.yaml.

@YuvalItzchakov YuvalItzchakov changed the title Add support for non restored state on savepoint Add support for non restored state on job submission Jul 2, 2019
SavepointPath: application.Spec.SavepointInfo.SavepointLocation,
EntryClass: entryClass,
ProgramArgs: programArgs,
AllowNonRestoredState: application.Spec.AllowNonRestoredState,
Copy link
Contributor

Choose a reason for hiding this comment

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

@anandswaminathan
Copy link
Contributor

Also you need to run make generate

@YuvalItzchakov
Copy link
Contributor Author

YuvalItzchakov commented Jul 3, 2019

@anandswaminathan Appended a check to the existing test, created a new test to make sure appendNonRestoredState is propagated properly when set and ran make generate

@anandswaminathan
Copy link
Contributor

anandswaminathan commented Jul 3, 2019

Great. Thanks @YuvalItzchakov

Merging it. You can always use the image on the master - docker.io/lyft/flinkk8soperator:{SHA} and not wait for a release version.

@anandswaminathan anandswaminathan merged commit 9aaa992 into lyft:master Jul 3, 2019
@anandswaminathan
Copy link
Contributor

anandswaminathan commented Jul 8, 2019

@YuvalItzchakov This is not the right way to do it.

You will have to plumb the flag through the state machine. This case will not work when there is a rollback scenario.

Check this: https://github.com/lyft/flinkk8soperator/blob/master/pkg/controller/flinkapplication/flink_state_machine.go#L266

You will need to add the flag to the status as well - https://github.com/lyft/flinkk8soperator/blob/master/pkg/controller/flinkapplication/flink_state_machine.go#L366

@YuvalItzchakov
Copy link
Contributor Author

Thanks @anandswaminathan, I’ll get to work on the fix.

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.

2 participants