Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rerun gpg recv-keys command multiple times #3544

Merged
merged 8 commits into from
Dec 1, 2023
Merged

Rerun gpg recv-keys command multiple times #3544

merged 8 commits into from
Dec 1, 2023

Conversation

adamfarley
Copy link
Contributor

@adamfarley adamfarley commented Nov 27, 2023 •
edited
Loading

That way, if a specific keyserver is slow and causes a timeout, we simply retry after a brief pause.

Fixes #3518

@adamfarley
Rerun gpg recv-keys command multiple times
8bf4627 
That way, if a specific keyserver is slow and causes a timeout,
we simply retry on a different server.

Signed-off-by: Adam Farley <[email protected]>
@adamfarley adamfarley self-assigned this Nov 27, 2023
@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites labels Nov 27, 2023
@adamfarley
Copy link
Contributor Author

Ok, build's still running, but it's passed the point where we might see a timeout, so at least we know this change doesn't break anything. Seeking reviews and approval.

@adamfarley adamfarley mentioned this pull request Nov 27, 2023
Closed
@jiekang
Copy link
Contributor

jiekang commented Nov 27, 2023

Is there an issue that describes the problem in more detail? There is also a change to the keyserver URL; can that choice be explained further?

@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites and removed testing Issues that enhance or fix our test suites alpine-linux Issues that affect or relate to the Alpine LINUX OS labels Nov 27, 2023
@adamfarley
Copy link
Contributor Author

adamfarley commented Nov 27, 2023 •
edited
Loading

Hi Jie. Yes, the issue is a timeout, mentioned here.

Also, I explained the change in keyserver via a comment in the change. This new keyserver is actually something that provides a different keyserver ip address each time you access it. That way, if we access a slow server, or a down server, we just loop back around and use a different server.

In theory anyway. For some reason this seems to work just fine on Alpine, mac, and Windows, but fails on all the other Linux flavours.

@adamfarley adamfarley changed the title Rerun gpg recv-keys command multiple times WIP: Rerun gpg recv-keys command multiple times Nov 27, 2023
@adamfarley
Letting the user know which server we're trying to reach
6254c6d 
For debugging purposes.

Signed-off-by: Adam Farley <[email protected]>
@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites and removed testing Issues that enhance or fix our test suites alpine-linux Issues that affect or relate to the Alpine LINUX OS labels Nov 27, 2023
@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites and removed testing Issues that enhance or fix our test suites alpine-linux Issues that affect or relate to the Alpine LINUX OS labels Nov 27, 2023
@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites and removed testing Issues that enhance or fix our test suites alpine-linux Issues that affect or relate to the Alpine LINUX OS labels Nov 27, 2023
@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites and removed alpine-linux Issues that affect or relate to the Alpine LINUX OS labels Nov 27, 2023
@adamfarley
Removing use of keys.gnupg.net
b74a8b8 
Only works on some platforms. The web page linked below suggests that
the network it depends on has been deprecated, and attempts to
ping the url directly have failed.

https://stackoverflow.com/questions/66217436/error-gpg-keyserver-
receive-failed-no-name/68132500#68132500

So I've replaced the url with an array of 3 alternatives.

Let's try iterating over those.

- Adam
@adamfarley adamfarley force-pushed the retry_keyserver_key_fetch branch from 8a68331 to b74a8b8 Compare November 27, 2023 16:42
@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites and removed testing Issues that enhance or fix our test suites alpine-linux Issues that affect or relate to the Alpine LINUX OS labels Nov 27, 2023
@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites and removed testing Issues that enhance or fix our test suites alpine-linux Issues that affect or relate to the Alpine LINUX OS labels Nov 27, 2023
@jiekang
Copy link
Contributor

jiekang commented Nov 27, 2023

Does it make sense to link the PR to the issue via one of the methods described here: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

In the future, it would be good to reference the issue if it exists in your initial PR comment so everyone can see it.

What was your source for your URL choice? When doing some preliminary searches online around GPG keyservers, there isn't too much 'official' looking documentation on active (and safe) keyservers to use...

When making a change like that, it would be good to provide justification for it.

@adamfarley
Copy link
Contributor Author

Hi Jie,

Does it make sense to link the PR to the issue via one of the methods described here: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

I considered doing this, but elected not to do this because I didn't want to automatically close the issue before Stewart had a chance to review the fix (on the off chance it got approved and merged yesterday). Plus, the last fix didn't solve the problem like we thought it would, so auto-close seemed unduly optimistic.

In the future, it would be good to reference the issue if it exists in your initial PR comment so everyone can see it.

Good point. Will do. :)

What was your source for your URL choice? When doing some preliminary searches online around GPG keyservers, there isn't too much 'official' looking documentation on active (and safe) keyservers to use...

This was my source, which isn't even slightly trustworthy, but I didn't think that mattered in this case. My understanding is that keyservers are not trustworthy sources of keys in and of themselves. This is why we specify a fingerprint when we use the gpg command, and we also verify the key upon receipt (see the gpg --verify command a few lines later).

When making a change like that, it would be good to provide justification for it.

That's fair. I will remember this for future PRs.

netomi
netomi reviewed Nov 28, 2023
View reviewed changes
sbin/prepareWorkspace.sh Outdated Show resolved Hide resolved
@adamfarley
Returning to a "Try it 10 times" design
e28b18b 
This allows us to hopefully provide some tolerance against the
timeout errors, while at the same time using the same keyserver;
fixing the problem while not affecting our risk profile.

Signed-off-by: Adam Farley <[email protected]>
@github-actions github-actions bot added alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites and removed testing Issues that enhance or fix our test suites alpine-linux Issues that affect or relate to the Alpine LINUX OS labels Dec 1, 2023
@adamfarley adamfarley enabled auto-merge (squash) December 1, 2023 13:29
@adamfarley adamfarley changed the title WIP: Rerun gpg recv-keys command multiple times Rerun gpg recv-keys command multiple times Dec 1, 2023
andrew-m-leonard
andrew-m-leonard approved these changes Dec 1, 2023
View reviewed changes
Copy link
Contributor

@andrew-m-leonard andrew-m-leonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

steelhead31
steelhead31 approved these changes Dec 1, 2023
View reviewed changes
Copy link
Contributor

@steelhead31 steelhead31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamfarley adamfarley merged commit da2408e into adoptium:master Dec 1, 2023
23 checks passed
@karianna karianna mentioned this pull request Jan 8, 2024
Merged
@adamfarley adamfarley deleted the retry_keyserver_key_fetch branch July 10, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@netomi netomi netomi left review comments

@steelhead31 steelhead31 steelhead31 approved these changes

@andrew-m-leonard andrew-m-leonard andrew-m-leonard approved these changes

Assignees

@adamfarley adamfarley

Labels
alpine-linux Issues that affect or relate to the Alpine LINUX OS testing Issues that enhance or fix our test suites
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

jdk11u Alpine linux build failure: gpg keyserver timeout
5 participants
@adamfarley @jiekang @netomi @andrew-m-leonard @steelhead31