# February 5th, 2019 Partial Prow Outage

Created By: [Katharine Berry]  
Last Modified: 2019-02-05

## Summary

The main Kubernetes production Prow cluster, [prow.k8s.io], became unable to correctly
execute jobs due to a change that resulted in the pod namespace being ignored. Additionally,
had jobs been running, many PRs would have been unable to merge due to outdated GitHub branch
protection settings. This resulted in a 47 minute period during which nothing could merge.

## Impact

Most jobs started by prow failed to complete initialisation, so the jobs appeared to be hung.
Additionally, prow did not report statuses for optional jobs, but the GitHub status continued to
expect them. The outage lasted for 47 minutes, during which time 415 jobs were unable to start.

Both issues was resolved after 47 minutes, and all stuck jobs were restarted correctly. Jobs
with missing statuses did not have the statuses applied, and so will be unable to resolve until the
PRs are corrected on a case-by-case basis.

## Root Cause

After Prow was updated, plank began creating pods in the `default` namespace of the cluster,
rather than the expected `test-pods` namespace. This caused almost all pods to get stuck trying
to start as they attempted to access secrets that were not present in the `default` namespace.
This resulted in all nontrivial jobs failing.

The failure to create pods in the correct namespace was caused by [a typo] introduced during a
recent refactor of the kubernetes clients, causing it to read the `ProwJobNamespace` instead of the
`PodNamespace`. Prior deployments of the same change set these to the same value, so the error
wasn't noticed.

Separately, another change caused skipped jobs to no longer have a status reported indicating that
they had been skipped. A change to `branchprotector` was made to update the GitHub branch
protection configuration to match. While an updated `branchprotector` was deployed and run in
advance, deployment automation that runs on all commits reverted the update and an older version
later ran and reverted the changes. This caused PRs updated during the outage to be unable to be
merged even after the outage ended.

## Detection

The `branchprotector` issue was detected within ten minutes of initial rollout due to a second PR to
complete the rollout getting wedged. The pod namespace issue was detected while attempting to roll
back, because the tests on the rollback PR were apparently stuck, and the deploy job itself was
wedged. 

## Resolution

Resolution was initially attempted by creating a revert PR for the original deployment PR. This PR
could not be automatically merged due to incorrect branch protection settings, so it was merged
manually. Prow was then unable to deploy itself, because the deployment job was in the
wrong namespace and so stuck trying to mount missing secrets. Prow was manually deployed
using `prow/deploy.sh`, and service was resumed shortly thereafter.

The 415 spurious pods created during the outage were later manually cleaned up using
`kubectl delete pods -l created-by-prow=true`.

## Lessons learned

### What went well

- Most of prow continued to function while job execution was unavailable
- Stuck jobs were automatically rerun after the revert successfully deployed.

### What went poorly

- Breaking something that approves and then deploys itself makes rolling back trickier.
- The strong ordering requirement (branchprotector must be deployed at least four hours before anything else)
  added complexity to the rollout, which failed.
- PRs that are missing "optional" statuses did not automatically recover after the outage and must
  manually kicked to recover them.
- Merging multiple major changes simultaneously is less than ideal.

### Where we got lucky

- Apparently nobody was watching their PRs very closely around 9am PST, so nobody noticed.
- It appears that very few PRs have missing statuses

## Action items

- Before retrying, ensure the updated branchprotector run is not reverted: [#11134]

## Timeline

All times in PST, on February 5th 2019

* [08:47] Prow's auto-generated bump PR is merged, triggering a deploy. **Outage begins.**
* [08:57] @Katharine notices that some GitHub statuses that should've been removed earlier are still 
present and thereby blocking any PR that omits a conditionally triggered status
* [09:02] @Katharine creates a PR to roll back the change pending investigation
* [09:12] @Katharine manually merges the rollback PR, which is itself blocked on missing status
contexts.
* [09:19] @Katharine begins suspecting that prow is unable to deploy itself for some reason
* [09:27] @Katharine determines that the deploy job was created with the wrong namespace
* [09:29] @Katharine manually rolls back prow.
* [09:34] Prow is confirmed to be functional. **Outage ends.**
* [10:01] @Katharine verifies that all pods created during the outage are in the wrong namespace
* [10:33] @stevekuznetsov finds and fixes the typo causing prow to use the wrong namespace
* [10:45] @Katharine deletes all the stray pods that are still trying to start.
* [14:31] @Katharine determines why an older branchprotector reverted the required branch protection
changes.

## Appendix 

### #testing-ops chat logs

Edited for clarity to remove discussion of an unrelated openshift prow outage

---

>Katharine [8:27 AM]  
It's prow bump time!  
For real, probably! Unlike the last several times.  
>
>stevekuznetsov [8:33 AM]  
dope  
let's do it  
>
>Katharine [8:57 AM]  
hmmmmmm  
not sure the branchprotector run actually had the desired effect.  
ugh, this is not right  
rolling back
>
>bentheelder [9:03 AM]  
shipit: :ship_it_parrot:  
rollback: :rollback_parrot: (edited) 
>
>stevekuznetsov [9:04 AM]  
lol  
what happened?
>
>Katharine [9:05 AM]  
Unclear  
>
>stevekuznetsov [9:05 AM]  
did another BP run happen between your manual one and right now?  
what caused you to panic?  
tide logs?
>
>Katharine [9:05 AM]  
I updated the cronjob too so it shouldn't have mattered  
¯\\\_(ツ)\_/¯
>
>stevekuznetsov [9:05 AM]  
ok, true  
ok  
rope me in as much as you want on the debugging  
happy to jump into a hangouts session  
just lmk
>
>Katharine [9:06 AM]  
It's possible I'm just confused by what the expected statuses are, but I'm guessing that we shouldn't be seeing "required" statuses that are never reported.  
(and which are not always run)
>
>stevekuznetsov [9:07 AM]  
YUes
>
>bentheelder [9:07 AM]  
actually you can have required for `run_if_changed` (?)
>
>stevekuznetsov [9:07 AM]  
If the job posts a status conditionally we should not see a required status for it  
No, not anymore  
`run_if_changed` will be posted _if_ it runs
>
>Katharine [9:07 AM]  
That is what made me panic
>
>stevekuznetsov [9:07 AM]  
You should not see required statuses in branch protection for jobs that are `run_if_changed` (or not `always_run` in general)
>
>bentheelder [9:07 AM]  
wait, so how do you require a job that is not always_run? previously you could require a job to be (run && passed) || skipped (edited) 
>
>stevekuznetsov [9:07 AM]  
Right, so there are no longer any skipped statuses  
>
>prow APP [9:12 AM]  
*Warning:* Katharine (@Katharine) manually merged 2 commit(s) into master: https://github.com/kubernetes/test-infra/compare/8c1cd8046656...4956da7c3699
>
>bentheelder [9:14 AM]  
(prow was stuck, rollback of the bump)
>
>Katharine [9:19 AM]  
I'm not convinced that prow is actually in a sufficiently healthy state to roll itself back.
>
>bentheelder [9:19 AM]  
we can manually run the yamls
>
>Katharine [9:19 AM]  
Suspect we will have to  
Trying to verify first
>
>Katharine [9:27 AM]  
further update: it looks like pods are landing in the wrong namespace  
which is why things are extremely broken now
>
>stevekuznetsov [9:27 AM]  
... why?
>
>Katharine [9:28 AM]  
Unclear  
but nothing can mount anything and is consequently getting stuck (edited)   
because they're in the wrong namespaces
>
>bentheelder [9:28 AM]  
I'd guess the client code change didn't go smooth for our instance.
>
>stevekuznetsov [9:29 AM]  
do you set PodNamespace or ProwJobNamespace in your config?  
are you seeing pods in the build cluster in the wrong namespace  
or prow infra service pods?
>
>Katharine [9:30 AM]  
prowjob pods
>
>stevekuznetsov [9:31 AM]  
pods created by `plank` to target the build cluster  
and fulfill `prowjobs`?
>
>Katharine [9:31 AM]  
At least the trusted ones, which was my immediate concern because it is why it can't roll itself back
>
>stevekuznetsov [9:31 AM]  
those are not on the build cluster  
but on the local cluster?
>
>Katharine [9:31 AM]  
Yes  
In their own namespace
>
>stevekuznetsov [9:31 AM]  
can you paste plank logs from your deploymetn  
at the start?
>
>Katharine [9:32 AM]  
but they are instead ending up in the same one as the service pods
>
>stevekuznetsov [9:32 AM]  
hrm
>
>Katharine [9:32 AM]  
gimme a bit  
trying to get it back up first
>
>stevekuznetsov [9:32 AM]  
ok  
i will look at the code
>
>Katharine [9:34 AM]  
okay, manual rollback worked
>
>stevekuznetsov [9:35 AM]  
how were you specifying which namespace would go with the trusted namespace?  
it's not clear to me how you schedule pods explicitly to something other than `config.PodNamespace`
>
>Katharine [9:37 AM]  
I think we're going to have to do something to recover all the jobs that got wedged. Or just tell people to retest I guess.
>
>Katharine [9:39 AM]  
the stuck jobs are recovering on their own, so that's good.
>
>stevekuznetsov [9:41 AM]  
yeah as long as the ProwJob record exists  
`plank` will recover it
>
>Katharine [9:41 AM]  
I thought so, but they were being slow and I was mildly concerned
>
>stevekuznetsov [9:41 AM]  
ack
>
>Katharine [10:01 AM]  
verified: for both the trusted cluster and the build cluster, all the plank-created pods got dropped in the `default` namespace instead of `test-pods`  
(they end up in the correct cluster though)
>
>stevekuznetsov [10:31 AM]  
what is your PodNamespace in the config?
>
>Katharine [10:32 AM]  
`test-pods`: https://github.com/kubernetes/test-infra/blob/master/prow/config.yaml#L41
>
>stevekuznetsov [10:32 AM]  
very interesting  
oh  
lmao  
:facepalm:  
https://github.com/kubernetes/test-infra/pull/11130  
now  
that will schedule to `test-pods`  
but  
did you have any clusters with more than one NS?  
or is it always `test-pods` across all clusters  
>
>Katharine [10:35 AM]  
I'm unsure. To my knowledge it's just `test-pods`, but I may be wrong.
>
>stevekuznetsov [10:36 AM]  
then that PR should fix you
>
>Katharine [11:09 AM]  
I am still attempting to figure out why the branchprotector failed to drop required statuses
>
>Katharine [11:17 AM]  
upon investigation, it looks like an old branchprotector ended up running again, though I'm not clear how or why.
>
>Katharine [11:27 AM]  
I've now re-updated the cronjob, suspended it, manually invoked branchprotector, and verified the correct version is running. Assuming it does something reasonable, we can try this exciting adventure again at 3pm.  
(which is roughly when it will finish)


[Katharine Berry]: https://github.com/Katharine
[a typo]: https://github.com/stevekuznetsov/test-infra/commit/d6078ec1b6ac6b79abaf7b1491a386ffb6cc93c2#diff-c3c219515b589f861f234b9c4074b769R122
[prow.k8s.io]: https://prow.k8s.io
[#11134]: https://github.com/kubernetes/test-infra/pull/11134
