search

GoogleCloudPlatform / flink-on-k8s-operator

[DEPRECATED] Kubernetes operator for managing the lifecycle of Apache Flink and Beam applications.
Apache License 2.0
658 stars 266 forks source link

Use StatefulSet instead of Deployment #354

Closed shashken closed 3 years ago

shashken commented 3 years ago

@functicons . Closes #353

functicons commented 3 years ago

Thanks for the PR! I need some time to digest this change. Sorry for the delayed response.

shashken commented 3 years ago

All good @functicons tell me if there is anything you want to discuss.

functicons commented 3 years ago

Sorry for the delayed response, I was super busy this week. I left some comments in #353, let's first discuss it there. Thanks!

shashken commented 3 years ago

@functicons Changed, sorry for the delay. There are some function names that I havn't changed like 'reconcileDeployment' do you think we should change those names as well?

functicons commented 3 years ago

Yes, I think it is better to update all of them.

shashken commented 3 years ago

Yes, I think it is better to update all of them.

Done

functicons commented 3 years ago

/gcbrun

shashken commented 3 years ago

@functicons I missed the review about CRD and doc, added them now. let me know if there is something else you want me to do before merging this.

thanidev commented 3 years ago

@shashken Can you please also add example sample Flink jobs and deployment involving RocksDB as state backend an SSD to config samples? This will be helpful!.

functicons commented 3 years ago

Could you sync your repo and resolve the conflicts? @shashken

shashken commented 3 years ago

@functicons resolved conflicts, please verify as the last PR merged today was a big one. @thanidev Good idea, I will happily create a different PR for that, but as this one takes a bit too long I want to merge this first and then I'll create a new small PR with the example If there aren't any problems lets merge this one, long lived PRs are not so healthy

functicons commented 3 years ago

/gcbrun

functicons commented 3 years ago

@shashken I have tested the PR, it worked perfectly, thanks!

burkaygur commented 3 years ago

Looks like the current examples are not compatible with this PR. I am getting the following error:

2020-12-09T23:06:58.272Z    ERROR   controller-runtime.controller   Reconciler error    {"controller": "flinkcluster", "request": "default/flinksessioncluster-sample", "error": "FlinkCluster.flinkoperator.k8s.io \"flinksessioncluster-sample\" is invalid: [status.components.jobManagerStatefulSet: Required value, status.components.taskManagerStatefulSet: Required value]"}
thanidev commented 3 years ago

@shashken @functicons Can you add examples - Sample Stateful Flink jobs and deployment involving RocksDB as state backend and SSD to config samples?

More examples are better for adoption of operator

ghost commented 3 years ago

@shashken @functicons Would the latest operator image v1beta1-8 on gcr.io support the changes made to use StatefulSets? Looks like there are still references to Deployment when using the sample configs for the job and session clusters.

shashken commented 3 years ago

@gshen92 I dont know how or when those images are built, you can build the docker image from the master branch and upload to your container registry for now to see that it works :) make operator-image push-operator-image IMG=<your_docker_registry>/flink-google-operator:0.0.<your_version> And you can install the operator with the helm chart from the master branch as well. @functicons Can probably help if you need an official image release