-
Notifications
You must be signed in to change notification settings - Fork 14
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.
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
Adding the generic_package.sh script file #44
base: main
Are you sure you want to change the base?
Conversation
This script file is used to build the rpms for Debian and Ubuntu versions. We can run this script on a ubuntu machine. We create a chroot environment using pbuilder create. This gives us the environment for both ubuntu and debian. On this we build, sign and push the packages. We update the changelogs and push them to the glusterfs-debian repo as well. Signed-off-by: Sheetal Pamecha <[email protected]> Signed-off-by: hari gowtham <[email protected]> Signed-off-by: Shwetha K Acharya <[email protected]>
This helps in automating the packaging task of Gluster for Debian and Ubuntu. Change-Id: Icdbc718f0a3f020715d959486d01e962f89fa80f Signed-off-by: hari gowtham <[email protected]> Signed-off-by: Shwetha K Acharya <[email protected]>
- string: | ||
default: | ||
description: Release number for the package to be built against. | ||
Leave it empty if you are building above series 5. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mhh, what is series 5, you mean version 5 branch ? is it still supported, or can we drp it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can drop them off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the comment is still here, what is series 5 ? Do you mean "release 5" ? (and if we can drop, should it be removed from the description ?)
../scripts/generic_package.sh ubuntu focal $SERIES $VERSION $RELEASE | ||
elif [ "$OS" == "debian" ]; then | ||
echo "packing debian alone" | ||
if [ "$FLAVOR" == "stretch" ] || [ "$FLAVOR" == "9" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be cleaner to have 1 single if with the 6 possibles values than 3 separate ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please elaborate this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, here we have 3 lines with if, they all run the same script with the same argument, the only difference is the echo. This seems harder to read while :
case $FLAVOR in
"stretch" | "9" | "buster" | "10" | "bullseye" | "11")
../scripts/generic_package.sh $OS $FLAVOR $SERIES $VERSION $RELEASE $LATEST_SERIES $LATEST_VERSION
is shorter
tar czf ~/${os}-${flavor}-Glusterfs-${version}/${flavor}-apt-amd64-${version}.tgz pool/ dists/ | ||
|
||
echo "Pushing Changelog changes.." | ||
git push origin ${flavor}-${series}-local:${flavor}-glusterfs-${series} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not going to work if the clone is done over HTTP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sorry, but I still see https for the clone. I was not precise enough, I want to point that we can't push over http, including https. And if we need to push a new changelog, we need a ssh key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clone is now changed to ssh
sudo pbuilder build ~/${os}-${flavor}-Glusterfs-${version}/build/glusterfs_${version}-${release}.dsc | tee build.log | ||
|
||
#move the packages to packages directory. | ||
mv /var/cache/pbuilder/result/glusterfs*${version}-${release}*.deb ~/${os}-${flavor}-Glusterfs-${version}/packages/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure jenkins used can erase file in /var/cache , so mv would fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kalebskeithley Any suggestions on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can safely replaced by "cp", at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to still be mv :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But these files should be removed to avoid eventual filling up of space, How is such case handled generally with jenkins? @mscherer
echo "Uploading the packages.." | ||
if [ "$os" == "ubuntu" ]; then | ||
cd .. | ||
dput ppa:gluster/glusterfs-${series} glusterfs_${version}-${os}1~${flavor}1_source.changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it requires some authentication ? I do not see how it is done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, AIFIK. @kalebskeithley any more insights?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, if could be without authentication, but then, we need some kind of signature, cause I do not think anyone can push debian package for us, no ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we would need to follow https://help.launchpad.net/Packaging/PPA/Uploading to get required authentiacation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation do not tell much, but if that use ssh, then we need to have the key available to the builder, which is not declared in the job yaml file. There is example here: https://github.com/gluster/build-jobs/blob/master/build-gluster-org/jobs/centos7-regression.yml#L64 for adding a credential
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OpenPGP keys are used for signing as described here
The above link also shows how a new key can be created and added to launcpad account.
$ gpg --list-keys on the machine rhs-vm-17.storage-dev.lab.eng.bos.redhat.com under the user glusterpackager, shows the already existing keys. (documented the same in the mojo doc draft: let me know if any more information is needed to be documented)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that's a internal VM, and Jenkins is hosted outside of the lan. I may miss something obvious, but Jenkins can't connect to that server, and I think Product Security would strongly dislike that a external server (build.gluster.org) is able to remotely execute any code on a internal system ( rhs-vm-17.storage-dev.lab.eng.bos.redhat.com ).
So we need to have the key as secret in the job, and store that in Jenkins. We can't use RH internal system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we can store the key as secret in the job. We can add it in jenkins machine, I can help in getting it from rhs-vm-17.storage-dev.lab.eng.bos.redhat.com
|
||
echo "Building source package.." | ||
cd ../glusterfs-${version} | ||
debuild -S -sa -k${debuild_key} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where would the signing key stored ? (since -k requires that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debuild_key variable contains the key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't clear on my question. From what I understand, -k is the key identifier. But the actual private key is somewhere else (I think in ~/.gnupg), and so we need to discuss how that part is going to be managed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are initialising the values for debuild keys in https://github.com/gluster/build-jobs/pull/44/files/b32e0625d90b8c89432e753aab502d5e72b64b46#diff-02d2c32f5282eea2a7412f831420ae8c2f55ce0ed17671f6878e110c258b2e6bR29
The key creation is something we have maintained in a particular machine, whose access is not given to wider audience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then, where is that machine ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the draft of mojo doc that contains machine details: https://source.redhat.com/.motion/communitiesatredhat/crosscuttingco/pjmcop/prodev/study_grp/study_group_sign_up/A883E22D-0D6C-EB11-80F2-000D3A020FEB/.object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Page does not exist :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
cd /var/www/repos/apt/debian/ | ||
|
||
rm -rf pool/* dists/* db/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure jenkins can erase file there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can have a trail run to verify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if we create file here, shouldn't it be cleaned with a trap, like the rest ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, can be done
This helps in automating the packaging task of Gluster for Debian and Ubuntu. Change-Id: Icdbc718f0a3f020715d959486d01e962f89fa80f Signed-off-by: hari gowtham <[email protected]> Signed-off-by: Shwetha K Acharya <[email protected]>
This helps in automating the packaging task of Gluster for Debian and Ubuntu. Change-Id: Icdbc718f0a3f020715d959486d01e962f89fa80f Signed-off-by: hari gowtham <[email protected]> Signed-off-by: Shwetha K Acharya <[email protected]>
update mv to cp Signed-off-by: Shwetha K Acharya <[email protected]>
Signed-off-by: Shwetha K Acharya <[email protected]>
Signed-off-by: Shwetha K Acharya <[email protected]>
Signed-off-by: Shwetha K Acharya <[email protected]>
Requesting next round of review @mscherer |
#removing folders created while packaging | ||
rm -rf ~/${os}-${flavor}-Glusterfs-${version} | ||
} | ||
trap finish EXIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Traps should be placed at the start of the script. Otherwise, it be used only when the script is over, which is not useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
#copy the tar.gz file produced by the build to download.rht.gluster.org:/var/www/scratch | ||
scp $flavor-apt-amd64-$version.tgz [email protected]:/var/www/scratch | ||
|
||
ssh [email protected] /var/www/html/pub/gluster/unpacking-script.sh series version os flavor latest_version latest_series |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be "$version", etc ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
#move the packages to packages directory. | ||
cp /var/cache/pbuilder/result/glusterfs*${version}-${release}*.deb ~/${os}-${flavor}-Glusterfs-${version}/packages/ | ||
rm -rf /var/cache/pbuilder/result/glusterfs*${version}-${release}*.deb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not mv instead of cp + rm ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cp /var/cache/pbuilder/result/glusterfs*${version}-${release}*.deb ~/${os}-${flavor}-Glusterfs-${version}/packages/ | ||
rm -rf /var/cache/pbuilder/result/glusterfs*${version}-${release}*.deb | ||
|
||
if [ "$flavor" != "stretch" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, why is stretch special here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is because /var/cache/pbuilder/result/libg*${version}-${release}*.deb are not created only in stretch. In buster and bullseye, they will be created
replace cp+rm with mv add misisng $ Signed-off-by: Shwetha K Acharya <[email protected]>
Signed-off-by: Shwetha K Acharya <[email protected]>
Signed-off-by: Shwetha K Acharya <[email protected]>
Signed-off-by: Shwetha K Acharya <[email protected]>
@mscherer all the requested change sets are uploaded. |
So, since the review is getting a bit messy with comments, the blocking part is the gpg key secret integration for now. |
(and conflicts) |
name: ANNOUNCE_EMAIL | ||
|
||
builders: | ||
- shell: /opt/qa/debian-ubuntu-package.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Location is not correct. You have added the file in build-gluster-org/scripts/debian-ubuntu-package.sh
according to the actual loaction of the script Signed-off-by: Shwetha K Acharya <[email protected]>
echo "building everything" | ||
echo "packing debian distribution" | ||
for i in ${!deb_flavors[@]}; do | ||
~/build-gluster-org/scripts/generic_package.sh debian ${deb_flavors[$i]} $SERIES $VERSION $RELEASE $LATEST_SERIES $LATEST_VERSION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This location is wrong here. It should be ${WORKSPACE}/build-gluster-org/scripts/generic_package.sh
Signed-off-by: Shwetha K Acharya <[email protected]>
Signed-off-by: Shwetha K Acharya <[email protected]>
Action plan to resolve gpg key integration part: https://docs.google.com/document/d/1KRuzMC9zPD6K-5k03_5vUGjWrR0kUFiN2UhaYm18phQ/edit?usp=sharing Also as agreed earlier, infra team is upposed to create the following users for these scripts to work:
|
@@ -0,0 +1,57 @@ | |||
- job: | |||
name: debian-package-builder | |||
node: master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the label debian10
for this job. We have a machine existing on jenkins https://build.gluster.org/computer/builder-deb10-1.int.rht.gluster.org/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deepshikhaaa done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deepshikhaaa please validate the liburing-devel support as well.
https://build.gluster.org/computer/builder-deb10-1.int.rht.gluster.org/ Signed-off-by: Shwetha K Acharya <[email protected]>
We need a machaine with kernel 5.1 or above inorder to enable io-uring support with the builds. Please let me know which node can be used. As mentioned earlier there is an Action plan to resolve gpg key integration part. @mscherer @deepshikhaaa can we take this forward? |
We only have Debian 10 hosts, not Ubuntu one. I can try to spin a Ubuntu one but that would be the 1st we have, so I am unsure how long it will take. |
Ok so the Ubuntu installer do not work (and I do not understand why yet, there is no obvious error message). But so, do we need the kernel to be up to date for package building ? Given this will use a pbuilder, the headers would be there and nothing more should be needed, unless I am missing something ? |
So the Ubuntu installer didn't work because:
So I am doing 18.04 and upgrade manually. |
Right, in this script the latest chroot will be created. We would require a ubuntu mechine as discussed offline as it is tested in ubuntu. |
But testing is done on a different job than the packaging one, no ? I still do not get the issue, at what point is the deb packages tested, and how ? |
So we have 20.04 builder now: https://build.gluster.org/computer/builder-ubuntu2004-1.int.rht.gluster.org/ |
(https://build.gluster.org/computer/builder-ubuntu2004-1.int.rht.gluster.org/) Signed-off-by: Shwetha K Acharya <[email protected]>
This script file is used to build the rpms for
Debian and Ubuntu versions.
We can run this script on a ubuntu machine.
We create a chroot environment using pbuilder create.
This gives us the environment for both ubuntu and debian.
On this we build, sign and push the packages.
We update the changelogs and push them to the glusterfs-debian
repo as well.
Signed-off-by: Sheetal Pamecha [email protected]
Signed-off-by: hari gowtham [email protected]
Signed-off-by: Shwetha K Acharya [email protected]