Bug 1635488 - add Fenix version-bump task. #16361

MihaiTabara · 2020-11-04T00:12:55Z

Goal is to add the version-bump task that's supposed to update the version.txt file in the repository.

I need to double check the arguments against mozilla-releng/scriptworker-scripts#289 to see which payload arguments we keep for git and which we don't.

Following that I can also ammend ci-admin and ci-config + adding SOPS/cloudops-infra configs for the mobile-{1,3}-tree worker.

CC @JohanLorenzo

JohanLorenzo

Thanks for this first stab at the patch. This is just a preliminary review I'm making. I'm going to test it end-to-end on my own fork. I just made the comments below in order to remind me about the things I may have to change to get this patch fully tested. No need to address these comments now 🙂

JohanLorenzo · 2020-11-04T09:02:17Z

taskcluster/fenix_taskgraph/worker_types.py

+                Required("paths"): [text_type],
+            }
+        ],
+        Optional("dontbuild"): bool,


I'd say: no need to support DONTBUILD yet. It's just supported for HG, so far https://hg.mozilla.org/ci/taskgraph/file/4c4d72d32c5ec36a3d0187bf792ac08f1348f9fa/src/taskgraph/decision.py#l190

Agreed. Also TIL taskgraph supports DONTBUILD at that level 👍

JohanLorenzo · 2020-11-04T09:02:35Z

taskcluster/fenix_taskgraph/worker_types.py

+@payload_builder(
+    "scriptworker-tree",
+    schema={
+        # FIXME: do we need upstream artifacts?


I don't think we do.

I was trying to get rid of upstream-artifacts and wasn’t sure how. But I realized in the meantime, upstream-artifacts is being added in my task given https://github.com/mozilla-mobile/fenix/blob/master/taskcluster/fenix_taskgraph/transforms/multi_dep.py#L74 so I can simply switch in the payload from Required to Optional for now.

JohanLorenzo · 2020-11-04T09:03:44Z

taskcluster/fenix_taskgraph/worker_types.py

+    task_def['payload'] = {'actions': []}
+    actions = task_def['payload']['actions']
+    if worker['tags']:
+        # TODO: add tags logic here


I don't think we need to support tags, at the moment. Githubscript implicitly creates one whenever a github release is made.

Ah, so we solve this problem in github-release task concurrently from this, makes sense.

JohanLorenzo · 2020-11-04T09:09:36Z

taskcluster/ci/version-bump/kind.yml

+kind-dependencies:
+    - github-release
+
+primary-dependency: github-release # FIXME: should we block on pushapk instead or both?


push-apk is a good idea. github-release doesn't depend on the state of the tree so both tasks can run at the same time, in my opinion.

Agreed, changed to depend on pushapk instead.

codecov-io · 2020-11-06T12:51:24Z

Codecov Report

Merging #16361 (8a2641f) into master (4570f5a) will decrease coverage by 0.04%.
The diff coverage is 8.91%.

@@             Coverage Diff              @@
##             master   #16361      +/-   ##
============================================
- Coverage     29.97%   29.92%   -0.05%     
+ Complexity     1224     1223       -1     
============================================
  Files           457      457              
  Lines         18770    18802      +32     
  Branches       2582     2586       +4     
============================================
+ Hits           5626     5627       +1     
- Misses        12704    12735      +31     
  Partials        440      440

Impacted Files	Coverage Δ	Complexity Δ
...pp/src/main/java/org/mozilla/fenix/FeatureFlags.kt	`100.00% <ø> (ø)`	`4.00 <0.00> (-1.00)`
...src/main/java/org/mozilla/fenix/components/Core.kt	`18.23% <0.00%> (-1.02%)`	`1.00 <0.00> (ø)`
...lla/fenix/components/toolbar/DefaultToolbarMenu.kt	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...trol/viewholders/topsites/TopSiteItemViewHolder.kt	`17.85% <0.00%> (-0.33%)`	`2.00 <0.00> (ø)`
...g/mozilla/fenix/settings/SecretSettingsFragment.kt	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...in/java/org/mozilla/fenix/settings/SupportUtils.kt	`77.08% <ø> (ø)`	`13.00 <0.00> (ø)`
...org/mozilla/fenix/settings/TabsSettingsFragment.kt	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...java/org/mozilla/fenix/tabtray/FenixTabsAdapter.kt	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../mozilla/fenix/tabtray/MultiselectSelectionMenu.kt	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
.../org/mozilla/fenix/tabtray/SyncedTabsController.kt	`86.36% <ø> (ø)`	`3.00 <0.00> (ø)`
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d45bdfb...8a2641f. Read the comment docs.

JohanLorenzo

I managed to test this patch end-to-end. It required a few changes that are unnecessary on prod. I tested:

A version bump from shipit: https://firefox-ci-tc.services.mozilla.com/tasks/L71svzYuTyqbI88_p-wJ3Q/runs/1
A version bump from github-releases: https://firefox-ci-tc.services.mozilla.com/tasks/cOrQjiFjT1G7AqeJg8_Ybw
A wrong version number, which is failing as expected https://firefox-ci-tc.services.mozilla.com/tasks/AuxX3sTMSmWnVjduyvqSLA/runs/0

I'm feeling confident we can land this on Monday and test the next release with it. @st3fan, when is the first 84 beta planned?

@MihaiTabara, feel free to take a look, if you have enough time.

JohanLorenzo · 2020-11-13T09:58:28Z

.taskcluster.yml

@@ -243,7 +243,7 @@ tasks:
                              # Note: This task is built server side without the context or tooling that
                              # exist in tree so we must hard code the hash
                              image:
-                                  mozillareleases/taskgraph:decision-mobile-6607973bc60e32323a541861cc5856cd6a0f51ea9fd664ef7d43bca8df53db47@sha256:8c471aacc469ea8e7bb4846c16efe086f7350a5cc1df570cc6c86b22895a2456
+                                  mozillareleases/taskgraph:decision-mobile-682fbaa1ef17e70ddfe3457da3eaf8e776c4a20fe5bfbdbeba0641fd5bceae2a@sha256:bbb2613aaab79d17e590fbd78c072d0643be40fd1237195703f84280ecc3b302


I didn't remember how to publish a new docker image. I'll write some documentation because I think only Tom, Callek, and I knew the procedure.

Good point on adding docs! I think we have some for image_builders in https://moz-releng-docs.readthedocs.io/en/latest/taskcluster/uploading_an_image.html - it might be worth refreshing that or adding another section there?

Quick question, why do we need to bump this image?

If I understand correctly, the python packages are baked in the image. Bumping taskgraph above doesn't update them.

JohanLorenzo · 2020-11-13T09:59:33Z

.taskcluster.yml

                                            taskcluster/scripts/decision-install-sdk.sh &&
                                            ln -s /builds/worker/artifacts artifacts &&
                                            ~/.local/bin/taskgraph action-callback
                                        else: >
                                            PIP_IGNORE_INSTALLED=0 pip install --user /builds/worker/checkouts/taskgraph &&
-                                            PIP_IGNORE_INSTALLED=0 pip install --user arrow taskcluster pyyaml &&


pyyaml is already embedded in taskgraph, so no need to reinstall it: https://hg.mozilla.org/ci/taskgraph/file/2b2622598df02bde211d8cedb334b7b22fb883a4/requirements/base.in#l5

arrow and taskcluster were used pre-taskgraph. I checked: we don't need them anymore.

Nice cleanup 👍

JohanLorenzo · 2020-11-13T10:00:07Z

.taskcluster.yml

@@ -261,12 +261,13 @@ tasks:
                                        $if: 'tasks_for == "action"'
                                        then: >
                                            PIP_IGNORE_INSTALLED=0 pip install --user /builds/worker/checkouts/taskgraph &&
+                                            PIP_IGNORE_INSTALLED=0 pip install --user mozilla-version &&


mozilla-version is used mainly to support github-releases. We may be able to remove it once we drop support.

Nit: should we add a quick TODO to revisit this once we drop support for github-releases?

I'm unsure whether we could remove it. I'd say: let's not put a misleading TODO for now. I'll revisit once I know more.

JohanLorenzo · 2020-11-13T10:01:05Z

taskcluster/ci/version-bump/kind.yml

+                "3": true
+                # If you set the following line to true, you need to grant write access
+                # to https://github.com/mozilla-release-automation-bot-staging on your
+                # fork.


Doing so enabled me to get this kind of staging commits JohanLorenzo@f493a3a

Thanks for the comment, that's useful to know!

JohanLorenzo · 2020-11-13T10:02:14Z

taskcluster/fenix_taskgraph/parameters.py

        parameters["target_tasks_method"] = "release"

-
-def resolve_release_type(head_tag):


This function is not useful anymore since we can rely on https://github.com/mozilla-releng/mozilla-version. That library knows how to detect a beta version from a release one.

mozilla-version FTW!

JohanLorenzo · 2020-11-13T10:06:45Z

taskcluster/fenix_taskgraph/parameters.py

+
+        version = FenixVersion.parse(version_string)
+        if version.is_beta:
+            next_version = version.bump("beta_number")


In order to prevent mismatch, we need github-releases to bump version.txt too. Shipit knows how to provide what the next version is. We have to replicate this logic on github-release, while we support this use case.

JohanLorenzo · 2020-11-13T10:07:25Z

taskcluster/fenix_taskgraph/release_promotion.py

 RELEASE_PROMOTION_PROJECTS = (
-    "https://github.com/JohanLorenzo/fenix",


My fork may be needed in the future, but we don't need it to be whitelisted on the main repo.

JohanLorenzo · 2020-11-13T10:08:00Z

taskcluster/fenix_taskgraph/release_promotion.py

-    parameters['release_type'] = resolve_release_type(parameters['head_tag'])
+    version_string = parameters['version']
+    if version_string != version_in_file:
+        raise ValueError("Version given in tag ({}) does not match the one in version.txt ({})".format(version_string, version_in_file))


This error should never happen, but it prevents shipit from being regressed.

JohanLorenzo · 2020-11-13T10:08:41Z

taskcluster/fenix_taskgraph/release_promotion.py

+    parameters['next_version'] = input['next_version']
+
+    version = FenixVersion.parse(version_string)
+    if version.is_beta:


I could have factorized this behavior with what we have in parameters.py. Although, I intend that logic to be gone by the end of the year.

This is why, I'm okay to have a little bit of code duplication here.

Makes sense 👍

JohanLorenzo · 2020-11-13T10:10:33Z

version.txt

@@ -1 +1 @@
-4.0.0-beta.2
+84.0.0-beta.1


v83 is already on its stabilization branch and will ship sometimes next week. As far as I understand, 84 is what's on the master branch.

Can't wait to test this in production!

MihaiTabara

@JohanLorenzo Just to confirm, if this patch merges, along with the scriptworker-scripts - we will have the ability to trigger staging releases and to version bumps vs github-releases. But we still need to turn-on the Fenix into Ship-it + potentially give mobile folks access to Ship-it, right?

MihaiTabara · 2020-11-13T10:50:14Z

.taskcluster.yml

@@ -243,7 +243,7 @@ tasks:
                              # Note: This task is built server side without the context or tooling that
                              # exist in tree so we must hard code the hash
                              image:
-                                  mozillareleases/taskgraph:decision-mobile-6607973bc60e32323a541861cc5856cd6a0f51ea9fd664ef7d43bca8df53db47@sha256:8c471aacc469ea8e7bb4846c16efe086f7350a5cc1df570cc6c86b22895a2456
+                                  mozillareleases/taskgraph:decision-mobile-682fbaa1ef17e70ddfe3457da3eaf8e776c4a20fe5bfbdbeba0641fd5bceae2a@sha256:bbb2613aaab79d17e590fbd78c072d0643be40fd1237195703f84280ecc3b302


Good point on adding docs! I think we have some for image_builders in https://moz-releng-docs.readthedocs.io/en/latest/taskcluster/uploading_an_image.html - it might be worth refreshing that or adding another section there?

MihaiTabara · 2020-11-13T10:52:42Z

.taskcluster.yml

@@ -8,7 +8,7 @@ tasks:
    - $let:
          taskgraph:
              branch: taskgraph
-              revision: 12992b0f984884ec2b0a7bdedc3b3ba467363eb4
+              revision: 2b2622598df02bde211d8cedb334b7b22fb883a4


Note to self: I reckon this is to get https://hg.mozilla.org/ci/taskgraph/rev/2b2622598df02bde211d8cedb334b7b22fb883a4 baked it. I see the previous taskgraph revision 12992b0f984884ec2b0a7bdedc3b3ba467363eb4 was the one right after kaniko update so I think this is fine, we already have the public/docker-contexts exposed.

MihaiTabara · 2020-11-13T10:53:16Z

.taskcluster.yml

@@ -243,7 +243,7 @@ tasks:
                              # Note: This task is built server side without the context or tooling that
                              # exist in tree so we must hard code the hash
                              image:
-                                  mozillareleases/taskgraph:decision-mobile-6607973bc60e32323a541861cc5856cd6a0f51ea9fd664ef7d43bca8df53db47@sha256:8c471aacc469ea8e7bb4846c16efe086f7350a5cc1df570cc6c86b22895a2456
+                                  mozillareleases/taskgraph:decision-mobile-682fbaa1ef17e70ddfe3457da3eaf8e776c4a20fe5bfbdbeba0641fd5bceae2a@sha256:bbb2613aaab79d17e590fbd78c072d0643be40fd1237195703f84280ecc3b302


Quick question, why do we need to bump this image?

MihaiTabara · 2020-11-13T10:54:04Z

.taskcluster.yml

@@ -261,12 +261,13 @@ tasks:
                                        $if: 'tasks_for == "action"'
                                        then: >
                                            PIP_IGNORE_INSTALLED=0 pip install --user /builds/worker/checkouts/taskgraph &&
+                                            PIP_IGNORE_INSTALLED=0 pip install --user mozilla-version &&


Nit: should we add a quick TODO to revisit this once we drop support for github-releases?

MihaiTabara · 2020-11-13T10:54:25Z

.taskcluster.yml

                                            taskcluster/scripts/decision-install-sdk.sh &&
                                            ln -s /builds/worker/artifacts artifacts &&
                                            ~/.local/bin/taskgraph action-callback
                                        else: >
                                            PIP_IGNORE_INSTALLED=0 pip install --user /builds/worker/checkouts/taskgraph &&
-                                            PIP_IGNORE_INSTALLED=0 pip install --user arrow taskcluster pyyaml &&


Nice cleanup 👍

MihaiTabara · 2020-11-13T10:56:29Z

taskcluster/fenix_taskgraph/parameters.py

        parameters["target_tasks_method"] = "release"

-
-def resolve_release_type(head_tag):


mozilla-version FTW!

MihaiTabara · 2020-11-13T10:59:00Z

taskcluster/fenix_taskgraph/parameters.py

+        version_string = parameters["version"]
+        version_in_file = read_version_file()
+        if version_string != version_in_file:
+            raise ValueError("Version given in tag ({}) does not match the one in version.txt ({})".format(version_string, version_in_file))


Just to confirm I get this right - if for some reason the next two-three releases are done via github-releases instead of Ship-it, wouldn't that mean we have to manually bump the version.txt file?

MihaiTabara · 2020-11-13T11:01:02Z

taskcluster/fenix_taskgraph/parameters.py

+        version_string = parameters["version"]
+        version_in_file = read_version_file()
+        if version_string != version_in_file:
+            raise ValueError("Version given in tag ({}) does not match the one in version.txt ({})".format(version_string, version_in_file))


Later edit: nevermind the above. Regardless of it being triggered from Ship-it or via github-release, the task group that we generate, both contain the version-bump tasks so we can't fall behind.

MihaiTabara · 2020-11-13T11:01:58Z

taskcluster/fenix_taskgraph/release_promotion.py

+    parameters['next_version'] = input['next_version']
+
+    version = FenixVersion.parse(version_string)
+    if version.is_beta:


Makes sense 👍

MihaiTabara · 2020-11-13T11:02:33Z

version.txt

@@ -1 +1 @@
-4.0.0-beta.2
+84.0.0-beta.1


Can't wait to test this in production!

JohanLorenzo · 2020-11-13T13:21:51Z

we will have the ability to trigger staging releases

To trigger real releases.

But we still need to turn-on the Fenix into Ship-it + potentially give mobile folks access to Ship-it, right?

Correct.

MihaiTabara added the pr:work-in-progress PRs that are not ready to be reviewed yet and are actively being worked on label Nov 4, 2020

MihaiTabara requested review from a team as code owners November 4, 2020 00:12

MihaiTabara self-assigned this Nov 4, 2020

JohanLorenzo reviewed Nov 4, 2020

View reviewed changes

MihaiTabara requested a review from a team as a code owner November 13, 2020 09:52

JohanLorenzo changed the title ~~WIP. Bug 1635488 - add Fenix version-bump task.~~ Bug 1635488 - add Fenix version-bump task. Nov 13, 2020

Mihai Tabara and others added 5 commits November 13, 2020 10:56

WIP. Bug 1635488 - add Fenix version-bump task.

d788761

Addressing Johan's comments

fd8d812

Remove scopes as they are not needed for version-bump

f877ae8

Add push payload item

0c808da

Changes after end-to-end tests

8a2641f

JohanLorenzo force-pushed the version-bump branch from 7a281d8 to 8a2641f Compare November 13, 2020 09:57

JohanLorenzo approved these changes Nov 13, 2020

View reviewed changes

JohanLorenzo removed the pr:work-in-progress PRs that are not ready to be reviewed yet and are actively being worked on label Nov 13, 2020

MihaiTabara commented Nov 13, 2020

View reviewed changes

JohanLorenzo merged commit 7d3b23c into mozilla-mobile:master Nov 16, 2020

		parameters["target_tasks_method"] = "release"


		def resolve_release_type(head_tag):

		RELEASE_PROMOTION_PROJECTS = (
		"https://github.com/JohanLorenzo/fenix",

		@@ -1 +1 @@
		4.0.0-beta.2
		84.0.0-beta.1

		@@ -1 +1 @@
		4.0.0-beta.2
		84.0.0-beta.1

Bug 1635488 - add Fenix version-bump task. #16361

Bug 1635488 - add Fenix version-bump task. #16361

Conversation

MihaiTabara commented Nov 4, 2020 • edited Loading

JohanLorenzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Nov 6, 2020 • edited Loading

Codecov Report

JohanLorenzo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MihaiTabara left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohanLorenzo commented Nov 13, 2020 • edited Loading

MihaiTabara commented Nov 4, 2020 •

edited

Loading

codecov-io commented Nov 6, 2020 •

edited

Loading

JohanLorenzo left a comment •

edited

Loading

JohanLorenzo commented Nov 13, 2020 •

edited

Loading