Celery-Kubernetes-Operator icon indicating copy to clipboard operation
Celery-Kubernetes-Operator copied to clipboard

Add production ready CRD spec

Open gautamp8 opened this issue 4 years ago • 8 comments

This PR adds a production-ready custom resource definition for the celery cluster. On a high-level, it adds support for the following parameters -

  • image - Container image name to run in the worker and flower deployments
  • imagePullPolicy - Image pull policy. One of Always, Never, IfNotPresent
  • imagePullSecrets - ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used
  • appName - Application name for worker and flower deployments, will be suffixed accordingly
  • celeryApp - Celery app instance to use (e.g. module.celery_app_attr_name)
  • celeryVersion
  • workerReplicas - Number of worker pods to be run
  • flowerReplicas - Number of flower pods to be run
  • workerSpec - Worker deployment-specific parameters (worker args, envs, nodeSelector and resources spec)
  • flowerSpec - Flower deployment specific parameters (flower args, envs, nodeSelector, resources and flower service spec)
  • initContainers - List of initialization containers belonging to the worker and flower pods
  • volumeMounts - Define some extra Kubernetes Volume mounts for Celery cluster pods
  • volumes - Define some extra Kubernetes Volumes for Celery cluster pods
  • livenessProbe - Periodic probe of container liveness
  • readinessProbe - Periodic probe of container readiness

In the future, we plan to include broker config, update strategy, and autoscaling config support to this CRD as well.

gautamp8 avatar Feb 21 '21 12:02 gautamp8

Is this ready for review?

thedrow avatar Feb 28 '21 10:02 thedrow

Is this ready for review?

Yes, CRD can be reviewed, but, before that I'm thinking to include a detailed documentation as well so that it's easy to review. It's a 3000 line thing, might be difficult. I'll ping once done.

Next step is to create a custom resource for the same and update Python handlers to setup and teardown of workers and flower deployment. I'm thinking of writing a Pod generator (something like this - https://github.com/davlum/incubator-airflow/blob/master/airflow/kubernetes/pod_generator.py#L111)

gautamp8 avatar Feb 28 '21 11:02 gautamp8

@thedrow I had added documentation for the CRD in a structured format. You could start by reviewing it and suggest any changes if needed.

I'm working on modeling spec received via handlers and generating a Kubernetes deployment programmatically. I'll push that commit in the same PR in a day or two completing this first patch.

gautamp8 avatar Mar 21 '21 10:03 gautamp8

the CRD.yaml file is too big.

Yes. It includes support for lots of standard Kubernetes resources/objects specification like volumes, volumeMounts, envs, init containers etc. that are commonly used while deploying any application. I wrote brief documentation to outline the major parts and I think that should be a good starting point for review.

For reference, I reviewed the CRDs of other popular operators to write ours -

  • https://github.com/elastic/cloud-on-k8s/blob/master/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml
  • https://github.com/banzaicloud/kafka-operator/tree/master/config/base/crds

did you use it for OpenStack deployment? did you tried anything from helm 3.x?

The deployment stage will come later, I've access to a production EKS cluster from my work where I'll test this operator once ready. Could use OpenStack too.

Helm we'll use later to package CRDs and other resources to deploy the operator. We'd still need this CRD spec. I'm yet to work on the packaging and releasing part, Helm will be useful there if I'm correct.

I'm currently writing the handlers to deal with the resource created using this CRD. I'm using Kind for simulating local K8s cluster. I should have that commit ready by tomorrow.

gautamp8 avatar Mar 25 '21 10:03 gautamp8

This pull request introduces 3 alerts and fixes 2 when merging c03b6991b4de85c13e32da457d9f8905f87a6254 into 3163d6c30702a5f2430dcfb2dc07aa54c002803a - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'
  • 1 for Unused import
  • 1 for Wrong number of arguments in a call

fixed alerts:

  • 1 for Unused local variable
  • 1 for Unused import

lgtm-com[bot] avatar Mar 26 '21 18:03 lgtm-com[bot]

@auvipy @thedrow this is ready for review

gautamp8 avatar Apr 02 '21 12:04 gautamp8

@auvipy @thedrow this is ready for review

I'll probably only have time to review this after 5.1 is released.

thedrow avatar Apr 06 '21 11:04 thedrow

@auvipy @thedrow this is ready for review

I'll probably only have time to review this after 5.1 is released.

Thanks, Sorry for the slow progress from my side as well. I try to commit every Friday to this project. Might have to pause for a bit because of the extreme covid surge in my country(India) to take care of other commitments. I'll try to cover things up next month.

gautamp8 avatar Apr 22 '21 07:04 gautamp8