Fix support for Knative services

[mark-burnett]

This re-instates support for image substitution with Knative services, which appears to have been broken when the fix for not substituting images in CRDs was implemented in #3833

[codecov]

Codecov Report

Merging #4249 into master will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted Files	Coverage Δ
pkg/skaffold/deploy/kubectl/visitor.go	95.83% <100.00%> (+2.28%)	⬆️
pkg/skaffold/kubernetes/log.go	36.84% <0.00%> (+0.32%)	⬆️
...affold/kubernetes/portforward/kubectl_forwarder.go	68.29% <0.00%> (+2.43%)	⬆️

[briandealwis]

Thanks for your PR @mark-burnett! So this patch turns an O(1) lookup into an O(n) linear search. I think this whitelist can be expressed as a map[apimachinery.GroupKind]bool?

[mark-burnett]

Hey @briandealwis, thanks for taking a look. I think this lookup is effectively also O(k), since the cost is bounded by the length of the whitelist rather than O(n) on the input.

That said, after benchmarking both, the map is faster even when only looking for the first element in the list. My only guess is that this is related to string comparison being possibly slower than hashing the object.

I'll follow up with an update.

Benching Map
  avg 106ns;  min 54ns;  p50 101ns;  max 582µs;
  p90 105ns;  p99 136ns;  p999 399ns;  p9999 13.4µs;
       54ns [ 4123649] ██████████
      100ns [15717538] ████████████████████████████████████████
      150ns [   36341] 
      200ns [   26603] 
      250ns [   19593] 
      300ns [      58] 
      350ns [   56929] 
      400ns [   12095] 
      450ns [    2020] 
      500ns+[    5174] 

Benching List
  avg 126ns;  min 76ns;  p50 122ns;  max 248µs;
  p90 124ns;  p99 140ns;  p999 207ns;  p9999 15.7µs;
       76ns [       1] 
       80ns [       0] 
      100ns [ 1734240] ███▌
      120ns [18029092] ████████████████████████████████████████
      140ns [  151172] 
      160ns [   22589] 
      180ns [   35973] 
      200ns [   11689] 
      220ns [    3155] 
      240ns+[   12089] 

Benching Indexed List
  avg 138ns;  min 85ns;  p50 131ns;  max 85.2µs;
  p90 142ns;  p99 186ns;  p999 262ns;  p9999 18.1µs;
       85ns [       1] 
      100ns [       0] 
      120ns [17496609] ████████████████████████████████████████
      140ns [ 2094673] ████▌
      160ns [  192674] 
      180ns [   78874] 
      200ns [   68662] 
      220ns [   37147] 
      240ns [   10704] 
      260ns+[   20656] 

Benching Indexed List Valid Only
  avg 121ns;  min 62ns;  p50 111ns;  max 262µs;
  p90 132ns;  p99 146ns;  p999 262ns;  p9999 18.3µs;
       62ns [       1] 
       80ns [       0] 
      100ns [16863182] ████████████████████████████████████████
      120ns [ 1926083] ████▌
      140ns [ 1067329] ██▌
      160ns [   67193] 
      180ns [   25375] 
      200ns [   18533] 
      220ns [    7406] 
      240ns+[   24898] 

[briandealwis]

Thanks!