Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for additional deployment configuration #18

Open
mikebaum opened this issue May 3, 2024 · 5 comments
Open

Add Support for additional deployment configuration #18

mikebaum opened this issue May 3, 2024 · 5 comments

Comments

@mikebaum
Copy link
Contributor

mikebaum commented May 3, 2024

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:

  • Secrets
  • Resource Requirements (limits/requests)
  • Additional Labels and possibly Annotations
  • env configuration via Env and/or EnvFrom
  • Additional Volumes
  • Additional flags for the benthos cmd if configuration is not supported via env vars

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
Copy link
Member

mfamador commented May 3, 2024

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

@mikebaum
Copy link
Contributor Author

mikebaum commented May 3, 2024

@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
Copy link
Member

mfamador commented May 3, 2024

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
Copy link
Contributor Author

mikebaum commented May 3, 2024

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
Copy link
Contributor Author

mikebaum commented May 4, 2024

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...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants