Dennis-Mircea commented on PR #1173: URL: https://github.com/apache/flink-kubernetes-operator/pull/1173#issuecomment-5255578799
> @gyfora apologies if I've misread this and am wasting your time with a false alarm > > I was solely going off this bit: > > https://github.com/apache/flink-kubernetes-operator/blob/fdfcc695daba16e4f4a8c9a6c08b7f470787de00/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java#L257-L263 > > I thought this meant it'd reject a savepoint job with a null or whitespace checkpoints directory. Could you let me know what I missed please? @dalelane this is a good finding, you didn't misread it. That check applies to any non-stateless upgrade mode, so a savepoint job without a checkpoints directory is rejected. The `or` in the table is wrong as things stand. @gyfora is describing what is conceptually necessary, and with `kubernetes.operator.job.upgrade.last-state-fallback.enabled` set to false only the savepoint directory would be. The validator does not consult that flag, so in practice both are still required. I'd say it is worth considering gating the checkpoint directory check on `kubernetes.operator.job.upgrade.last-state-fallback.enabled`, so savepoint mode does not demand config it will never use. The catch is that the flag is dynamic while validation happens once at admission, so a job validated with the fallback off would be left in an invalid state if it is turned back on later.@gyfora what do you think? -- 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]
