Windows spot instances can fail to register due to incorrect tagging

[chadlwilson]

There seems to be some race conditions possible when creating and tagging spot instance requests which can lead to a Windows spot instance being created with name ${cluster}_LINUX_SPOT_INSTANCE despite being a Windows instance.

At root this seems to be due to the LINUX fallback at

gocd-ecs-elastic-agent/src/main/java/com/thoughtworks/gocd/elasticagent/ecs/domain/Platform.java

Line 25 in b1be8c3

return Arrays.stream(values()).filter(p -> p.name().equalsIgnoreCase(platform)).findFirst().orElse(LINUX);

This seems to be used by the code here during server ping

gocd-ecs-elastic-agent/src/main/java/com/thoughtworks/gocd/elasticagent/ecs/aws/SpotInstanceService.java

Lines 136 to 148 in b1be8c3

	public void tagSpotInstances(PluginSettings pluginSettings) {
	List<SpotInstanceRequest> spotRequests = spotInstanceHelper.getSpotRequestsWithARunningSpotInstance(pluginSettings, pluginSettings.getClusterName());
	LOG.debug("[server-ping] There are total of: '{}' Spot Requests with Spot-Request-Ids: '{}' which have a running Spot Instance. Starting tagging of Spot Instance.",
	spotRequests.size(), spotRequestIds(spotRequests));
	Map<String, List<SpotInstanceRequest>> requestsByPlatform = groupSpotRequestsByPlatform(spotRequests);
	requestsByPlatform.forEach((platform, spotInstanceRequests) -> {
	List<String> spotInstanceIds = spotInstanceRequests.stream().map(SpotInstanceRequest::getInstanceId).collect(toList());
	spotInstanceHelper.tagSpotResources(pluginSettings, spotInstanceIds, Platform.from(platform));
	});
	}

Somewhere along the line, the platform tag is either empty (rather than not found?) or has got some default value of Linux set. Not quite sure of the root here, but likely some kind of race condition when creating new spot requests and/or tagging untagged instances via the server ping.

There is also some hard-to-grok code here that is outside the synchronized block and suspicious looking synchronization in general:

gocd-ecs-elastic-agent/src/main/java/com/thoughtworks/gocd/elasticagent/ecs/aws/SpotInstanceService.java

Lines 124 to 131 in b1be8c3

	/*
	A spot instance request is tagged post creation. AWS takes time to sync up the the tags on the spot request, hence
	querying aws for the spot requests with tag filters does not yield results. Hence the SpotInstance service maintains
	a list of new spot instance requests and removes from the list in future calls or as part of server ping.
	*/
	if (isSpotRequestValid(spotInstanceRequest)) {
	untaggedSpotRequests.add(spotInstanceRequest);
	}