1288742 - Increase expiration of gecko decision artifacts

Status: VERIFIED FIXED

(Taskcluster :: General, defect)

Description

[Armen [:armenzg]]

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?

Comment 1

[Armen [:armenzg]]

Attachment: Bug 1288742 - Increase expiration date of all gecko decision artifacts bar image context tar balls.

Review commit: https://reviewboard.mozilla.org/r/66488/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66488/

Comment 2

[Jonas Finnemann Jensen (:jonasfj)]

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.

Comment 3

[Armen [:armenzg]]

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

Comment 4

[Jonas Finnemann Jensen (:jonasfj)]

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 5

[Jonas Finnemann Jensen (:jonasfj)]

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.

Comment 6

[Armen [:armenzg]]

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/

Comment 7

[Armen [:armenzg]]

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.

Comment 8

[Armen [:armenzg]]

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

Comment 9

[Armen [:armenzg]]

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

Comment 10

[Armen [:armenzg]]

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 11

[Armen [:armenzg]]

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/

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

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 <[e-mail]>
>  
>  # 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 13

[Armen [:armenzg]]

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/

Comment 14

[Armen [:armenzg]]

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.

Comment 15

[Armen [:armenzg]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c7be86e9599

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

Good point; the second one (`public/docker_image_contexts/{}/context.tar.gz`) is less redundant and would be best.

Comment 17

[Armen [:armenzg]]

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 18

[Dustin J. Mitchell [:dustin] (he/him)]

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

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f5fbb3066c9
Increase expiration date of all gecko decision artifacts bar image context tar balls. r=dustin

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9f5fbb3066c9

Comment 21

[Armen [:armenzg]]

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