MANTA-4744 'manta-adm update' should guard image usage by image.name #21
Conversation
cmd/manta-adm.js
Outdated
| @@ -967,6 +968,13 @@ MantaAdm.prototype.do_update.options = [ | |||
| 'type': 'bool', | |||
| 'help': 'Allow deployment of experimental services' | |||
| }, | |||
| { | |||
| 'names': [ 'skip-verify-remote' ], | |||
There was a problem hiding this comment.
Why the extra skip-verify-remote option? I don't see the utility beyond what 'skip-verify-channel' provides and it adds some complexity.
- if an operator can't reach updates.jo (air gapped or whatever), then they need to specific
--skip-verify-channelalready - What would be the case for specifying
--skip-verify-remotebut NOT--skip-verify-channel? - Couldn't an operator be surprised that specifying
--skip-verify-remoteimplies that the channel will not be checked. Many will not make the connection that channel info isn't kept on a locally imported image.
There was a problem hiding this comment.
Right, I agree with some of that: skip-verify-remote confusingly does imply skip-verify-channel. The reverse isn't true.
Developers would still need --skip-verify-channel (eg. when upgrading to dev image on a system with the "release" channel set) Now imagine the images they want to provision also aren't available locally yet, so our "check for local images" error message would inform them of that.
In that scenario, I'd rather also tell them that they can't deploy "mantav1-foo" here as early as possible, rather than simply not doing any remote lookups (if I understand you correctly, you were thinking that skip-verify-channel would also not do any remote lookups) Otherwise, developers will go to the trouble of downloading all of the images locally, only then, we'd tell them that they can't use the bits they downloaded, and we're back to whack-a-mole error states, which is a shame.
So what we could do is to require --skip-verify-channel if --skip-verify-remote is passed, that way, we're properly representing how the channel-checks work. Would that be better?
There was a problem hiding this comment.
if I understand you correctly, you were thinking that skip-verify-channel would also not do any remote lookups
Nah (though apologies if I've changed my tune), I think we should just go ahead and look up the image remotely if we don't have it locally (sdcadm does this here). I.e. I agree with you that we should tell them about the image name mismatch as early as possible.
About the operator that has their Manta airgapped such that they cannot reach updates.joyent.com: let's cross that road if/when we come to it. I was being theoretical there. As well, I think for that case, this operator would only be specifying image UUIDs that they had already manually installed, so no real harm here.
Thoughts?
There was a problem hiding this comment.
Hmm, we can't just look up the image remotely only if the image isn't local, as sdcadm does - by default we always need to do a remote lookup to see if the image is in the correct channel, regardless of whether it exists locally as well.
Now, perhaps there's a case to be made for optimsation here: if we're told that we don't care about channel verification and all images exist locally, then we're wasting time doing remote lookups, but is that worth the extra complexity?
There was a problem hiding this comment.
The optimization would be nice, but I'm not going to stop this change for it. :)
I don't think the code for it need feel complex. In stages:
- get all the new images locally
- if opts.skip_verify_channel, then filter down the set of remote images to get to just those we didn't find locally
- get the remote image manifests, if any left
There was a problem hiding this comment.
Yeah, it really wasn't too bad. I've added that, along with the CLI check that skipping remote should also imply skipping channel verification.
This looks like:
[root@0cf080f3-51de-4103-a13b-e55a8853083d (coal-1:manta0) ~]# manta-adm update --skip-verify-remote new.json
manta-adm update: error: Channel verification requires remote imgapi lookups. Use --skip_verify_channel as well as --skip-verify-remote.
[root@0cf080f3-51de-4103-a13b-e55a8853083d (coal-1:manta0) ~]#
Then doing an update where we ought to reprovision two services, but only have one local image:
[root@0cf080f3-51de-4103-a13b-e55a8853083d (coal-1:manta0) ~]# manta-adm update new-one-local-one-remote-correct-channel.json
logs at /var/log/manta-adm.log
manta-adm: The following images are not available in the local imgapi instance:
29bcf204-59c7-4292-a1e8-ef3e10d5ff52 (webapi)
with log entries:
[2019-11-28T11:35:25.756Z] DEBUG: manta-adm/6660 on 0cf080f3-51de-4103-a13b-e55a8853083d: getting local image 94ea1644-a329-4e45-9cb6-f5a5fc102f87 for service storage
[2019-11-28T11:35:25.757Z] DEBUG: manta-adm/6660 on 0cf080f3-51de-4103-a13b-e55a8853083d: getting local image 29bcf204-59c7-4292-a1e8-ef3e10d5ff52 for service webapi
[2019-11-28T11:35:25.807Z] DEBUG: manta-adm/6660 on 0cf080f3-51de-4103-a13b-e55a8853083d: missing local image 29bcf204-59c7-4292-a1e8-ef3e10d5ff52 (webapi)
[2019-11-28T11:35:25.808Z] DEBUG: manta-adm/6660 on 0cf080f3-51de-4103-a13b-e55a8853083d: getting remote image 94ea1644-a329-4e45-9cb6-f5a5fc102f87 for service storage on channel release
[2019-11-28T11:35:25.811Z] DEBUG: manta-adm/6660 on 0cf080f3-51de-4103-a13b-e55a8853083d: getting remote image 29bcf204-59c7-4292-a1e8-ef3e10d5ff52 for service webapi on channel release
and now the same check, this time when skipping channel verification, we only look for one remote image:
[root@0cf080f3-51de-4103-a13b-e55a8853083d (coal-1:manta0) ~]# manta-adm update --skip-verify-channel new-one-local-one-remote-correct-channel.json
logs at /var/log/manta-adm.log
manta-adm: The following images are not available in the local imgapi instance:
29bcf204-59c7-4292-a1e8-ef3e10d5ff52 (webapi)
and the log entries:
[2019-11-28T11:38:15.133Z] DEBUG: manta-adm/7275 on 0cf080f3-51de-4103-a13b-e55a8853083d: getting local image 94ea1644-a329-4e45-9cb6-f5a5fc102f87 for service storage
[2019-11-28T11:38:15.133Z] DEBUG: manta-adm/7275 on 0cf080f3-51de-4103-a13b-e55a8853083d: getting local image 29bcf204-59c7-4292-a1e8-ef3e10d5ff52 for service webapi
[2019-11-28T11:38:15.144Z] DEBUG: manta-adm/7275 on 0cf080f3-51de-4103-a13b-e55a8853083d: missing local image 29bcf204-59c7-4292-a1e8-ef3e10d5ff52 (webapi)
[2019-11-28T11:38:15.144Z] DEBUG: manta-adm/7275 on 0cf080f3-51de-4103-a13b-e55a8853083d: not searching remote imgapi for 94ea1644-a329-4e45-9cb6-f5a5fc102f87, it exists locally
[2019-11-28T11:38:15.145Z] DEBUG: manta-adm/7275 on 0cf080f3-51de-4103-a13b-e55a8853083d: getting remote image 29bcf204-59c7-4292-a1e8-ef3e10d5ff52 for service webapi on channel null
[2019-11-28T11:38:17.332Z] DEBUG: manta-adm/7275 on 0cf080f3-51de-4103-a13b-e55a8853083d: missing remote image 29bcf204-59c7-4292-a1e8-ef3e10d5ff52 (webapi)
There was a problem hiding this comment.
From discussion offline earlier, Trent felt that the option to skip the remote lookup altogether was too much complexity.
So now, we'll only do remote lookups if the local image isn't present, and in that case, we're already in an error scenario (no local image)
If we happen to also fail to do the remote lookup (eg. due to an air-gapped network) then there's still no chance of us deploying a badly-named uuid as a result of an image name we couldn't lookup, so the system is still correct.
Yeah - we needed to write that code anyway in order to implement the name-check, so it came for free really. |
docs/man/man1/manta-adm.md
Outdated
| @@ -20,7 +20,7 @@ manta-adm - administer a Manta deployment | |||
|
|
|||
| `manta-adm show [-l LOG_FILE] [-js] SERVICE` | |||
|
|
|||
| `manta-adm update [-l LOG_FILE] [-n] [-y] [--no-reprovision] [--skip-verify-channel] FILE [SERVICE]` | |||
| `manta-adm update [-l LOG_FILE] [-n] [-y] [--no-reprovision] [--skip-verify-remote] [--skip-verify-channel] FILE [SERVICE]` | |||
There was a problem hiding this comment.
| `manta-adm update [-l LOG_FILE] [-n] [-y] [--no-reprovision] [--skip-verify-remote] [--skip-verify-channel] FILE [SERVICE]` | |
| `manta-adm update [-l LOG_FILE] [-n] [-y] [--no-reprovision] [--skip-verify-channel] FILE [SERVICE]` |
| imageUuid, opts.services_by_image[imageUuid]); | ||
| opts.local_imgapi.getImage(imageUuid, imgapi_opts, | ||
| function getImage(err, image) { | ||
| if (err) { |
There was a problem hiding this comment.
This should probably differentiate from a 404 (ResourceNotFound) an some other response error. Here is comparable code in sdcadm that does that:
self.imgapi.getImage(imgUuid, function (err, img) {
if (!err) {
ctx.imgFromUuid[imgUuid] = img;
nextImg();
} else if (err.restCode !== 'ResourceNotFound') {
nextImg(new errors.SDCClientError(
err, 'imgapi'));
} else {
nextImg();
}
});
There was a problem hiding this comment.
Thanks, added that in the latest commit.
lib/adm.js
Outdated
| services_by_image: services_by_image, | ||
| channel: self.ma_channel, | ||
| skip_verify_channel: opts.skip_verify_channel, | ||
| skip_verify_remote: opts.skip_verify_remote |
This adds a guard that checks that the services.js image name mapping for a service matches the image.name of the Image to be provisioned.
While we were in the area, we added a check that the Image exists in the local imgapi (rather than eventually blowing up later during provisioning if the image is missing) and allow a --skip-verify-remote flag for Triton instances that are not connected to a network capable of reaching https://updates.joyent.com