Don't download nodeup if already in the AMI

[johngmyers]

No description provided.

[olemarkus]

This is great :)
I have been meaning to create a nodeup configuration mode similar to warmpool, but that can be run with packer or similar to build AMIs. This would fit nicely with such a feature.

[hakman]

Probably best to move urls (not used anywhere else) and chmod to download-or-bust() and keep download-release() clean.

Suggested change

	local -r nodeup_urls=( $(split-commas "${NODEUP_URL}") )
	download-or-bust nodeup "${NODEUP_HASH}" "${nodeup_urls[@]}"
	chmod +x nodeup
	download-or-bust nodeup "${NODEUP_HASH}" "${NODEUP_URL}"

[johngmyers]

download-or-bust has two return points, so it is simpler to have the chmod outside it. Also, it's not completely impossible that we might later need to download a second, non-executable file before running nodeup.

[hakman]

I agree, chmod would be better outside. Could you move the nodeup_urls part though?

[johngmyers]

There's something to be said for having the wrong nodeup binary cause a bootstrap failure. In the case of an out-of-date AMI, a noticeable failure might be better than slowness.

[hakman]

If nodeup is re-downloaded, in most cases means a few seconds. Most people should not notice that part.
If we stop a cluster from scaling or replacing nodes just because the AMI is outdated, seems we would create quite a big issue. Maybe printing a warning would make it easier for people to notice this in logs.

[johngmyers]

A few seconds might be enough to blow your SLA. It might not be noticed until peak traffic and even then the cause wouldn't be obvious.

Failure to replace nodes on the rolling update following a configuration change would be much more likely to be noticed quickly, before it becomes a big problem.

[hakman]

If anyone is that serious about not starting if nodeup is out of date, could have a boot script that checks the sha256 and stops the process.
😄

[hakman]

I am ok with this as is. Thanks.
In case you would like more feedback...
/hold

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [hakman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[johngmyers]

I'm okay with this for now. We can iterate as follow-up.
/hold cancel