Add imagePullPolicy support to spinApp spec

[antweiss]

Implements #196

[calebschoepp]

I don't think we'll want to support everything that the Pod Spec supports but this seems like a reasonable addition to me. LGTM!

[calebschoepp]

Nit

Suggested change

	ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`
	// Checks defines health checks that should be used by Kubernetes to monitor the application.
	ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`
	// Checks defines health checks that should be used by Kubernetes to monitor the application.

[antweiss]

Agreed. No need to support the entire pod spec. And happy this looks reasonable. I hit this after 15 minutes playing with SpinKube and my guess I'm not the only one who'd expect this to be supported.

[calebschoepp]

Looks like your commit isn't signed. The contribution guide has more on doing that. Reach out if you get stuck on this.

And I forgot to say, thanks for your contribution. Thank you!

[antweiss]

Looks like your commit isn't signed. The contribution guide has more on doing that. Reach out if you get stuck on this.

And I forgot to say, thanks for your contribution. Thank you!

That's weird. I've spent about half an hour on setting up commit signing especially for this 😊
Will recheck a bit later.

Ok, now I see I forgot to add the GPG key to Github.
Looks fine now.

[endocrimes]

I'm a little hesitant as to whether we do want to support this actually (it was a deliberate omission from the first release).

Encouraging the use of mutable tags results in a lot of complexity and long term debugging and support headaches - and the defaults are reasonable (https://kubernetes.io/docs/concepts/containers/images/#imagepullpolicy-defaulting).

I'd rather guide folks towards having deterministic development environments (and being able to verify that they are actually testing the latest version of their code) than the confusion of the raciness of mutable image pulls 😅

The main argument for exposing some of this configuration is to allow exposing Never for clusters that are airgapped - but I'd at that point be tempted to make Always an error.

[antweiss]

@endocrimes I see what you mean. Encouraging the use of mutable tags is definitely not something we want to do. Yet the default behavior seems to encourage the use of latest, which can also be seen as a malpractice.
I just think making this available - exactly as it is in Pod API - would make the onboarding experience easier and more intuitive. Fact is I was expecting this to work. And it disappointed me it didn't. What do you think?

[endocrimes]

@antweiss It's a difficult balancing act between "flexibility" and pushing people towards safer practices. We don't encourage :latest (but it is equally as bad a practice as :dev or :branchname or any other unversioned artifact), but it is an escape hatch for those who "need" it.

I would like to hear a little bit about why that's a thing you want and how it aids your development workflow though? The Kubernetes project gets a lot of issues from folks who get into very broken situations because of mutable tags and Always policies because they never quite work like one would expect them to 😅.

[endocrimes]

(I ask how this interacts with your development workflow because their are potentially some more stable options than relying on imagePullPolicy by building more convenient tooling into the spin kube plugin, but I want to make sure we don't miss why you want this)

[rajatjindal]

I've reported spinkube/spin-plugin-kube#70 to try out an alternative approach by handling this on spin kube plugin side.

[antweiss]

To say the truth I wasn't even using the spin kube command. Thanks for bringing this to my attention. I was just spin registry push --build my-image:0.0.1 and then kubectl delete pod.
Looked at spin kube scaffold and what @rajatjindal is now suggesting. Definitely looks like a way to tackle this. But then in my case I could've just used latest and it would work too. I was looking for a way not to build anything as latest and also not to have to redeploy each time I rebuild the image.
I guess what I'm saying is I'm ok with mutability during development. But I realize it's a matter of personal preference.