https://flyte.org logo
Join Slack
Powered by
When using the `flyte-binary` helm chart is there ...
# flyte-deployment
f
When using the 
flyte-binary
helm chart is there a way to specify which project's you'd like automatically defined? I'm looking for something like 
initialProjects
from the 
flyte-core
helm chart
I'm also happy to contribute the change to the 
flyte-binary
helm chart which calls 
flyteadmin
in an 
initContainer
, but I'm working on the basis this wasn't ported to 
flyte-binary
for a reason?
t
cc @thankful-minister-83577 /@freezing-boots-56761
f
t
it wasn’t ported because i think it was an extra init container in the other chart, and we just wanted to avoid that.
f
maybe we can just make it a config?
t
we can i suppose.
adds to the config though, and it’s a lot as it is.
f
true
t
not against it, just a bit concerned about config bloat
f
t
helm config
f
got it 👍🏼
t
flyte admin config is more easily managed i feel
if we add this, i’d add this only to the single binary config
f
From my perspective, being able to update our helm values.yaml and pass values into the admin_config has been quite nice.
f
right
t
like the cfg.Admin block here
f
t
yeah right?
f
that’s what i was thinking too. then some plumbing from helm values into the configmap?
t
yeah
mike you want to do this? or jeev?
f
@faint-smartphone-23356 up to you
f
I'm happy to take a look at it. It gives me a good reason to better familiarize myself with your configmap.
t
awesome
🙂
f
is flytestdlib/config just a light wrapper over pflag?
(lazy web, I'm looking now :))
t
yeah
f
If I create an issue to track this; can one of you assign it to me?
t
yup
thanks
f
👍🏼
ty
t
thank you
f
@freezing-boots-56761 & @thankful-minister-83577 I (finally!!) got around to implementing this. https://github.com/flyteorg/flyte/pull/3631 I'm not sure where is best to document this change; I'm happy to do so, but I'll need a pointer to where best to put those docs.
t
oh nice thanks.
👍🏼 1
not to pile, but would you be up for adding to the flyte-binary helm chart as well?
i think it’s okay to keep as seed projects
f
@faint-smartphone-23356 looks like the PR is close and just needs some final touches :)
wanna talk about the names and configmap structure here?
f
hey @freezing-boots-56761 👋🏼
just looking over the changes; there are three outstanding: • not bumping the chart number - I’ve rolled this back • moving the flyte args up to 000-core.yaml - done • what to call the helm chart key - I’m happy to take direction on this.
f
@faint-smartphone-23356: still seeing the version changes, but that probably needs 
make helm
as far as the helm chart key goes: do we even need it? or can we just expose directly under the 
configuration
key? im not super opinionated on this. will defer to you and @thankful-minister-83577. everything else lgtm! thanks for the valuable contribution!
f
@freezing-boots-56761 good spot with the `make helm`; I forgot that. I’ve run it and the version changes have bumped. re: the configuration key. You’re suggesting: • 
configuration.admin
• 
configuration.propeller
• 
configuration.dataCatalog
instead of 
configuration.flyte-binary.[admin,propeller,dataCatalog]
. I’m fine with this personally; it’s sort of the same thing from my POV 🙂 so I’m happy to do whatever you and @thankful-minister-83577 suggest I do.
f
no strong opinions from me. i was suggesting that because all of configuration is technically already for 
flyte-binary
f
@freezing-boots-56761 / @thankful-minister-83577 here’s a diff that demonstrates the suggestion above: https://github.com/ossareh/flyte/pull/1/files some things to consider: 1. 
000-core.yaml
already has a top level 
admin
entry 2. the args for 
flyte-binary
are in a 
Section
called 
flyte
a. we either change 
cmd/single/config.go
to use a different section name b. or we accept that 000-core.yaml cannot have a top level 
admin
key and we put the flyte-binary args under 
flyte:
in the ConfgMap. If the diff above is acceptable, I’ll cherry pick the commit onto the existing PR in the flyte repo - the PR above is just for demonstration purposes.
Here’s another example: https://github.com/ossareh/flyte/pull/2 This is based on this feedback from Yee.
t
i like that last one.
jeev if you don’t mind can we just merge that?
👍 1
f
yea feel free!
🦜 1
f
OK; I'll migrate the commit over to the main PR branch and then ping y'all here when it's pushed.
@thankful-minister-83577 I've pushed the update with the new values.yaml - as per above. LMK if you see anything odd.
158 Views
Open in Slack
PreviousNext
Powered by