1480112 - AMI creation successful even when there are DSC failures

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: OpenCloudConfig, task)

Description

[Pete Moore [:pmoore][:pete]]

This deployment was successful, and an AMI was created and copied between regions, and the worker type definition was updated to use the new AMIs:

https://tools.taskcluster.net/groups/MhbYjmMLRPeBDQVq201rXw/tasks/c2Xn3VSUT3OpUNhoatpQpQ/runs/0/logs/public%2Flogs%2Flive.log

However, one of the steps failed:

https://papertrailapp.com/systems/2145621272/events?focus=961298798150340624&selected=961298798150340624

VERBOSE: [I-03ED1FCC50A54]: LCM:  [ Start  Resource ]  [[Script]FileDownload_NSSMDownload] 
VERBOSE: [I-03ED1FCC50A54]: LCM:  [ Start  Test     ]  [[Script]FileDownload_NSSMDownload] 
VERBOSE: [I-03ED1FCC50A54]:                            [[Script]FileDownload_NSSMDownload] Validate-PathsExistOrNotRequested :: 1 validation specified. 
VERBOSE: [I-03ED1FCC50A54]:                            [[Script]FileDownload_NSSMDownload] Path absent: C:\Windows\Temp\NSSMInstall.zip 
VERBOSE: [I-03ED1FCC50A54]:                            [[Script]FileDownload_NSSMDownload] Validations not satisfied 
VERBOSE: [I-03ED1FCC50A54]: LCM:  [ End    Test     ]  [[Script]FileDownload_NSSMDownload]  in 0.0070 seconds. 
VERBOSE: [I-03ED1FCC50A54]: LCM:  [ Start  Set      ]  [[Script]FileDownload_NSSMDownload] 
VERBOSE: [I-03ED1FCC50A54]:                            [[Script]FileDownload_NSSMDownload] Performing the operation "Set-TargetResource" on target "Executing the SetScript with the user supplied credential". 
VERBOSE: [I-03ED1FCC50A54]:                            [[Script]FileDownload_NSSMDownload] GET https://nssm.cc/ci/nssm-2.24-103-gdee49fc.zip with 0-byte payload 
VERBOSE: [I-03ED1FCC50A54]: LCM:  [ End    Set      ]  [[Script]FileDownload_NSSMDownload]  in 0.6160 seconds. 
PowerShell DSC resource MSFT_ScriptResource  failed to execute Set-TargetResource functionality with error message: The  
remote server returned an error: (503) Server Unavailable.  
    + CategoryInfo          : InvalidOperation: (:) [], CimException 
    + FullyQualifiedErrorId : ProviderOperationExecutionFailure 
    + PSComputerName        : localhost 
sshd: Server listening on :: port 22. 
sshd: Server listening on 0.0.0.0 port 22. 
PowerShell DSC resource MSFT_ScriptResource  failed to execute 
Set-TargetResource functionality with error message: The remote server 
returned an error: (503) Server Unavailable. 
    + CategoryInfo          : InvalidOperation: (:) [], CimException 
    + FullyQualifiedErrorId : ProviderOperationExecutionFailure 
    + PSComputerName        : localhost 


Since nssm couldn't be downloaded, the AMI rollout and worker type update should have been halted, rather than continuing, and the worker type getting updated.

The root cause was that the hosting for the NSSM zip file was down, which I have attempted to resolve in https://github.com/mozilla-releng/OpenCloudConfig/commit/6e1fdc4f4184116128bdc8e7bc64db8d4b528bb6 by publishing the binary to tooltool. If the staging rollout works with this change, I'll create a PR to get this landed for our other worker types.

Comment 1

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Attachment: https://github.com/mozilla-releng/OpenCloudConfig/pull/165

Comment 2

[Rob Thijssen [:grenade (EET/UTC+0300)]]

we don't currently have a good way to communicate

Comment 3

[Rob Thijssen [:grenade (EET/UTC+0300)]]

copy paste failure above. should have read:

if a dsc run fails (indicated by the presence of "failed to execute Set-TargetResource" in the dsc transcript), the logic in the patch will terminate the instance if it is a worker and cause it to go idle if it is the ami creation instance.

if the dsc failure occurs on the ami creation instance, we don't have a way to communicate with the taskcluster-github job to tell it that the dsc run has failed. the best we can do is sleep until the taskcluster-github job fails, because of a task timeout.

Comment 4

[Rob Thijssen [:grenade (EET/UTC+0300)]]

i rethought the logic and modified the patch to only handle failures on the ami creation instances. since this bug is about not deploying amis when something in dsc has failed, i thought it better to only deal with that specific issue here.

Comment 5

[Pete Moore [:pmoore][:pete]]

Thanks Rob!

(In reply to Rob Thijssen (:grenade UTC+2) from comment #3)

> if the dsc failure occurs on the ami creation instance, we don't have a way
> to communicate with the taskcluster-github job to tell it that the dsc run
> has failed. the best we can do is sleep until the taskcluster-github job
> fails, because of a task timeout.

John, can you think of any options here? The issue we have is that a taskcluster task triggers the creation of an instance with OCC installed, waits for it to shutdown, and then creates an AMI of it. It assumes that once the machine has shutdown, everything was successful, and it can go ahead and create the AMI.

We need a way to signal to the taskcluster task whether the OCC run was successful or not.

The output from OCC is logged to papertrail, but the taskcluster task doesn't have access to the papertrail logs. So the option of writing a "special" message to the logs is harder, because then the taskcluster task would need read access to the papertrail logs.

Another option could be that the instance self-terminates (calling the ec2 terminate-instances API) on failure - but that requires that the instances have the permission to call this ec2 endpoint.

In future this is probably solved with https://github.com/taskcluster/taskcluster-rfcs/issues/122 but I'm not sure what to do until then. I'm curious if you have an elegant solution up your sleeve!

Comment 6

[John Ford [:jhford] CET/CEST Berlin Time]

I'm not too sure about how OCC works internally, but as I understand, a graph of dependencies is built and then passed to DSC to execute.  What if we had a single task at the end of the graph which depended on all other tasks that wrote out a flag somewhere to signify whether the OCC run succeeded.  That task could write out a file (e.g. C:\occ-success) and use an IAM Instance Profile to set a tag on the instance which is running the OCC process (e.g. OccSuccessTime: "Fri Aug  3 15:20:44 CEST 2018").

The file would make it possible for whatever invokes the worker to check for a completed OCC run.  The tag would make it possible for whatever does the build process for images to see status from outside of the instances after it shuts itself down.

https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#ec2-instance-profile

Comment 7

[Pete Moore [:pmoore][:pete]]

I like the idea. I think though, that probably enabling the worker to terminate itself might expose a smaller attack surface than allowing the worker to set an instance tag, which if in the wrong hands, could possibly be used to falsify other tags.

I think if we leak the ability to terminate an instance, at worst that could cause delays and retries, but shouldn't allow a malicious actor to affect any task artifacts that might make their way into a release etc. At worst it should only expose the ability to bring down instances which should just cause delays/retries, or failed OCC jobs etc.

So I think my preference at the moment is that we grant instances spawned by OCC the ability to terminate themselves, and use this to signify an unsuccessful OCC run. I believe that the task that generates the AMI would automatically fail if the instance it was waiting to snapshot was terminated.

What do you think about this Rob?

Comment 8

[Pete Moore [:pmoore][:pete]]

(an advantage of John's approach, on the other hand, is it is possible to share more state about the cause of the failure using a tag - however, the task log already has a link to papertrail logs, so that same information could be logged in the papertrail logs, it is just one more step of indirection)

Comment 9

[Rob Thijssen [:grenade (EET/UTC+0300)]]

i think that implementation proposed in the patch above (wait for the task to time out) provides the simplest solution.

- no new credentials or iam roles need to be managed.
- no new area of vulnerability is introduced
- the only downsides are:
 - the error message from the failed task says that the task timed out instead of that dsc failed.
 - the task takes longer to fail than an explicit failure triggered by the dsc failure would

any of the other solutions would still require the user to examine the papertrail logs to determine the cause of the error. so i don't see how exposing some new vulnerability by passing in credentials or applying an iam role actually reduces the debug effort of determining what caused the problem. i think they're interesting solutions that do not very much to improve the end result of making it easy to determine why the task failed. they simply make it quicker for someone who isn't monitoring the logs to know that they need to take a look at the logs.

you would get a task failure quicker using either of the alternative solutions but i don't see how that makes it worthwhile to add a new attack surface. someone who is monitoring the pt logs during deployment would know about the error as quickly as an earlier task failure would enable.

when you also consider that we're trying to phase out occ in favour of a new solution, i really don't see the point of adding extra complexity to occ at this time. i think it could be prudent to employ simple solutions like the one proposed in the patch rather than spending time trying to determine the purest, most elegant, all encompassing solution for a piece of software that we intend to replace as soon as possible.

Comment 10

[Pete Moore [:pmoore][:pete]]

(In reply to Rob Thijssen (:grenade UTC+2) from comment #9)
> i think that implementation proposed in the patch above (wait for the task
> to time out) provides the simplest solution.
> 
> - no new credentials or iam roles need to be managed.
> - no new area of vulnerability is introduced
> - the only downsides are:
>  - the error message from the failed task says that the task timed out
> instead of that dsc failed.
>  - the task takes longer to fail than an explicit failure triggered by the
> dsc failure would
> 
> any of the other solutions would still require the user to examine the
> papertrail logs to determine the cause of the error. so i don't see how
> exposing some new vulnerability by passing in credentials or applying an iam
> role actually reduces the debug effort of determining what caused the
> problem. i think they're interesting solutions that do not very much to
> improve the end result of making it easy to determine why the task failed.
> they simply make it quicker for someone who isn't monitoring the logs to
> know that they need to take a look at the logs.
> 
> you would get a task failure quicker using either of the alternative
> solutions but i don't see how that makes it worthwhile to add a new attack
> surface. someone who is monitoring the pt logs during deployment would know
> about the error as quickly as an earlier task failure would enable.
> 
> when you also consider that we're trying to phase out occ in favour of a new
> solution, i really don't see the point of adding extra complexity to occ at
> this time. i think it could be prudent to employ simple solutions like the
> one proposed in the patch rather than spending time trying to determine the
> purest, most elegant, all encompassing solution for a piece of software that
> we intend to replace as soon as possible.

I disagree. We need to support OCC while we wait for the replacement, and every hack that is added makes it less maintainable and harder to reason about. Moving review to avoid deadlocking.

Comment 11

[Rob Thijssen [:grenade (EET/UTC+0300)]]

(In reply to Pete Moore [:pmoore][:pete] from comment #10)

> We need to support OCC while we wait for the replacement
we continue to support occ. the patch here is one of many that do so

> every hack that is added makes it less maintainable and harder to reason
> about.
i'm not sure how i feel about this patch being called a hack. it does the best it can with the functionality that is available. also the reasoning behind it is documented well both here and in the inline comments in code

> Moving review to avoid deadlocking.
thanks!

Comment 12

[Rob Thijssen [:grenade (EET/UTC+0300)]]

this hasn't moved. we failed to decide between two options:

- modify the github task ci configuration so that an ami creation instance has ec2 credentials in an iam role that allows it to terminate instances. the iam role configuration to do this and the logic to write that role is complicated. see: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_examples_ec2_terminate-ip.html additionally, the github-task worker would need ec2 permissions itself in order to grant that role. in my view, this adds two layers of complexity and a new attack surface (the permissions granted to the github task worker to create and assign ec2 roles) for very little gain (a better failure message about why the ami creation task failed).

- use the patch i wrote already to simply allow the task to time out and fail

i will close as wontfix since i'm not prepared to add the level of complexity and vulnerability introduced by the first option and i failed to get a review or consensus on the second option.

Comment 13

[Kendall Libby [:fubar] (he/him)]

Rob, if you can test this on a beta worker with a purposefully broken DSC config to show that it works as expected I'll r+ your patch. We need to block AMI generation if DSC has issues and this is quick, easy, and no worse than the existing process for determining what is the problem.

Comment 14

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Comment on attachment 8996755 [details] [review]
https://github.com/mozilla-releng/OpenCloudConfig/pull/165

cheers. tested and had an issue on the beta run where the sleep didn't last as long as it needed to. so i modified the code to loop through 10 minute sleeps and send output to the papertrail logs every 10 minutes which worked a lot better. i think there was something going on with the output buffer needing to be cleared in order for the occ powershell task to stay alive.

this is what the papertrail output now looks like when there's dsc bustage:

> Sep 06 13:40:31 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: VERBOSE: [I-02CD8EBCBB0F1]:                            [[Script]CommandRun_IntentionalBustage] Validate-PathsExistOrNotRequested :: 1 validation specified. 
> Sep 06 13:40:31 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: VERBOSE: [I-02CD8EBCBB0F1]:                            [[Script]CommandRun_IntentionalBustage] Path absent: C:\path\does\not\exist 
> Sep 06 13:40:31 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: VERBOSE: [I-02CD8EBCBB0F1]:                            [[Script]CommandRun_IntentionalBustage] Validations not satisfied 
> Sep 06 13:40:32 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: VERBOSE: [I-02CD8EBCBB0F1]: LCM:  [ End    Test     ]  [[Script]CommandRun_IntentionalBustage]  in 0.0070 seconds. 
> Sep 06 13:40:32 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: VERBOSE: [I-02CD8EBCBB0F1]: LCM:  [ Start  Set      ]  [[Script]CommandRun_IntentionalBustage] 
> Sep 06 13:40:32 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: VERBOSE: [I-02CD8EBCBB0F1]:                            [[Script]CommandRun_IntentionalBustage] Performing the operation "Set-TargetResource" on target "Executing the SetScript with the user supplied credential". 
> Sep 06 13:40:32 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: VERBOSE: [I-02CD8EBCBB0F1]: LCM:  [ End    Set      ]  [[Script]CommandRun_IntentionalBustage]  in 0.2730 seconds. 
> Sep 06 13:40:32 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: PowerShell DSC resource MSFT_ScriptResource  failed to execute Set-TargetResource functionality with error message:  
> Sep 06 13:40:32 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com dsc-run: This command cannot be run due to the error: The system cannot find the file specified.  
...
> Sep 06 13:40:31 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com OpenCloudConfig: dsc run failed. 
...
> Sep 06 13:40:31 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com OpenCloudConfig: waiting for occ ci task to fail due to timeout. shutdown in 300 minutes. 
...
> Sep 06 13:50:31 i-02cd8ebcbb0f10109.gecko-t-win10-64-beta.usw2.mozilla.com OpenCloudConfig: waiting for occ ci task to fail due to timeout. shutdown in 290 minutes. 

the ci task fails about 3 hours in and the instance shuts itself down shortly after that. ami creation & deployment is aborted as expected.

Comment 15

[Kendall Libby [:fubar] (he/him)]

Nice!