Chain Of Trust can't find a worker_impl for decision task of Fennec graph 1

RESOLVED FIXED

Status

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jlorenzo, Assigned: jlorenzo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Another fallout of bug 1259627 (which was hidden by bug 1396472).

Now that group Id of graph one matches the decision task, Chain Of Trust has some trouble to validate the origin of it. In my opinion, the fix is to split Fennec graph 1 and 2, so that graph 2 doesn't know anything about graph 1. Then, the only decision task that Chain of Trust will know is one that is made in-tree.

Error gotten:

> 2017-09-04T08:21:32 CRITICAL - Chain of Trust verification error!
> Traceback (most recent call last):
>   File "/builds/scriptworker/lib/python3.5/site-packages/scriptworker/cot/verify.py", line 1364, in verify_chain_of_trust
>     await build_task_dependencies(chain, chain.task, chain.name, chain.task_id)
>   File "/builds/scriptworker/lib/python3.5/site-packages/scriptworker/cot/verify.py", line 588, in build_task_dependencies
>     await build_task_dependencies(chain, task_defn, task_name, task_id)
>   File "/builds/scriptworker/lib/python3.5/site-packages/scriptworker/cot/verify.py", line 582, in build_task_dependencies
>     link.task = task_defn
>   File "/builds/scriptworker/lib/python3.5/site-packages/scriptworker/cot/verify.py", line 210, in task
>     self.worker_impl = guess_worker_impl(self)
>   File "/builds/scriptworker/lib/python3.5/site-packages/scriptworker/cot/verify.py", line 309, in guess_worker_impl
>     raise_on_errors(errors)
>   File "/builds/scriptworker/lib/python3.5/site-packages/scriptworker/cot/verify.py", line 260, in raise_on_errors
>     raise CoTError("\n".join(errors))
> scriptworker.exceptions.CoTError: "guess_worker_impl: can't find a worker_impl for signing:decision:decision!\n{'created': '2017-09-04T07:16:49.317Z', 'retries': 5, 'dependencies': [], 'schedulerId': '-', 'workerType': 'human-decision', 'payload': {'description': 'required'}, 'requires': 'all-completed', 'taskGroupId': 'fOOcKf2qQlixNT1ctM39zQ', 'deadline': '2017-09-08T07:16:49.317Z', 'metadata': {'description': 'Release Promotion atomic submission task', 'source': 'https://github.com/mozilla/releasetasks', 'name': 'fennec mozilla-beta atomic submission task', 'owner': 'release@mozilla.com'}, 'routes': ['index.releases.v1.mozilla-beta.da291ce74248b2830a0010a8bf21bba5f408bd43.fennec.56_0b9.build2.atomic_submission', 'index.releases.v1.mozilla-beta.latest.fennec.latest.atomic_submission'], 'priority': 'high', 'extra': {'notifications': {'task-completed': {'message': 'fennec mozilla-beta atomic submission task has completed successfully! Yay!', 'ids': ['releasetasks'], 'subject': 'Completed: fennec mozilla-beta atomic submission task'}, 'task-exception': {'message': 'Uh-oh! fennec mozilla-beta atomic submission task resulted in an exception.', 'ids': ['releasetasks'], 'subject': 'Exception: fennec mozilla-beta atomic submission task'}, 'task-failed': {'message': 'Uh-oh! fennec mozilla-beta atomic submission task failed.', 'ids': ['releasetasks'], 'subject': 'Failed: fennec mozilla-beta atomic submission task'}}, 'build_props': {'product': 'fennec', 'mozharness_changeset': 'da291ce74248b2830a0010a8bf21bba5f408bd43', 'platform': None, 'version': '56.0b9', 'release_eta': None, 'revision': 'da291ce74248b2830a0010a8bf21bba5f408bd43', 'branch': 'mozilla-beta', 'build_number': 2, 'locales': [None]}, 'signing': {'signature': 'eyJhbGciOiJSUzUxMiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE1MDQ1MDk0MDksInZlcnNpb24iOiIxIiwidGFza0lkIjoiT3FFdUZhVE5TV3F6SnBTQlRXSjd5ZyIsImV4cCI6MTUwNDg1NTAwOX0.YZY26q03Cfk9tFk2eiD0VLWd104VD-wGOXDrr0Cvtc70uh9Ss864sc-w36JgYh4DJ3ii3CRoLNawOkAr94BbQMAQZ0qAi0pbIDZfli1He9dJs1dd8m-rPibVOD_BvClmf_gnzo4rbsVkqmbOaza2201ylXIITDhTryaDhaRHJC3QJxHU-yEPO-FXt0v4H_JGAIwYI2IkT1ZPT4veDMhGHWKLXJktR0pTuNWM2U8wFJSsuTKitJG68vvJtBTI6NcoLzes-jqsLcblqqMwXGmYVfpYIJGEw0TvcguoUg4OWIhGEGZTpFUwBdZx-xKmhOQILmSjrK3H9P6igLeQ95Xt1g'}}, 'scopes': [], 'expires': '3017-09-04T07:16:49.317Z', 'provisionerId': 'null-provisioner', 'tags': {}}"


https://public-artifacts.taskcluster.net/d68nHtNzTyCHXsncxypcOQ/0/public/logs/chain_of_trust.log
(Assignee)

Updated

2 years ago
Summary: Chain Of Trust can't can't find a worker_impl for decision task of Fennec graph 1 → Chain Of Trust can't find a worker_impl for decision task of Fennec graph 1
Comment hidden (mozreview-request)
(Assignee)

Comment 2

2 years ago
Changes are tested on this staging release: https://tools.taskcluster.net/groups/fg7RGfy9S2Kvav0W5OwZbA. As you can see, the decision task doesn't depend on graph 1 anymore. The signing tasks (not yet executed) will only check this graph, which should work like a regular nightly.

On IRC, Aki said we should change scriptworker instead. However, change scriptworker may take longer. Hence, here are the hotfixes. 

What do you think, Mihai?
Attachment #8904232 - Flags: review?(mtabara)
Comment hidden (mozreview-request)

Comment 4

2 years ago
mozreview-review
Comment on attachment 8904231 [details]
Bug 1396517 - Make Fennec graph 2 not reference graph 1

https://reviewboard.mozilla.org/r/176012/#review181030

::: buildfarm/release/release-runner.py:528
(Diff revision 1)
>                  "update_verify_requires_cdn_push": branchConfig.get("update_verify_requires_cdn_push", False),
>              }
>  
>              # TODO: en-US validation for multiple tasks
>              # validate_graph_kwargs(queue, gpg_key_path, **kwargs)
> -            task_group_id, toplevel_task_id, tasks = make_task_graph_strict_kwargs(**kwargs)
> +            task_group_id, tasks = make_task_graph_strict_kwargs(**kwargs)

Hm, question: why don't we just s,task_grup_id,decision_task_id everywhere here? It seems a bit confusing to have two variables pointing to the same value.

::: buildfarm/release/release-runner.py:528
(Diff revision 1)
>                  "update_verify_requires_cdn_push": branchConfig.get("update_verify_requires_cdn_push", False),
>              }
>  
>              # TODO: en-US validation for multiple tasks
>              # validate_graph_kwargs(queue, gpg_key_path, **kwargs)
> -            task_group_id, toplevel_task_id, tasks = make_task_graph_strict_kwargs(**kwargs)
> +            task_group_id, tasks = make_task_graph_strict_kwargs(**kwargs)

Are you sure we're not getting `ValueError: too many values to unpack` for not catching that third argument which is supposed to be None? via https://github.com/mozilla-releng/releasetasks/pull/274/files#diff-c818ca8b6195fd3f9c79324d373bd08dL106

::: buildfarm/release/release-runner.py:543
(Diff revision 1)
>  
>              rr.mark_as_completed(release)
>              l10n_url = rr.release_l10n_api.getL10nFullUrl(release['name'])
>              email_release_drivers(smtp_server=smtp_server, from_=notify_from,
>                                    to=notify_to, release=release,
>                                    task_group_id=task_group_id, l10n_url=l10n_url)

If we do the change above, we could s,task_group_id,decision_task_id here too.

::: buildfarm/release/releasetasks_graph_gen.py:124
(Diff revision 1)
>          "update_verify_channel": release_config["update_verify_channel"],
>          "update_verify_requires_cdn_push": release_config["update_verify_requires_cdn_push"],
>          "release_eta": release_config.get("release_eta"),
>      }
>  
> -    task_group_id, toplevel_task_id, tasks = make_task_graph_strict_kwargs(**kwargs)
> +    task_group_id, tasks, second_task_group_id = make_task_graph_strict_kwargs(**kwargs)

I know this might be tricky since we have basically two decision task ids here, but I think we should try to be consistent with the changes from release-runner.py, even if this is temporary. 

Nit: (just a prefernce matter)I would rename `the task_group_id` as `decision_task_id` and then `second_decision_task_id`. And s,old,new everywhere in the function.

::: buildfarm/release/releasetasks_graph_gen.py:125
(Diff revision 1)
>          "update_verify_requires_cdn_push": release_config["update_verify_requires_cdn_push"],
>          "release_eta": release_config.get("release_eta"),
>      }
>  
> -    task_group_id, toplevel_task_id, tasks = make_task_graph_strict_kwargs(**kwargs)
> +    task_group_id, tasks, second_task_group_id = make_task_graph_strict_kwargs(**kwargs)
> +    toplevel_task_id = task_group_id

I think having two variables pointing to the same value adds confusion here, even if the change is temporary. We should probably be consistent with changes from release-runner.py file and have `decision_task_id` and `second_decision_task_id` and s,old,new everywhere in the file(s).
Attachment #8904232 - Attachment is patch: true
Attachment #8904232 - Attachment mime type: text/x-github-pull-request → text/plain
Attachment #8904232 - Flags: review?(mtabara) → review+

Comment 5

2 years ago
mozreview-review
Comment on attachment 8904231 [details]
Bug 1396517 - Make Fennec graph 2 not reference graph 1

https://reviewboard.mozilla.org/r/176012/#review181036
Attachment #8904231 - Flags: review?(mtabara) → review+
(Assignee)

Comment 6

2 years ago
mozreview-review-reply
Comment on attachment 8904231 [details]
Bug 1396517 - Make Fennec graph 2 not reference graph 1

https://reviewboard.mozilla.org/r/176012/#review181030

> I think having two variables pointing to the same value adds confusion here, even if the change is temporary. We should probably be consistent with changes from release-runner.py file and have `decision_task_id` and `second_decision_task_id` and s,old,new everywhere in the file(s).

I'm a bit on the fence about renaming task_group_id into decision_task_id. It's technically both. task_group_id sounds righter to me because we submit a group of tasks, first and foremost.

I don't mind changing it, but I think it's not really a big deal, as both are true and it remains temporary (like it won't last the week)
(Assignee)

Comment 9

2 years ago
Comment on attachment 8904231 [details]
Bug 1396517 - Make Fennec graph 2 not reference graph 1

Reverted at https://hg.mozilla.org/build/tools/rev/2f7c00c33e0aab4d6aba1dd3307e12d6e7e0bec5
Attachment #8904231 - Flags: checked-in-
(Assignee)

Comment 11

2 years ago
Deployed on bm{83,85}

Fixed by comment 7 and comment 8.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.