add OCIS_ENABLE_OCM env var #9784
Conversation
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
butonic
requested review from
wkloucek,
micbar,
phil-davis and
individual-it
as code owners
ocis-pkg/config/config.go
Outdated
| @@ -56,6 +56,7 @@ type Runtime struct { | |||
| Services []string `yaml:"services" env:"OCIS_RUN_EXTENSIONS;OCIS_RUN_SERVICES" desc:"A comma-separated list of service names. Will start only the listed services." introductionVersion:"pre5.0"` | |||
| Disabled []string `yaml:"disabled_services" env:"OCIS_EXCLUDE_RUN_SERVICES" desc:"A comma-separated list of service names. Will start all default services except of the ones listed. Has no effect when OCIS_RUN_SERVICES is set." introductionVersion:"pre5.0"` | |||
| Additional []string `yaml:"add_services" env:"OCIS_ADD_RUN_SERVICES" desc:"A comma-separated list of service names. Will add the listed services to the default configuration. Has no effect when OCIS_RUN_SERVICES is set. Note that one can add services not started by the default list and exclude services from the default list by using both envvars at the same time." introductionVersion:"pre5.0"` | |||
| EnableOCM bool `yaml:"enable_ocm" env:"OCIS_ENABLE_OCM" desc:"Enable all OCM config flags and start the ocm service." introductionVersion:"%%NEXT%%"` | |||
There was a problem hiding this comment.
Why do we need an extra envvar for this? Whats wrong with OCIS_ADD_RUN_SERVICES=ocm?
There was a problem hiding this comment.
AFAICT pm wanted a single env var to enable ocm. that means we have to also make the runtime pick up the env var to start the service.
There was a problem hiding this comment.
Please let's NOT do this. We have too many envvars anyways. We cannot add envvars for specific deployment scenarios. Next week there'll be OCIS_MY_CUSTOM_DEPLOYMENT which start antivirus and webfinger but not search.
Tell PO OCIS_ADD_RUN_SERVICES IS a single envvar.
There was a problem hiding this comment.
I agree. I was trying to argue that the helm chart is the layer that uses feature flags like this. @micbar was against it. And the whole point of this PR is to add a feature flag env var OCIS_ENABLE_OCM that sets all the other env vars. Making it also enable the service at least makes it not a half baked solution ...
all or nothing IMO
There was a problem hiding this comment.
I was trying to argue that the helm chart is the layer that uses feature flags like this
I'm 100% with you here. Please let's not do things we both know are wrong.
And the whole point of this PR is to add a feature flag env var OCIS_ENABLE_OCM that sets all the other env vars
Why don't we just default them all to true? Then OCIS_ADD_RUN_SERVICES would suffice the requirements. What is the point in starting ocm service if you don't want to use it anyways?
There was a problem hiding this comment.
setting them to true results in various error messages in the log because the ocm service cannot be contacted.
Looking at the list item New variable like OCIS_ENABLE_OCM should summarize most of the variable to simplify enablement of ocm we could just add OCIS_ENABLE_OCM to the existing boolean env vars and not start ocm ... but that seems to just be a half assed solution.
There was a problem hiding this comment.
Looking at the list item New variable like OCIS_ENABLE_OCM should summarize most of the variable to simplify enablement of ocm we could just add OCIS_ENABLE_OCM to the existing boolean env vars and not start ocm
I'd like that more. The runtime already listens to: OCIS_ADD_RUN_SERVICES, OCIS_EXCLUDE_RUN_SERVICES and OCIS_RUN_SERVICES. Listening to a fourth envvar will only make it more complicated and produces edge cases that need to be considered. (e.g. OCIS_EXCLUDE_RUN_SERVICES=ocm and OCIS_ENABLE_OCM=true)
OCIS_ENABLE_OCM activates all service specific envvars, ocm service needs to be started separately. That sounds nice and simple to me.
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
|
add OCIS_ENABLE_OCM env var
We added a new
OCIS_ENABLE_OCMenv var that will enable all ocm flags and addocmto the services started by the runtime.