-
Notifications
You must be signed in to change notification settings - Fork 200
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
Option for additionalConfig
to update the opensearch.yml
#308
Comments
I think we should not give the user a choice to avoid more complexity (both in the implementation and for the user) and just only use the Sidenote: Using |
Hi everybody 😄 // Add config map with opensearch.yml
volumeName := fmt.Sprintf("%s-volume", cmName)
volumes = append(volumes, corev1.Volume{
Name: volumeName,
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: cmName,
},
},
},
})
volumeMounts = append(volumeMounts, corev1.VolumeMount{
Name: volumeName,
MountPath: "/usr/share/opensearch/config/opensearch.yml",
SubPath: "opensearch.yml",
}) If I understand the issue correctly, the error stems from My attempt for the logic for still keeping the base configuration generated by configuration.go was this: for _, nodePool := range r.instance.Spec.NodePools {
headlessService := builders.NewHeadlessServiceForNodePool(r.instance, &nodePool)
result.CombineErr(ctrl.SetControllerReference(r.instance, headlessService, r.Client.Scheme()))
result.Combine(r.ReconcileResource(headlessService, reconciler.StatePresent))
cmName := fmt.Sprintf("%s-config-%s", r.instance.Name, nodePool.Component)
baseConfig := r.reconcilerContext.OpenSearchConfig
withGeneral := helpers.MergeConfigs(baseConfig, r.instance.Spec.General.AdditionalConfig)
mergedConfigs := helpers.MergeConfigs(withGeneral, nodePool.AdditionalConfig)
cm := builders.NewClusterConfigMapForCR(
r.instance, cmName, mergedConfigs,
)
result.CombineErr(ctrl.SetControllerReference(r.instance, cm, r.Client.Scheme()))
result.Combine(r.ReconcileResource(cm, reconciler.StatePresent))
result.Combine(r.reconcileNodeStatefulSet(nodePool, username, cmName))
} Any ideas on how to tackle this @swoehrl-mw ? |
I don't think even it's added to environment variable. I have this in my general section, ideally as per this MR bootstrap POD should honour the general additionalconfig setting but it doesn't show the setting in env variable.
Also as suggested in MR I tried to use the following but it gives error in validation.
Error
One more issue why it doesnt allow the boolean values that's why I am using false as string.
|
Hi @ervikrant06. The PR you linked is not yet part of a release, as such you will only get that functionality if you are building the operator yourself from master.
This is a limitation of the type definitions we are using to define the CRD. Providing things like booleans or integers as strings is the normal way for this. |
Thanks @swoehrl-mw After building the image from master I can see the custom setting reflected in env variable, will continue discussion in original bug report.
|
Closing the issue :) |
Problem Statement
Coming from the comment #204 (comment).
Passing
additionalConfig
to the yaml, does not update theopensearch.yml
file directly, but add as anenv
value to the generated statefulset, one way this solves the purpose to pass the additional config to the cluster via environment values, but leaves a gap for additional config that has to be directly passed to theopensearch.yml
.Solution Proposed:
Have a mechanism to pass the
additionalConfig
directly toopensearch.yml
and reload/restart the cluster.Have a way for a user to distinguish if passed
additionalConfig
should be as an env or inject toopensearch.yml
, so this way the existing setup is not broken and new support to inject toopensearch.yml
is solved.@idanl21 @swoehrl-mw @segalziv @dbason and all other community folks, please add your thoughts
The text was updated successfully, but these errors were encountered: