fix(deploy): Gracefully handle missing artifacts #918
Conversation
When deploying a version of Spinnaker before Kayenta, the distributed GCE deployment currently throws an NPE.
|
With this change (and after re-generating some missing GCE images), I was able to successfully deploy both 1.6.0 and 1.6.1 using a distributed GCE setup. |
| return String.format("projects/%s/global/images/%s", | ||
| getGoogleImageProject(deploymentName), | ||
| String.join("-", "spinnaker", artifactName, version.replace(".", "-").replace(":", "-"))); | ||
| if (version == null) { |
There was a problem hiding this comment.
I think this should be handled upstream from here. I realize this is where the exception will be thrown, but I think the deployer in charge of picking which services to create should be smarter about asking for "artifact ids" of services that can't be created.
There was a problem hiding this comment.
I'm sort of torn on whether this should be handled here or upstream. I actually think that it's this function (or perhaps the downstream one getArtifactVersion) that's in the best position to know whether or not a given service should be built. Effectively, the question of whether a service should be built is answered by looking at whether it's in the BOM---which is what this function is doing to get the version to deploy. So I think it make sense that asking "what version of X should I deploy?" would return null if the answer is not to deploy X.
That being said, I agree that it's a bit silly that we still end up with an entry in endpoints.services for a service that's not being deployed...so maybe we should write a boolean serviceExists() that just checks if there's a version at all and short-circuit all the settings-building if it's false. Let me see what this last idea would look like.
|
Tested by doing the following deployments: LocalDebian, BakeDebian, Kubernetes (V1), Kubernetes (V2), Distributed GCE. |
|
@lwander : Mind taking a look at this updated change? |
When deploying a version of Spinnaker before Kayenta, the distributed GCE deployment currently throws an NPE.