[Airship-discuss] Divingbell overrides functionality regression - Assistance needed

[ANDERSON, CRAIG]

According to https://github.com/imdario/mergo/blob/master/merge.go#L212-L214 :

// MergeWithOverwrite will do the same as Merge except that non-empty dst attributes will be overriden by
// non-empty src attribute values.
// Deprecated: use Merge(…) with WithOverride

So it would appear that this library made a change that broke backwards compatibility with their interface added in version 0.3.7, and that MergeWithOverwrite should be used with this verison of the library. Sprig exposes this with its `mergeOverwrite` function. Note that Sprig made the mergo version update (from 0.2.2 to 0.3.7) in the same commit that mergeOverwrite was added:

https://github.com/Masterminds/sprig/commit/4c59c1250a68e7f7a7e48c54f68c38b5c4c26d20#diff-040def46b3be654b6673438b43faf644

Given this as the case, I don't know how it would be possible that we could have a version of helm which both fails (because of new mergo version) and at the same time doesn't have mergeOverwrite (which was missing according to your earlier test). Just to confirm, were you using Helm 1.12.x when you tried replacing `merge` with `mergOverwrite`?

-----Original Message-----
From: Roman Gorshunov [mailto:paye600 at gmail.com] 
Sent: Wednesday, March 06, 2019 9:28 AM
To: ANDERSON, CRAIG <ca846m at att.com>; airship-discuss at lists.airshipit.org
Cc: TANG, ROY <rt7380 at att.com>
Subject: Re: Divingbell overrides functionality regression - Assistance needed

Oops, update:
.. with the help of Pete, not Scott. Sorry.

On Wed, Mar 6, 2019 at 5:49 PM Roman Gorshunov <paye600 at gmail.com> wrote:
>
> Hello Craig, team,
>
> With some help from Scott I've found exact commit in dependency of 
> helm - mergo, which breaks functionality:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_imdari
> o_mergo_commit_df0700a1e899196d8020767981b82623c8738932&d=DwIFaQ&c=LFY
> Z-o9_HUMeMTSQicvjIg&r=aimn2OylFgog_5_aS85wtQ&m=1MOIIBAVEcoin5q75GkbgRj
> DRRkoS7kHgavWlrLPTj0&s=jwfTMA8dPqMjzUGEXnUJ9nSrsD7jI8B_LVuBu0HhnJQ&e=
> If line 52 (new one) returns `false`, then everything seems to work again.
>
> Would appreciate any help. Thank you.
>
> Details I have collected are in this Story:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__storyboard.openst
> ack.org_-23-21_story_2005095-23comment-2D117207&d=DwIFaQ&c=LFYZ-o9_HUM
> eMTSQicvjIg&r=aimn2OylFgog_5_aS85wtQ&m=1MOIIBAVEcoin5q75GkbgRjDRRkoS7k
> HgavWlrLPTj0&s=4wYIA7lsyCya6IttBmz8A2mwUDQg-HEAYt3AbYfjuQ4&e=,
> including some help on how to reproduce the issue and test different 
> versions.
>
> Best regards,
> -- Roman Gorshunov
>
> On Thu, Feb 28, 2019 at 5:32 PM ANDERSON, CRAIG <ca846m at att.com> wrote:
> >
> > Ok, our version of Helm must not be new enough to contain the version of Sprig library which has that patchset, so I guess it must be a different issue. But I would still suspect something with `merge` or one of the other changes to dict.go between helm versions.
> >
> > -----Original Message-----
> > From: Roman Gorshunov [mailto:paye600 at gmail.com]
> > Sent: Thursday, February 28, 2019 2:52 AM
> > To: airship-discuss at lists.airshipit.org
> > Cc: TANG, ROY <rt7380 at att.com>; ANDERSON, CRAIG <ca846m at att.com>
> > Subject: Re: Divingbell overrides functionality regression - 
> > Assistance needed
> >
> > Hello Craig,
> > Thank you for your reply and support.
> > I have tried to do a quick replace of `merge` to `mergeOverwrite`, and it didn't work. Linting fails on the following error:
> >
> > > [ERROR] templates/: parse error in
> > > "helm-toolkit/templates/utils/_daemonset_overrides.tpl": template:
> > > helm-toolkit/templates/utils/_daemonset_overrides.tpl:52: function 
> > > "mergeOvewrite" not defined
> >
> > Best regards,
> > -- Roman Gorshunov