Skip to content

HBASE-23339 Release scripts should use forwarded gpg-agent - #1620

Closed
busbey wants to merge 5 commits into
apache:masterfrom
busbey:HBASE-23339
Closed

HBASE-23339 Release scripts should use forwarded gpg-agent#1620
busbey wants to merge 5 commits into
apache:masterfrom
busbey:HBASE-23339

Conversation

@busbey

@busbeybusbey commented May 1, 2020

Copy link
Copy Markdown
Contributor

This gets through full builds now both on a local mac with docker desktop and on a remote GCE debian box.

  • put together a docker container that can use host gpg-agent forwarded over ssh.
  • use gpg-agent forwarding container on OS X and directly forward the agent on Linux
  • clean up the release container on exit
  • use docker mounts instead of the deprecated volume syntax
  • use image names within our project namespace
  • update README for gce builds to include forwarding gpg-agent to VM such that it can be forwarded to docker on the VM.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@saintstacksaintstack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Man. The amount of pain involved getting to here I can only imagine. Masochist!


cd "$WORKDIR"
rm -rf "$WORKDIR/output"
rm -rf "${WORKDIR}/gpg-proxy.ssh.pid" "${WORKDIR}/gpg-proxy.cid" "${WORKDIR}/release.cid"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move to a trap/signal handler? Follow-on.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All these clean ups should be in a cleanup handler... Not your issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hows this relate to the below nice cleanup function?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

so the clean up function should take care of them, but something can go wrong such that they don't get handled.

since docker will fail loudly if the passed container id file path exists, this ensures we have a smooth path back to running again even if something goes wrong.

also the latest version of the WIP has an option to purposefully leave those files in place at the end of execution in case someone needs to debug what the containers are doing.

Comment threaddev-support/create-release/do-release-docker.sh
Comment threaddev-support/create-release/release-build.sh Outdated
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@busbey

Copy link
Copy Markdown
ContributorAuthor

sorry for the lag here. missed that I had comments.

@busbey

Copy link
Copy Markdown
ContributorAuthor

shellcheck and whitespace complaints should be easy enough to clean up. will get to it after I get PRs up for all the commits on the branch that are not this particular jira.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@busbeybusbey changed the title HBASE-23339 Release scripts should not need to write out a copy of gpg key material - WIP do not mergeHBASE-23339 Release scripts should use forwarded gpg-agentMay 23, 2020
@busbey

Copy link
Copy Markdown
ContributorAuthor

I think this is ready now.

@mattf-apache

mattf-apache commented Jun 14, 2020

Copy link
Copy Markdown
Member

If you've set up your gpg agent and unlocked your private key for it then you do not need to give the passphrase during the build. the README goes over increasing the timeout to a day instead of 2 hours.

Nice, I missed that on my earlier read-thru. Thanks.

yes your gpg agent has to be running on your local machine before hand. ...
I could add an example of forcing a clean start of the gpg-agent, e.g. gpgconf --kill all && gpg-connect-agent /bye
The test signature at container launch should ensure the key is unlocked prior to us building artifacts.

I recommend adding that example in the README. I think most people do not have gpg-agent running by default, may not know how to launch it (I was vague on it), and while it is true that the test signature at container launch will catch a missing agent timely, it won't tell the user how to fix the problem. -- So, maybe also have that test sig command catch failure and give an instructive error message. Please?

@ndimiduk

Copy link
Copy Markdown
Member

I was able to use this patch over the weekend to build an rc as dry run. all the gpg passthrough worked well for me.

@ndimiduk

Copy link
Copy Markdown
Member

@busbey@mattf-apache I'm trying to use this to create an RC for real, using -r .git. Does this mean anything to you?

$ cat ~/build-rc0/output/tag.log
...
21:57:58 [INFO] ------------------------------------------------------------------------
21:57:58 [INFO] BUILD SUCCESS
21:57:58 [INFO] ------------------------------------------------------------------------
21:57:58 [INFO] Total time: 5.609 s
21:57:58 [INFO] Finished at: 2020-06-15T21:57:58Z
21:57:58 [INFO] ------------------------------------------------------------------------
ESC[0m
+ git commit -a -m 'Preparing development version 2.3.1-SNAPSHOT'
error: object directory /home/vagrant/repos/hbase/.git/objects does not exist; check .git/objects/info/alternates.
error: object directory /home/vagrant/repos/hbase/.git/objects does not exist; check .git/objects/info/alternates.

@ndimiduk

Copy link
Copy Markdown
Member

Using it without -r doesn't work either.

Cloning into 'hbase'...
fatal: unable to access 'https://ndimiduk:hXXXXXXXXX@gitbox.apache.org/repos/asf/hbase.git/': Port number ended with 'h'

@mattf-apache

mattf-apache commented Jun 16, 2020

Copy link
Copy Markdown
Member

@ndimiduk@busbey , both the creation of the -r option and the use of shared objects while cloning local repo, were part of commit a9fefd7f533c of PR #1725 (HBASE-24297 release scripts should be able to use an existing project clone). I didn't test with it, sorry.

Parenthetically, I am confused by the -r .git idiom. If I understand the GIT_REPO usage properly, it needs to point at an actual repository, while .git is the usual name of the git account metadata directory. Why would I point -r at it? The processing code doesn't shed any light for me.

@mattf-apache

Copy link
Copy Markdown
Member

Similarly, regarding the failure without -r, I don't have insights. The code in release-utils.sh:git_clone_overwrite() is supposed to url-encode the ASF_PASSWORD, and it was inherited from prior code that worked in-situ before PR #1725 . See https://github.pie.apple.com/IPR/apache-hbase/blob/8ff8e70edfbb50967cf123e6afdd9cc5dcfa4878/dev-support/create-release/release-build.sh#L116-L119

@ndimiduk

ndimiduk commented Jun 16, 2020

Copy link
Copy Markdown
Member

Similarly, regarding the failure without -r, I don't have insights. The code in release-utils.sh:git_clone_overwrite() is supposed to url-encode the ASF_PASSWORD, and it was inherited from prior code that worked in-situ before PR #1725 . See https://github.pie.apple.com/IPR/apache-hbase/blob/8ff8e70edfbb50967cf123e6afdd9cc5dcfa4878/dev-support/create-release/release-build.sh#L116-L119

I tracked this down to a character that the library helpfully does not encode by default. See PR#1907.

@mattf-apache

Copy link
Copy Markdown
Member

Righto. Nice catch.

@busbey

Copy link
Copy Markdown
ContributorAuthor

error: object directory /home/vagrant/repos/hbase/.git/objects does not exist; check .git/objects/info/alternates.

the object directory messages are a side effect of using the shared objects. It shouldn't be listed as an error since git then immediately checks the alternates we provide and finds what it needs.

Parenthetically, I am confused by the -r .git idiom. If I understand the GIT_REPO usage properly, it needs to point at an actual repository, while .git is the usual name of the git account metadata directory. Why would I point -r at it? The processing code doesn't shed any light for me.

the .git metadata directory is itself a repository, albeit one only on the local system. using it for a release candidate means the tag generated will be in that local repository. If the RM wants the tag published somewhere else then they need to push it to the other remote.

@mattf-apache

Copy link
Copy Markdown
Member

Thanks for the explanation.

@ndimiduk

Copy link
Copy Markdown
Member

the object directory messages are a side effect of using the shared objects. It shouldn't be listed as an error since git then immediately checks the alternates we provide and finds what it needs.

I think the build failed for me due to this error. Will try it again for RC1.

busbeyand others added 3 commits June 24, 2020 00:35
… container.
* put together a docker container that can use host gpg-agent forwarded over ssh.
* use gpg-agent forwarding container on OS X and directly forward the agent on Linux
* clean up the release container on exit
* use docker mounts instead of the deprecated volume syntax
* use image names within our project namespace
@busbey

Copy link
Copy Markdown
ContributorAuthor

force push was just me rebasing to the current HEAD with the changes as they were. the next commit is a minor issue I found while doing non-docker builds.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@busbey

Copy link
Copy Markdown
ContributorAuthor

yes your gpg agent has to be running on your local machine before hand. ...
I could add an example of forcing a clean start of the gpg-agent, e.g. gpgconf --kill all && gpg-connect-agent /bye
The test signature at container launch should ensure the key is unlocked prior to us building artifacts.

I recommend adding that example in the README. I think most people do not have gpg-agent running by default, may not know how to launch it (I was vague on it), and while it is true that the test signature at container launch will catch a missing agent timely, it won't tell the user how to fix the problem. -- So, maybe also have that test sig command catch failure and give an instructive error message. Please?

@mattf-apache latest commit adds in an example in the README and some error handling to point to it.

@busbey

Copy link
Copy Markdown
ContributorAuthor

these two additional commits are both minor tweaks. @ndimiduk and @mattf-apache please shout if you'd like to give further feedback. barring that I'll go ahead with merging after the qabot comes back and I get time to squash+merge.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 37sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 8sMaven dependency ordering for branch
_ Patch Compile Tests _
+0 🆗mvndep0m 8sMaven dependency ordering for patch
_ Other Tests _
1m 46s
SubsystemReport/Notes
DockerClient=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1620/10/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#1620
Optional Tests
unameLinux 74f5e59a6074 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / b556343
Max. process+thread count45 (vs. ulimit of 12500)
modulesC: U:
Console outputhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1620/10/console
versionsgit=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f)
Powered byApache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 32sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗shelldocs0m 0sShelldocs was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗mvndep0m 22sMaven dependency ordering for branch
_ Patch Compile Tests _
+0 🆗mvndep0m 8sMaven dependency ordering for patch
+1 💚hadolint0m 3sThere were no new hadolint issues.
+1 💚shellcheck0m 3sThere were no new shellcheck issues.
+1 💚whitespace0m 0sThe patch has no whitespace issues.
_ Other Tests _
+0 🆗asflicense0m 0sASF License check generated no output?
2m 13s
SubsystemReport/Notes
DockerClient=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1620/10/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1620
Optional Testsdupname asflicense shellcheck shelldocs hadolint
unameLinux 612a9920d620 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / b556343
Max. process+thread count50 (vs. ulimit of 12500)
modulesC: U:
Console outputhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1620/10/console
versionsgit=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) shellcheck=0.4.6 hadolint=1.17.5-0-g443423c
Powered byApache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec2m 16sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 8sMaven dependency ordering for branch
_ Patch Compile Tests _
+0 🆗mvndep0m 8sMaven dependency ordering for patch
_ Other Tests _
3m 25s
SubsystemReport/Notes
DockerClient=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1620/10/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#1620
Optional Tests
unameLinux 24983de37e6a 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / b556343
Max. process+thread count48 (vs. ulimit of 12500)
modulesC: U:
Console outputhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1620/10/console
versionsgit=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f)
Powered byApache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

@busbey
busbey deleted the HBASE-23339 branch June 25, 2020 07:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@busbey@Apache-HBase@mattf-apache@ndimiduk@saintstack@apurtell