Closed Bug 1288742 Opened 8 years ago Closed 8 years ago

Increase expiration of gecko decision artifacts

Categories

(Taskcluster :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: armenzg, Assigned: armenzg)

Details

Attachments

(1 file)

They currently expire after a week and causes "adding new jobs" not to work (bug 1288741).

Anyone has any objections to bump this to a year?
Assignee: nobody → armenzg
I'm just realizing that maybe we don't want this for the image building contexts... Which are artifacts of the decision task if I recall correctly...

I think we can move them to a different folder and have two artifact folders... That should solve it.
The ones that are under public/decision_task [1]?

How can I get this done? Like this?
        artifacts:
          'public':
            type: 'directory'
            path: '/home/worker/artifacts'
            expires: '{{#from_now}}364 days{{/from_now}}'
+         'public/decision_task':
+           type: 'directory'
+           path: '/home/worker/artifacts/decision_task'
+           expires: '{{#from_now}}7 days{{/from_now}}'



[1] https://tools.taskcluster.net/task-inspector/#IYA6_4ZSSFKKYkcDOnNgtg/0
hmm...

We probably have move the decision_task artifacts... Perhaps:
        artifacts:
          'public':
            type: 'directory'
            path: '/home/worker/artifacts'
            expires: '{{#from_now}}364 days{{/from_now}}'
+         'public/decision_task':
+           type: 'directory'
+           path: '/home/worker/decision_task_artifacts'
+           expires: '{{#from_now}}7 days{{/from_now}}'

Then we have to change the script that writes these files to write to:
  /home/worker/decision_task_artifacts
instead of:
  /home/worker/artifacts/decision_task

Artifact folders are recursive, so we can't have two inside each other.
Especially, not if they target the same prefix.
Comment on attachment 8773812 [details]
Bug 1288742 - Increase expiration date of all gecko decision artifacts bar image context tar balls.

https://reviewboard.mozilla.org/r/66488/#review63846

::: .taskcluster.yml:111
(Diff revision 1)
>  
>          artifacts:
>            'public':
>              type: 'directory'
>              path: '/home/worker/artifacts'
> -            expires: '{{#from_now}}7 days{{/from_now}}'
> +            expires: '{{#from_now}}364 days{{/from_now}}'

See comments on bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1288742#c4

We better move the context tarballs used for docker image generation to another folder with a different expiration, as they can be rather large.
Attachment #8773812 - Flags: review?(jopsen) → review-
Comment on attachment 8773812 [details]
Bug 1288742 - Increase expiration date of all gecko decision artifacts bar image context tar balls.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66488/diff/1-2/
Attachment #8773812 - Attachment description: Bug 1288742 - Increase expiration of gecko decision task and artifacts. → Bug 1288742 - Increase expiration date of all gecko decision artifacts bar image context tar balls.
Attachment #8773812 - Flags: review- → review?(dustin)
Thanks Jonas for the feedback. Passing it to dustin since he's been involved with some of the recent changes for "run-decision".

dustin I will remove the review request until I see the try push work. I've got the flow with mozreview mixed up.
Attachment #8773812 - Flags: review?(dustin)
Is there a reason why this change did not run on try?
Do we need to recreate the docker image? (probably)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4a95191d8f5
1:28 PM <dustin> you can't have two keys named "public"
1:28 PM <dustin> in the artifacts dict
1:29 PM <dustin> I'd suggest naming one of them public/docker-image-contexts
1:29 PM <dustin> and naming the artifact directory on the worker similarly
I think it worked this time:
https://tools.taskcluster.net/task-inspector/#UN3jFpZoQjqG7irO5MQzWA/0

Even though I can't tell if they each have different expiry dates.
Comment on attachment 8773812 [details]
Bug 1288742 - Increase expiration date of all gecko decision artifacts bar image context tar balls.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66488/diff/2-3/
Attachment #8773812 - Flags: review?(dustin)
https://reviewboard.mozilla.org/r/66488/#review64226

Sorry, this is a lot of comments for a short patch :)

::: .taskcluster.yml:109
(Diff revision 3)
>          artifacts:
>            'public':
>              type: 'directory'
>              path: '/home/worker/artifacts'
> +            expires: '{{#from_now}}364 days{{/from_now}}'
> +          'public/docker-image-contexts':

Since the local directory is named with `_`, and `image_contexts` has `_`, we should probably use `_` here too.

::: taskcluster/taskgraph/task/docker_image.py:74
(Diff revision 3)
>              image_parameters['context_path'] = context_path
>              image_parameters['artifact_path'] = 'public/image.tar'
>              image_parameters['image_name'] = image_name
>  
>              image_artifact_path = \
>                  "public/decision_task/image_contexts/{}/context.tar.gz".format(image_name)

This will need to be `public/docker-image-contexts/image_contexts/{}/context.tar.gz`

::: testing/docker/decision/Dockerfile:7
(Diff revision 3)
>  MAINTAINER    Greg Arndt <garndt@mozilla.com>
>  
>  # Add worker user
>  RUN useradd -d /home/worker -s /bin/bash -m worker
>  RUN mkdir /home/worker/artifacts && chown worker:worker /home/worker/artifacts
> +RUN mkdir /home/worker/docker_image_contexts && chown worker:worker /home/worker/docker_image_contexts

The `docker_image.py` code will create this directory, so no need to do so here (and thus no need to bump the decision docker image version)

::: testing/docker/decision/bin/run-decision:19
(Diff revision 3)
>  fi
>  
>  /home/worker/bin/checkout-gecko /home/worker/workspace/gecko
>  cd /home/worker/workspace/gecko
>  ln -s /home/worker/artifacts artifacts
> +ln -s /home/worker/docker_image_contexts docker_image_contexts

You can skip this too.  For `artifacts`, I think this is come compatibility for code that writes to `./artifacts` instead of `~/artifacts`, but we don't need it here.
Comment on attachment 8773812 [details]
Bug 1288742 - Increase expiration date of all gecko decision artifacts bar image context tar balls.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66488/diff/3-4/
I fixed all your concerns.

Do you prefer changing this:
> `public/docker_image_contexts/image_contexts/{}/context.tar.gz`
to this:
> `public/docker_image_contexts/{}/context.tar.gz`
? Or keep it as-is.
Good point; the second one (`public/docker_image_contexts/{}/context.tar.gz`) is less redundant and would be best.
Comment on attachment 8773812 [details]
Bug 1288742 - Increase expiration date of all gecko decision artifacts bar image context tar balls.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66488/diff/4-5/
Comment on attachment 8773812 [details]
Bug 1288742 - Increase expiration date of all gecko decision artifacts bar image context tar balls.

https://reviewboard.mozilla.org/r/66488/#review65512
Attachment #8773812 - Flags: review?(dustin) → review+
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9f5fbb3066c9
Increase expiration date of all gecko decision artifacts bar image context tar balls. r=dustin
https://hg.mozilla.org/mozilla-central/rev/9f5fbb3066c9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
I've added Buildbot and TaskCluster jobs to this push from over a week old:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=60f46b18ad5db0d6d6855183f009d3d50b860fae&filter-searchStr=reftest
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.