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.

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

Support creating mashes from multiple tags #10

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

clefrks
Copy link

@clefrks clefrks commented Oct 9, 2019

Currently koji-setup-scripts only cater 1 tag for snapshot.
Added feature for multiple tag support.

Also removing hardcoded value for dist- in bootstrap.sh and deploy-mash.sh

KOJI_REPO_PATH="$(realpath "$KOJI_DIR/repos/$KOJI_TAG-build/latest")"
KOJI_BUILD_NUM="$(basename "$KOJI_REPO_PATH")"
if [[ "$MASH_BUILD_NUM" -ne "$KOJI_BUILD_NUM" ]]; then
rm -rf "$MASH_DIR_NEW"
mkdir -p "$MASH_DIR_NEW"
create_dist_repos "$MASH_DIR_NEW"
if [[ -e "$MASH_TRACKER_DIR" ]]; then
mv "$MASH_TRACKER_DIR" "$MASH_DIR_OLD"
Copy link
Member

Choose a reason for hiding this comment

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

Since $MASH_DIR_OLD needs to exist now as the move is moving it to a subdirectory rather than just renaming the directory to .old I think this needs a mkdir -p $MASH_DIR_OLD.

Copy link
Author

Choose a reason for hiding this comment

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

ah yes! i will update that and $MASH_TRACKER_DIR!
thanks!

fi
mv "$MASH_DIR_NEW" "$MASH_TRACKER_DIR"
mv "$MASH_DIR_NEW/$KOJI_TAG" "$MASH_TRACKER_DIR"
Copy link
Member

Choose a reason for hiding this comment

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

Similarly with $MASH_TRACKER_DIR being used as a subdir, the script will need to ensure it is created first.

@clefrks clefrks requested a review from bryteise October 17, 2019 07:11
@gtkramer
Copy link
Contributor

gtkramer commented Oct 24, 2019

@clefrks we've had a number of changes go in recently that remove our usage of the mash tool and make the replacement script faster. It was the last python2 dependency we had in the DevOps flow this repo creates. Would you be able to rebase please? It looks like you have some good changes that I'd like to get in.

Currently snapshot aka mash is using clear as default value for repo
name.

With this changes, it'll follow the TAG_NAME that users set in
parameter.sh to create mash for the tags.

Signed-off-by: candrew <[email protected]>
Instead of hardcoding dist-, just add into $TAG_NAME in parameter.sh

Signed-off-by: candrew <[email protected]>
Ensure the creation of the folders before moving the collaterals into
the folders.

Signed-off-by: candrew <[email protected]>
@clefrks
Copy link
Author

clefrks commented Oct 25, 2019

@gtkramer i've done rebasing, please review thanks!

@@ -46,19 +46,19 @@ EOF
mkdir -p "$MASH_SCRIPT_DIR"
cp -f "$SCRIPT_DIR"/mash.sh "$MASH_SCRIPT_DIR"
mkdir -p /etc/systemd/system
cat > /etc/systemd/system/mash.service <<- EOF
cat > /etc/systemd/system/mash@$TAG_NAME.service <<- EOF
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for a case like this, we want the service file on disk to be named [email protected] to indicate to systemd that we want to pass a parameter, but not hard-code the parameter for all instantiations of the service.

Comment on lines 11 to +12
MASH_DIR="${MASH_DIR:-/srv/mash}"
MASH_TRACKER_FILE="$MASH_DIR"/latest-mash-build
MASH_TRACKER_FILE="$MASH_DIR"/latest-${KOJI_TAG}-build
Copy link
Contributor

@gtkramer gtkramer Oct 25, 2019

Choose a reason for hiding this comment

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

If I am understanding the intent of the PR correctly, I think we want

MASH_DIR="${MASH_DIR:-"/srv/mash/$KOJI_TAG"}" 

Then we wouldn't modify MASH_TRACKER_FILE, but rather, self-contain a similar structure to every mash in the name of a folder that varies by tag. This allows for more fine-grained management of each repo and allows multiple repos for each tag to build up independently, if so desired.

Comment on lines -40 to +52
make_repo "${source_dir}" "${output_dir}" "clear/${BUILD_ARCH}/os" "Packages" "${bin_rpm_paths}" "${comps_file}" &
make_repo "${source_dir}" "${output_dir}" "clear/${BUILD_ARCH}/debug" "." "${debuginfo_rpm_paths}" &
make_repo "${source_dir}" "${output_dir}" "clear/source/SRPMS" "." "${src_rpm_paths}" &
make_repo "${source_dir}" "${output_dir}" "${KOJI_TAG}/${BUILD_ARCH}/os" "Packages" "${bin_rpm_paths}" "${comps_file}" &
make_repo "${source_dir}" "${output_dir}" "${KOJI_TAG}/${BUILD_ARCH}/debug" "." "${debuginfo_rpm_paths}" &
make_repo "${source_dir}" "${output_dir}" "${KOJI_TAG}/source/SRPMS" "." "${src_rpm_paths}" &
wait

create_dnf_conf "${work_dir}/dnf-os.conf" "${output_dir}/clear/${BUILD_ARCH}/os" clear-os
create_dnf_conf "${work_dir}/dnf-debug.conf" "${output_dir}/clear/${BUILD_ARCH}/debug" clear-debug
create_dnf_conf "${work_dir}/dnf-SRPMS.conf" "${output_dir}/clear/source/SRPMS" clear-SRPMS
create_dnf_conf "${work_dir}/dnf-os.conf" "${output_dir}/${KOJI_TAG}/${BUILD_ARCH}/os" clear-os
create_dnf_conf "${work_dir}/dnf-debug.conf" "${output_dir}/${KOJI_TAG}/${BUILD_ARCH}/debug" clear-debug
create_dnf_conf "${work_dir}/dnf-SRPMS.conf" "${output_dir}/${KOJI_TAG}/source/SRPMS" clear-SRPMS

write_packages_file "${work_dir}/dnf-os.conf" "$output_dir/clear/$BUILD_ARCH/packages-os"
write_packages_file "${work_dir}/dnf-debug.conf" "$output_dir/clear/$BUILD_ARCH/packages-debug"
write_packages_file "${work_dir}/dnf-SRPMS.conf" "$output_dir/clear/source/packages-SRPMS"
write_packages_file "${work_dir}/dnf-os.conf" "$output_dir/${KOJI_TAG}/$BUILD_ARCH/packages-os"
write_packages_file "${work_dir}/dnf-debug.conf" "$output_dir/${KOJI_TAG}/$BUILD_ARCH/packages-debug"
write_packages_file "${work_dir}/dnf-SRPMS.conf" "$output_dir/${KOJI_TAG}/source/packages-SRPMS"
Copy link
Contributor

@gtkramer gtkramer Oct 25, 2019

Choose a reason for hiding this comment

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

For these function calls, we unfortunately need to keep the clear/ structure. For better or worse, a lot of our processes today depend on this naming convention. I don't think the change to these lines would be needed if we contain the mash structure in a root folder name that varies by tag.

@@ -5,10 +5,11 @@
set -e
. /etc/profile.d/proxy.sh || :

KOJI_TAG="${KOJI_TAG:-"$1"}"
Copy link
Contributor

Choose a reason for hiding this comment

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

"${1:?"Missing tag parameter"}"

Is probably a more accurate parameter expansion here. If the service is enabled without a parameter, we want to indicate that this is required and have the script fail.

Comment on lines -109 to +116
if [[ -e "$MASH_TRACKER_DIR" ]]; then
mv "$MASH_TRACKER_DIR" "$MASH_DIR_OLD"
if [[ -e "$MASH_TRACKER_DIR/$KOJI_TAG" ]]; then
mkdir -p "$MASH_DIR_OLD"
mv "$MASH_TRACKER_DIR/$KOJI_TAG" "$MASH_DIR_OLD/$KOJI_TAG"
fi
mv "$MASH_DIR_NEW" "$MASH_TRACKER_DIR"
mkdir -p "$MASH_TRACKER_DIR"
mv "$MASH_DIR_NEW/$KOJI_TAG" "$MASH_TRACKER_DIR"
rm -rf "$MASH_DIR_OLD"
rm -rf "$MASH_DIR_NEW"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think with the change to the root folder, none of these changes are needed?

@gtkramer
Copy link
Contributor

gtkramer commented Oct 25, 2019

Moving the change to support multiple tags to more of a root directory for each mash structure, we would also need to modify parameters.sh. The following changes would need made:

export TAG_NAME=dist-clear
export MASH_DIR="/srv/mash/$TAG_NAME"

To mirror what is in mash.sh.

@gtkramer
Copy link
Contributor

gtkramer commented Oct 25, 2019

@clefrks Would these suggestions work for you?

@gtkramer gtkramer changed the title Support multiple snapshot(mash) Support creating mashes from multiple tags Oct 25, 2019
Sign up for free to 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.

3 participants