benthosdev / benthos-captain

A Kubernetes Operator to orchestrate Benthos pipelines
MIT License
38 stars 8 forks source link

Add Support for additional deployment configuration #18

Open mikebaum opened 2 months ago

mikebaum commented 2 months ago

Thanks for starting this project. I'm currently evaluating using the operator to deploy pipelines in my organization. At this point I understand that it alpha, but I was wondering if you have any plans to add additional configuration options to the Pipeline resource.

I can see a clear need for the following configurable properties:

It would seem simple enough to add these and more if necessary. In fact I have already modified the controller to add: EnvFrom, Resources, Labels, and Annotations which correspond to the available configuration in the DeploymentSpec.

I did this only to evaluate if we could use the operator and it was very straightforward to implement. I added these properties to the root of the schema, but that was just a quick and dirty approach since it meant not having to create a new type, but rather just update the Deployment that the operator creates with the additional configuration.

I would be willing to submit PRs to help in the development effort.

mfamador commented 2 months ago

Hey, @mikebaum , sure, any contribution is very welcome! Feel free to submit a PR.

mikebaum commented 2 months ago

@mfamador Awesome, in terms of the design would you be okay with adding the properties to the root schema? Something like this:

type PipelineSpec struct {
    Config *apiextensionsv1.JSON `json:"config,omitempty"`
    Replicas int32 `json:"replicas,omitempty"`
    Image string `json:"image,omitempty"`

        // new properties below

    EnvFrom []v1.EnvFromSource `json:"envFrom,omitempty"`
    Resources v1.ResourceRequirements `json:"resources,omitempty"`
    Labels map[string]string `json:"labels,omitempty"`
    Annotations map[string]string `json:"annotations,omitempty"`
}

and then just setting them in the appropriate place in the DeploymentSpec that is created by the controller?

mfamador commented 2 months ago

maybe we can add them below deployment to be clearer those to be added to it. we might want to add others in the future, like hpa or service monitors

mikebaum commented 2 months ago

Sounds good to me. Some of these properties will come from varying levels of the DeploymentSpec schema, I feel like flattening the property path makes sense, but I fear a name collision. Using the original names makes sense to me, perhaps it's enough that the doc describes exactly what part of the schema the property will update, i.e. Labels --> DeploymentSpec.Template.Labels.

mikebaum commented 2 months ago

Created PR #19 to get the ball rolling on adding additional properties to the pipeline API. I hesitated to put this new property into a deployment section as I felt like in this case we are updating a config map not the deployment. There are already existing properties of the Pipeline that refer to deployment properties, i.e. image and replicas, so it would have felt odd to create a deployment section and not include those properties as well. Perhaps this is the approach that we could take, eg. add properties that potentially modify more than one resource (Deployment, ConfigMap, Secret, etc...).