Skip to content

Ability to customize default initContainer resources#451

Merged
HoustonPutman merged 6 commits intoapache:mainfrom
HoustonPutman:init-cont-resources
Jul 7, 2022
Merged

Ability to customize default initContainer resources#451
HoustonPutman merged 6 commits intoapache:mainfrom
HoustonPutman:init-cont-resources

Conversation

@HoustonPutman
Copy link
Contributor

@HoustonPutman HoustonPutman commented Jun 30, 2022

Resolves #448

The new SolrCloud option is Spec.customSolrKubeOptions.podOptions.defaultInitContainerResources.

The option can also be enabled in the Solr Helm chart through: podOptions.defaultInitContainerResources.

The default initContainers will also come with default resources starting with this PR.

Copy link

@huijieliu huijieliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this change!

}

volumePrepResources := corev1.ResourceList{
corev1.ResourceCPU: *DefaultSolrVolumePrepInitContainerCPU,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏This will help to make sure if we configure our solr resources with limit=request as well, the pod will get assigned with QoS class Guaranteed

{{- toYaml .Values.podOptions.topologySpreadConstraints | nindent 2 }}
{{ end }}
{{- if .Values.podOptions.defaultInitContainerResources -}}
defaultInitContainerResources:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed offline, using default resource to configure both init container resource should be good for now if we need to custom init container resources as both are light weight init containers.

Copy link
Contributor

@gerlowskija gerlowskija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[0] Looks like this comment is out of sync with the new field-name you ultimately went with here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@HoustonPutman HoustonPutman merged commit 8e773dd into apache:main Jul 7, 2022
@HoustonPutman HoustonPutman deleted the init-cont-resources branch July 7, 2022 16:40
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

Successfully merging this pull request may close these issues.

Allow option to specify init-containers (setup-zk, cp-solr-xml etc.) resources

3 participants

Comments