-
Notifications
You must be signed in to change notification settings - Fork 38
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
Issue 646: Add support for providing customized environment variables to controller #647
Conversation
… to controller Signed-off-by: anisha.kj <[email protected]>
@@ -211,6 +211,9 @@ func makeControllerPodSpec(p *api.PravegaCluster) *corev1.PodSpec { | |||
if p.Spec.Pravega.ControllerInitContainers != nil { | |||
podSpec.InitContainers = append(podSpec.InitContainers, p.Spec.Pravega.ControllerInitContainers...) | |||
} | |||
if p.Spec.Pravega.ControllerEnvVars != nil { | |||
podSpec.Containers[0].Env = p.Spec.Pravega.ControllerEnvVars | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not we rather append it to the existing array, like it's done for segment store?
podSpec.Containers[0].Env = append(podSpec.Containers[0].Env, p.Spec.Pravega.SegmentStoreContainerEnv...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For controller pods, there is no Env
added to podspec as in segmentstore. That is the reason added like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it clashes with this assignement:
pravega-operator/controllers/pravega_controller.go
Lines 288 to 294 in 5ddeabd
podSpec.Containers[0].Env = []corev1.EnvVar{ | |
{ | |
Name: "INFLUX_DB_SECRET_MOUNT_PATH", | |
Value: p.Spec.Pravega.InfluxDBSecret.MountPath, | |
}, | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jkhalack for pointing this. I have modified it as suggested
Signed-off-by: anisha.kj <[email protected]>
5ddeabd
to
92236f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: anisha.kj [email protected]
Change log description
Added support to provide customized env variables to controller pods
Purpose of the change
Fixes #646
What the code does
Added new field
ControllerEnvVars
in Pravega spec. Also updated CRDHow to verify it
Verified that able to provide customized env variables to controller pods