Closed Bug 1278312 Opened 4 years ago Closed 3 years ago

Uptake monitoring failed in graph2

Categories

(Release Engineering :: Release Automation: Other, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rail, Assigned: mtabara)

References

Details

Attachments

(3 files)

Assignee: nobody → mtabara
I'm a bit confused as to how some of those buildprops value are missing. The uptake monitoring script mainly needs four mandatory buildprops to function properly: version, tuxedo_server_url, partial_versions and platforms. Did some debugging and it seems that in both mozilla-release and mozilla-esr45 there are three buidprops args missing:

"partial_versions": null,
"platform": null, (not needed for uptake monitoring but just raising a flag that's also been set wrong)
"platforms": null, 

If it didn't fail in partial_versions, it would have failed anyway when dealing with platforms the next line: http://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/uptake_monitoring.py#l82

These configs are set just as the other scripts in releasetasks: https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/firefox/uptake_monitoring.yml.tmpl so I doubt the culprit is here. 

Could it be something missing from https://github.com/mozilla/releasetasks/tree/master/releasetasks/release_configs ? (both release and esr45)

On a separate note, I agree I could also add some more safeguarding when processing these values at http://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/uptake_monitoring.py#l81 and even add more checks like this http://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/uptake_monitoring.py#l118
Flags: needinfo?(rail)
Looks like the graph2 generator script expects you to pass partials: http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/releasetasks_graph_gen.py#l169

We definitely can put them into https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/firefox/common_extras.yml.tmpl and read from the script.
Flags: needinfo?(rail)
I added PR for releasetasks and build-tools to fix the uptake monitoring on relpro graph2
* partial_versions => stashed in in the common extras and grabbed it later on from the common_task_id
* platforms => similar behavior with final_verify_platforms, read from release configs / branch configs in both graphs

Questions:
1. Do I need to add the "uptake_monitoring_platforms" in jamun/date project branches?
2. Should I file a separate bug to deal with the inevitable cleanup that's imperative in http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/releasetasks_graph_gen.py#l169 ? (not just partials but also build-number, version and revision, etc)
Flags: needinfo?(rail)
Attachment #8766076 - Flags: review?(rail) → review+
Comment on attachment 8766076 [details]
Bug 1278312 - fix uptake monitoring for relpro graph2.

https://reviewboard.mozilla.org/r/61120/#review58156
Flags: needinfo?(rail)
Attachment #8766074 - Flags: review?(rail) → review+
(In reply to Mihai Tabara [:mtabara] from comment #6)
> I added PR for releasetasks and build-tools to fix the uptake monitoring on
> relpro graph2
> * partial_versions => stashed in in the common extras and grabbed it later
> on from the common_task_id
> * platforms => similar behavior with final_verify_platforms, read from
> release configs / branch configs in both graphs
> 
> Questions:
> 1. Do I need to add the "uptake_monitoring_platforms" in jamun/date project
> branches?

I'm not sure what you mean. I thought you use release_platforms in graph1 and uptake_monitoring_platforms for graph2. The latter is already addressed in your PR (the dev_* configs).


> 2. Should I file a separate bug to deal with the inevitable cleanup that's
> imperative in
> http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/
> releasetasks_graph_gen.py#l169 ? (not just partials but also build-number,
> version and revision, etc)

You mean killing the feature of passing things via cli args vs fetching them from a task? I think we can kill it.
(In reply to Rail Aliiev [:rail] from comment #8)
> I'm not sure what you mean. I thought you use release_platforms in graph1
> and uptake_monitoring_platforms for graph2. The latter is already addressed
> in your PR (the dev_* configs).

Ah, right. For graph1 I'm reusing branchConfig['release_platforms'] whilst for graph2 I've got the dev_*configs.

> > 2. Should I file a separate bug to deal with the inevitable cleanup that's
> > imperative in
> > http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/
> > releasetasks_graph_gen.py#l169 ? (not just partials but also build-number,
> > version and revision, etc)
> 
> You mean killing the feature of passing things via cli args vs fetching them
> from a task? I think we can kill it.

Yeah. But again, not just the partials argument, but also the other three which are most likely grabbed from common task id.
Merged the PR - https://github.com/mozilla/releasetasks/commit/d690c5e58840feead7ebb73edf11d00aad6ef76e
Landed the tools changes as well - http://hg.mozilla.org/build/tools/rev/58243ff61d9e

We'll have to wait for 48.0 release to test and eventual close the bug.
See Also: → 1283853
Had to tweak the common extras whilst fixing this bug last week. I stashed in common extras more data, with respect to the partials and platforms as well.Since most of the stuff is taken from common extras, I thought it'd be better to entirely rely on that, rather than the command line args.

Not sure though what to do with mozharness revision. Are we to default it always to mozilla_revision or should I stash it in the common extras as well since we're here?
Flags: needinfo?(jlund)
Attachment #8767750 - Flags: review?(jlund)
Comment on attachment 8767750 [details] [review]
Some quick code cleanup to rely on common task

this is fine if you feel strongly. I commented in the PR as to why I have it in there.
Flags: needinfo?(jlund)
Attachment #8767750 - Flags: review?(jlund) → review+
if the revision is defined in common extras, that seems fine. it be nice to let the graph 2 generation use a different mozharness rev in case we need to like we can with graph1
@jlund: Thanks for the info and explanations. Didn't know the context, TIL something - will drop the patch and close the PR.

Note to self: coming back to the main topic of the bug:
* before closing this bug, we'll have to wait for 48.0 to test the uptake monitoring fixup for a release
* currently, bug 1278312 blocks this one. We'll have to wait for 48.b6 to confirm the fix works as expected
Depends on: 1290918
You need to log in before you can comment on or make changes to this bug.