osd: allow for injecting extra env. variables via ConfigMap #9647
Conversation
rzarzynski
force-pushed
the
wip-extra-env-vars-2
branch
from
4cea311
to
26634f0
Compare
rzarzynski
force-pushed
the
wip-extra-env-vars-2
branch
from
26634f0
to
04ac3fc
Compare
| @@ -32,6 +32,7 @@ const ( | |||
| osdWalSizeEnvVarName = "ROOK_OSD_WAL_SIZE" | |||
| osdsPerDeviceEnvVarName = "ROOK_OSDS_PER_DEVICE" | |||
| osdDeviceClassEnvVarName = "ROOK_OSD_DEVICE_CLASS" | |||
| osdConfigMapOverrideName = "rook-ceph-osd-env-override" | |||
There was a problem hiding this comment.
Can we make this more generic and apply it to all the daemons instead of just the OSD?
There was a problem hiding this comment.
I just requested the opposite here. :)
There was a problem hiding this comment.
@travisn why not? This might be useful in the future for other daemons too..., looks simple too.
There was a problem hiding this comment.
How about if we merge this and follow up on this discussion? It will help unblock @rzarzynski.
For the follow-up, this is more like a development-only feature. This change is needed to work with Crimson in the short term, but is there another scenario? For any scenario we are aware of, the operator is expected to add env vars on the pod spec instead of the admin customizing it, so I'm not sure it's worth doing this for other daemons.
|
@leseb: I would be fine with extending this. How about a follow-up PR? |
I'm fine with that. |
| @@ -32,6 +32,7 @@ const ( | |||
| osdWalSizeEnvVarName = "ROOK_OSD_WAL_SIZE" | |||
| osdsPerDeviceEnvVarName = "ROOK_OSDS_PER_DEVICE" | |||
| osdDeviceClassEnvVarName = "ROOK_OSD_DEVICE_CLASS" | |||
| osdConfigMapOverrideName = "rook-ceph-osd-env-override" | |||
There was a problem hiding this comment.
I just requested the opposite here. :)
| @@ -0,0 +1,7 @@ | |||
| apiVersion: v1 | |||
There was a problem hiding this comment.
How about the following?
- Name this file
osd-env-override.yaml - Add comments at the top about how this configmap will be used. I don't think we need official documentation on this developer feature, just documenting here in the configmap seems sufficient.
- Add a simple comment below on why the ASAN_OPTIONS would be used in the example
This patch brings a mechanism to define arbitrary environment variables in OSD containers (both prepare and main ones). It bases on idea proposed by Sebastien Han to use an optional `ConfigMap` instance named `aook-ceph-daemon-env-override` as the source of these settings. The need for the patch comes from the fact that, although crimson finally exposes the same CLI interface as the classical OSD, the broadly used development builds have ASan built in. As ASan, by default, complains if it isn't the very first loaded DSO, we need a way to set the `ASAN_OPTIONS` environment variable to `verify_asan_link_order=0` to mitigate the early aborts. Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
rzarzynski
force-pushed
the
wip-extra-env-vars-2
branch
from
04ac3fc
to
17ca41f
Compare
| @@ -32,6 +32,7 @@ const ( | |||
| osdWalSizeEnvVarName = "ROOK_OSD_WAL_SIZE" | |||
| osdsPerDeviceEnvVarName = "ROOK_OSDS_PER_DEVICE" | |||
| osdDeviceClassEnvVarName = "ROOK_OSD_DEVICE_CLASS" | |||
| osdConfigMapOverrideName = "rook-ceph-osd-env-override" | |||
There was a problem hiding this comment.
How about if we merge this and follow up on this discussion? It will help unblock @rzarzynski.
For the follow-up, this is more like a development-only feature. This change is needed to work with Crimson in the short term, but is there another scenario? For any scenario we are aware of, the operator is expected to add env vars on the pod spec instead of the admin customizing it, so I'm not sure it's worth doing this for other daemons.
osd: allow for injecting extra env. variables via ConfigMap (backport #9647)
This is a reworked, implemented in a very different way version of the PR #9516.
Checklist:
make codegen) has been run to update object specifications, if necessary.