John Howard

> @howardjohn `You can see appendSidecarServices, it always respect k8s first when merging services. You should make the new feature flag work their too.` ah I understand. will do

sorry actually I do not get it. There is a feature flag protection there on L766 as far as I can tell?

> There is a diverge between `appendSidecarServices` and the new behavior in this pr. Do we need to add the new behavior in `appendSidecarServices` with the feature protection too. again...

> Please take a look at `appendSidecarServices`. L766 does not work on it at all. > > SidecarScope.services and egressListener.services have a big divergence now can you please just tell...

> https://github.com/istio/istio/blob/24e61d1441c179a6dd75cfe70ed5d25bdb6a58ec/pilot/pkg/model/sidecar.go#L421 > > It iterate over all the egressListener's services. > And later in `appendSidecarServices` it does not consider config namespace at all. > > https://github.com/istio/istio/blob/24e61d1441c179a6dd75cfe70ed5d25bdb6a58ec/pilot/pkg/model/sidecar.go#L981 > > If...

> appendSidecarServices should respect the confignamespace too. Isn't that this pr mostly working for I don't understand.. can you show a config that doesn't work as expected? this PR is...

why is that wrong? that seems like the intended behavior since the beginning? Did the behavior change in this PR? if not, I would strongly prefer to keep the scope...

Ok but my intent here is to make the non-sidecar behavior the same as the sidecar behavior. I am not convinced we want to change the behavior of sidecar which...

One thing to keep in mind is the main purpose of this is to fix a critical regression in Istio. Yes we have all sorts of crazy weird merging behavior.....

I really have minimal desire to ever change behavior of stuff like this in istio at this point - the best thing to do when something is in this position...