DCOS-46615 - Retry downloading pkgpanda pkg if file size incorrect #4549

mrnugget · 2019-02-19T07:56:30Z

High-level description

AWS S3, from which we download pre-built pkgpanda packages, might sometimes end the connection before the package has been fully downloaded. (See aws/aws-sdk-js#312 and boto/boto3#540 for more context on this)

In our case that leads to invalid tar.xz archives which cause unexpected end of input errors and result in faulty builds of DC/OS (dcos_generate_config.sh) which contain these invalid archives and then result in install-time errors for users. See this comment for an investigation into how this can affect installation errors: https://jira.mesosphere.com/browse/DCOS_OSS-4097?focusedCommentId=219259&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-219259

This change here adds a sanity check to the download function used by pkgpanda. It checks whether the final file size (after flushing the file to disk) matches the Content-Length we get from S3. If it doesn't match, it sleep for 2, 4, 8, ... seconds and then retries until it either downloads the file completely or the retries are exhausted. In that case, it raises an exception.

Corresponding DC/OS tickets (obligatory)

These DC/OS JIRA ticket(s) must be updated (ideally closed) in the moment this PR lands:

[DCOS-46615] Pkgpanda - package dcos-image - xz: (stdin): Unexpected end of input - JIRA

Related tickets (optional)

Other tickets related to this change:

[DCOS_OSS-4097] pkgpanda: Explicitly create user groups for all users - JIRA

Checklist for all PRs

Added a comprehensible changelog entry to CHANGES.md or explain why this is not a user-facing change: it's internal behaviour that's not exposed to the user
Included a test which will fail if code is reverted but test is not. If there is no test please explain here: I didn't include a test yet, since that would need to involve setting up a mock http server etc. and I wanted to get input on this first
Read the DC/OS contributing guidelines
Followed relevant code rules Rules for Packages and Systemd

Checklist for component/package updates:

If you are changing components or packages in DC/OS (e.g. you are bumping the sha or ref of anything underneath packages), then in addition to the above please also include:

Change log from the last version integrated (this should be a link to commits for easy verification and review): example
Test Results: [link to CI job test results for component]
Code Coverage (if available): [link to code coverage report]

PLEASE FILL IN THE TEMPLATE ABOVE / DO NOT REMOVE ANY SECTIONS ABOVE THIS LINE

Instructions and review process

What is the review process and when will my changes land?

All PRs require 2 approvals using GitHub's pull request reviews.

Reviewers should be:

Developers who understand the code being modified.
Developers responsible for code that interacts with or depends on the code being modified.

It is best to proactively ask for 2 reviews by @mentioning the candidate reviewers in the PR comments area. The responsibility is on the developer submitting the PR to follow-up with reviewers and make sure a PR is reviewed in a timely manner. Once a PR has 2 ship-it's, no red reviews, and all tests are green it will be included in the next train.

d2iq-mergebot · 2019-02-19T07:56:33Z

This repo has @mesosphere-mergebot integration. You can perform the following commands by submitting a comment. Submit a comment with content "@mesosphere-mergebot help" to view more detailed help text and examples. Be sure the have a look at the mergebot documentation, too.

@mesosphere-mergebot sync  
@mesosphere-mergebot merge-it  
@mesosphere-mergebot bump-ee  
@mesosphere-mergebot label [Ship It|Work In Progress|Request For Comment|Holding|merge-by-mergebot|Ready For Review] 
@mesosphere-mergebot override-status pr-status-check jira-url

PR creators can apply one of [Ready For Review|Work In Progress]. Owners can apply any label.

d2iq-mergebot · 2019-02-19T07:56:33Z

@mesosphere-mergebot bump-ee

d2iq-mergebot · 2019-02-19T07:56:58Z

Enterprise Bump PR: mesosphere/dcos-enterprise/pull/4694

d2iq-mergebot · 2019-02-19T09:02:14Z

@mesosphere-mergebot bump-ee

d2iq-mergebot · 2019-02-19T09:02:43Z

Enterprise Bump mesosphere/dcos-enterprise/pull/4694 updated.

gpaul

Good start

pkgpanda/test_util.py

gpaul · 2019-02-20T07:51:45Z

pkgpanda/test_util.py

+    mock_server_port = _get_free_port()
+    mock_server = MockHTTPDownloadServer(('localhost', mock_server_port), MockDownloadServerRequestHandler)
+
+    mock_server_thread = Thread(target=mock_server.serve_forever)


Threads in python are where the wheels come off for me. After some googling around this seems to be idiomatic code, but things like spawning threads that never terminate make me uneasy.

pkgpanda/test_util.py

pkgpanda/util.py

d2iq-mergebot · 2019-02-25T10:45:41Z

@mesosphere-mergebot bump-ee

d2iq-mergebot · 2019-02-25T10:46:06Z

Enterprise Bump mesosphere/dcos-enterprise/pull/4694 updated.

d2iq-mergebot · 2019-02-25T12:03:35Z

@mesosphere-mergebot bump-ee

d2iq-mergebot · 2019-02-25T12:04:03Z

Enterprise Bump mesosphere/dcos-enterprise/pull/4694 updated.

d2iq-mergebot · 2019-02-25T12:06:10Z

@mesosphere-mergebot bump-ee

d2iq-mergebot · 2019-02-25T12:06:39Z

Enterprise Bump mesosphere/dcos-enterprise/pull/4694 updated.

mrnugget · 2019-02-25T12:07:33Z

@mesosphere-mergebot label Ready for Review

mrnugget · 2019-02-25T12:26:05Z

@gpaul @adamtheturtle Can you re-review? I removed the useless legacy code, worked in all of your suggestions and switched to retrying – the only thing left is the long-running thread for the mock http server, which will run until the end of the test suite, I guess. I don't think that's a huge issue, but I'll look into it.

gpaul

LGTM!

drewvanstone

💪 Looks great!

mrnugget · 2019-02-26T13:18:55Z

@mesosphere-mergebot bump-ee

d2iq-mergebot · 2019-02-26T13:18:56Z

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

AWS S3, from which we download pre-built pkgpanda packages, might sometimes end the connection before the package has been fully downloaded. (See aws/aws-sdk-js#312 and boto/boto3#540 for more context on this) In our case that leads to invalid `tar.xz` archives which cause `unexpected end of input` errors and result in faulty builds of DC/OS (`dcos_generate_config.sh`) which contain these invalid archives and then result in install-time errors for users. See this comment for an investigation into how this can affect installation errors: https://jira.mesosphere.com/browse/DCOS_OSS-4097?focusedCommentId=219259&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-219259 This change here adds a sanity check to the `download` function used by pkgpanda. It checks whether the final file size (after flushing the file to disk) matches the `Content-Length` we get from S3. If it doesn't match, it sleep for 2, 4, 8, ... seconds and then retries until it either downloads the file completely or the retries are exhausted. In that case, it raises an exception.

d2iq-mergebot · 2019-02-26T14:30:06Z

@mesosphere-mergebot bump-ee

d2iq-mergebot · 2019-02-26T14:30:42Z

Enterprise Bump mesosphere/dcos-enterprise/pull/4694 updated.

gpaul · 2019-02-26T17:31:41Z

@mesosphere-mergebot sync

d2iq-mergebot · 2019-02-26T17:31:48Z

Mergebot Sync Summary

Sync actions for https://github.com/mesosphere/dcos-enterprise/pull/4694:

mergebot/review/approved/min_2: Approval Required from: @gpaul . Changes were requested in this PR.

mergebot/enterprise/review/approved/min_2: Received 2 approvals.

Aggregate Status Check:

{'context': {'EE-Integration-Test/E2E/group-1': ('success',
                                                 '2019-02-26T15:41:46Z'),
             'EE-Integration-Test/E2E/group-2': ('success',
                                                 '2019-02-26T16:12:47Z'),
             'EE-Integration-Test/E2E/group-3': ('success',
                                                 '2019-02-26T15:41:27Z'),
             'EE-Integration-Test/E2E/group-4': ('success',
                                                 '2019-02-26T15:48:27Z'),
             'mergebot/has_ship-it': ('success', '2019-02-26T17:15:40Z'),
             'mergebot/review/approved/min_2': ('success',
                                                '2019-02-26T17:10:57Z'),
             'teamcity/dcos/build/aws/cloudformation/custom/permissive': ('success',
                                                                          '2019-02-26T14:50:25Z'),
             'teamcity/dcos/build/aws/cloudformation/custom/strict': ('success',
                                                                      '2019-02-26T14:50:19Z'),
             'teamcity/dcos/build/dcos': ('success', '2019-02-26T14:46:30Z'),
             'teamcity/dcos/build/tox': ('success', '2019-02-26T14:35:16Z'),
             'teamcity/dcos/test/aws/onprem/static/strict/group/1': ('success',
                                                                     '2019-02-26T15:28:22Z'),
             'teamcity/dcos/test/aws/onprem/static/strict/group/2': ('success',
                                                                     '2019-02-26T15:55:55Z'),
             'teamcity/dcos/test/aws/onprem/static/strict/group/3': ('success',
                                                                     '2019-02-26T15:44:54Z'),
             'teamcity/dcos/test/aws/onprem/static/strict/group/4': ('success',
                                                                     '2019-02-26T15:49:55Z')},
 'state': 'success'}

gpaul · 2019-02-26T17:32:36Z

@mesosphere-mergebot sync

d2iq-mergebot · 2019-02-26T17:32:43Z

Mergebot Sync Summary

Sync actions for https://github.com/mesosphere/dcos-enterprise/pull/4694:

mergebot/review/approved/min_2: Received 3 approvals.

mergebot/enterprise/review/approved/min_2: Received 2 approvals.

Aggregate Status Check:

{'context': {'EE-Integration-Test/E2E/group-1': ('success',
                                                 '2019-02-26T15:41:46Z'),
             'EE-Integration-Test/E2E/group-2': ('success',
                                                 '2019-02-26T16:12:47Z'),
             'EE-Integration-Test/E2E/group-3': ('success',
                                                 '2019-02-26T15:41:27Z'),
             'EE-Integration-Test/E2E/group-4': ('success',
                                                 '2019-02-26T15:48:27Z'),
             'mergebot/has_ship-it': ('success', '2019-02-26T17:15:40Z'),
             'mergebot/review/approved/min_2': ('success',
                                                '2019-02-26T17:10:57Z'),
             'teamcity/dcos/build/aws/cloudformation/custom/permissive': ('success',
                                                                          '2019-02-26T14:50:25Z'),
             'teamcity/dcos/build/aws/cloudformation/custom/strict': ('success',
                                                                      '2019-02-26T14:50:19Z'),
             'teamcity/dcos/build/dcos': ('success', '2019-02-26T14:46:30Z'),
             'teamcity/dcos/build/tox': ('success', '2019-02-26T14:35:16Z'),
             'teamcity/dcos/test/aws/onprem/static/strict/group/1': ('success',
                                                                     '2019-02-26T15:28:22Z'),
             'teamcity/dcos/test/aws/onprem/static/strict/group/2': ('success',
                                                                     '2019-02-26T15:55:55Z'),
             'teamcity/dcos/test/aws/onprem/static/strict/group/3': ('success',
                                                                     '2019-02-26T15:44:54Z'),
             'teamcity/dcos/test/aws/onprem/static/strict/group/4': ('success',
                                                                     '2019-02-26T15:49:55Z')},
 'state': 'success'}

d2iq-mergebot · 2019-02-26T17:40:29Z

@mesosphere-mergebot merge-it

d2iq-mergebot · 2019-02-26T17:41:30Z

Enterprise Bump PR mesosphere/dcos-enterprise/pull/4694, and the current PR #4549 are now merged.

d2iq-mergebot added the Work In Progress label Feb 19, 2019

mrnugget force-pushed the DCOS-46615_pkgpanda_content_length branch from 89fd184 to 5f70d07 Compare February 19, 2019 09:02

gpaul suggested changes Feb 20, 2019

View reviewed changes

adamtheturtle reviewed Feb 20, 2019

View reviewed changes