Bug 1840343: pkg/verify: Parallelize HTTP(S) signature retrieval #393
Conversation
openshift-ci-robot
added
the
bugzilla/severity-medium
|
@wking: This pull request references Bugzilla bug 1840343, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
bugzilla/valid-bug
openshift-ci-robot
added
the
approved
This consolidates around abstract signature retrieval, removing the split between stores and locations which we grew in 9bbf366 (verify: Refactor the verify package to have common code, 2019-12-09, openshift#279). This will make it easier to make future changes like parallel HTTP(S) signature retrieval retrieval [1]. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1840343
Fixing [1,2]: $ grep 'unable to load signature' cvo.log I0526 13:13:36.123153 1 verify.go:404] unable to load signature: Get https://storage.googleapis.com/openshift-release/official/signatures/openshift/release/sha256=baa687f29b0ac155d8f4c6914056d36d68f343feb9c1e82b46eef95819d00be5/signature-1: dial tcp 172.217.4.48:443: connect: connection timed out I0526 13:15:47.195128 1 verify.go:404] unable to load signature: Get https://storage.googleapis.com/openshift-release/official/signatures/openshift/release/sha256=baa687f29b0ac155d8f4c6914056d36d68f343feb9c1e82b46eef95819d00be5/signature-2: dial tcp 172.217.9.80:443: connect: connection timed out I0526 13:17:10.718027 1 verify.go:404] unable to load signature: Get https://storage.googleapis.com/openshift-release/official/signatures/openshift/release/sha256=baa687f29b0ac155d8f4c6914056d36d68f343feb9c1e82b46eef95819d00be5/signature-3: context deadline exceeded I0526 13:19:44.764143 1 verify.go:404] unable to load signature: Get https://storage.googleapis.com/openshift-release/official/signatures/openshift/release/sha256=baa687f29b0ac155d8f4c6914056d36d68f343feb9c1e82b46eef95819d00be5/signature-1: dial tcp 172.217.4.48:443: connect: connection timed out I0526 13:21:55.835063 1 verify.go:404] unable to load signature: Get https://storage.googleapis.com/openshift-release/official/signatures/openshift/release/sha256=baa687f29b0ac155d8f4c6914056d36d68f343feb9c1e82b46eef95819d00be5/signature-2: dial tcp 172.217.4.48:443: connect: connection timed out I0526 13:23:18.233801 1 verify.go:404] unable to load signature: Get https://storage.googleapis.com/openshift-release/official/signatures/openshift/release/sha256=baa687f29b0ac155d8f4c6914056d36d68f343feb9c1e82b46eef95819d00be5/signature-3: context deadline exceeded In this case, that was because a restricted network blocked access to storage.googleapis.com, but you'd get similar behavior if storage.googleapis.com itself was slow. We want to hit other signature sources (like our mirrors [3]) before giving up on signatures entirely, and with this commit one HTTP(S) store no longer blocks another. The local ConfigMap store still comes first with a serial store, because we don't want to involve external stores before we've exhausted that local-cluster store. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1840343 [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1838497#c10 [3]: https://github.com/openshift/cluster-update-keys/blob/cca4ce696383e70ae669e770bd63265a9540b721/manifests.rhel/0000_90_cluster-update-keys_configmap.yaml#L5
wking
force-pushed
the
parallel-http-signature-stores
branch
from
0aab8dc to
a068ee3
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jottofar, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@wking: All pull requests linked via external trackers have merged: openshift/cluster-version-operator#393. Bugzilla bug 1840343 has been moved to the MODIFIED state. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cherry-pick release-4.5 |
|
@wking: new pull request created: #487 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
5 participants
Builds on top of #294. You can review that one first if you like, or review both commits in this PR (and GitHub will mark #294 merged if this one lands first, because it will pull #294's tip into the target
masterbranch). With #294, the change we need to make to parallelize HTTP(S) retrieval is pretty small. I'm just adding a parallel store implementation and using that instead of the serial wrapper for HTTP(S) retrieval.