Adding missing Env variables for core sts #465
Conversation
jackyalbo
force-pushed
the
fix_root_secret_upgrade
branch
from
9ffad8c
to
d7c1595
Compare
pkg/system/phase2_creating.go
Outdated
| for j := range r.DefaultCoreApp.Env { | ||
| if (!existingEnvs[r.DefaultCoreApp.Env[j].Name] && | ||
| (r.DefaultCoreApp.Env[j].Value != "" || r.DefaultCoreApp.Env[j].ValueFrom != nil)) { | ||
| c.Env = append(c.Env, r.DefaultCoreApp.Env[j]) | ||
| } |
There was a problem hiding this comment.
I don't think that copy is enough as sometimes we leave envs values empty on purpose so the code will reconcile them with custom logic. If I will just delete n env that has an empty value (in the template) it will not get its reconciled value back.
I think a better approach is to take the list from the template then merge into it the current envs then run the reconcile code (the for with the switch statement) on the merged list
There was a problem hiding this comment.
@nb-ohad is right. just add a mergeEnvs function and call it before the envs for
There was a problem hiding this comment.
Just to clarify, you should merge only the missing envs from the template into the CR, then the reconcile env loop can stay the same
jackyalbo
force-pushed
the
fix_root_secret_upgrade
branch
from
d7c1595
to
3a23696
Compare
|
@dannyzaken @nb-ohad fixed to merge - please re-review |
pkg/util/util.go
Outdated
| for _, item := range *envB { | ||
| if !existingEnvs[item.Name] { | ||
| *envA = append(*envA, item) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I think something with the indentation here is wrong
jackyalbo
force-pushed
the
fix_root_secret_upgrade
branch
from
3a23696
to
0baf9ee
Compare
jackyalbo
force-pushed
the
fix_root_secret_upgrade
branch
2 times, most recently
from
d08acd0
to
0949ac9
Compare
jackyalbo
changed the title
jackyalbo
force-pushed
the
fix_root_secret_upgrade
branch
from
0949ac9
to
bb0b5a3
Compare
Signed-off-by: jackyalbo <jalbo@redhat.com>
jackyalbo
force-pushed
the
fix_root_secret_upgrade
branch
from
bb0b5a3
to
4cdc8bb
Compare
Added a new check on missing env variable and adding them if they appear in the yaml
Doing this only for core sts for now just for you guys to take a look - we can copy the logic to other yamls if we need to - for now tagged as WIP until we figure this out
Signed-off-by: jackyalbo jalbo@redhat.com