Pass --recreate-pods to helm by default in dev mode

[stevenjm]

It is common in development to just rebuild a Docker image with the same tag, which will not trigger a rolling update of the deployment because the pod spec does not change. Passing --recreate-pods will forcibly terminate the running pods and allow new ones to be spawned with the new image.

This change only applies in dev mode because it would be harmful for production deployments, which should push an image with a new tag and allow the usual rolling update process to replace the old pods.

This change would resolve #289, without requiring an additional configuration option. (This behaviour can be disabled at runtime with --recreate-pods=false if needed for some reason.)

For now, I have only added a test to verify that a helm deployment works if recreatePods == true. Developing a test for the behaviour of actually recreating pods would be a little more involved, and I'm not sure it's worth including in this PR.

[codecov-io]

Codecov Report

Merging #946 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #946      +/-   ##
==========================================
+ Coverage   43.78%   43.82%   +0.04%     
==========================================
  Files          63       63              
  Lines        2654     2656       +2     
==========================================
+ Hits         1162     1164       +2     
  Misses       1370     1370              
  Partials      122      122

Impacted Files	Coverage Δ
pkg/skaffold/deploy/helm.go	63.95% <100%> (+0.36%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba4bf67...887767a. Read the comment docs.

[dgageot]

@stevenjm Thank you for this contribution! Since this behavior is helm only, would it make sense to add an option to the helm deployer and make it the default?

I'm sure the code will be simpler too.

[stevenjm]

@dgageot Sure, that makes sense, and should be a straightforward change. I'll hopefully get some time tomorrow to implement it.

[stevenjm]

Whoops, since there's now a new option in skaffold.yaml I should also update the relevant examples/documentation. Will push another version of this change a bit later.

[stevenjm]

@dgageot I've made the change you requested. I haven't made the default true because this should never be on for production, so it's safer to explicitly turn it on for development.

In an ideal world, I'd still like to have it on by default for skaffold dev and not skaffold run, but as far as I can tell there is no way to determine whether the invoked command is dev or run from within the helm deployment implementation.

[dgageot]

@stevenjm Thanks! Maybe you could create a ticket to later have a default value based on the command (dev or run)