Aitozi commented on a change in pull request #85:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/85#discussion_r835883766
##########
File path:
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultDeploymentValidator.java
##########
@@ -245,6 +241,19 @@
JobSpec oldJob = oldSpec.getJob();
JobSpec newJob = newSpec.getJob();
+
+ if (lastReconciledSpec(deployment) != null
+ && ReconciliationUtils.triggerRestart(deployment)) {
+ if (newJob.getState() != JobState.RUNNING) {
+ return Optional.of(
+ String.format("Cannot restart job to %s state",
newJob.getState()));
+ }
+ if (oldJob.getState() != JobState.RUNNING) {
+ return Optional.of(
+ String.format("Cannot restart job in %s state",
oldJob.getState()));
+ }
+ }
Review comment:
> but why would anything happen anyways
I think the `restartNonce` have the semantic of bring the job to a
`start/running` status somehow? If not go with this check it will work like ask
job to restart once, but it still suspend :).
> we could discuss whether we should allow spec changes in suspended state
If JobState changes from suspended to suspended, with other spec changes, It
will not take effect, I think it will not bring bad impact to allow the change.
The problem of the `restartNonce` field change in suspended state is only the
problem of the semantic, as I said first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
