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

Release management for VGCN images #78

Closed
4 of 6 tasks
kysrpex opened this issue Oct 19, 2023 · 16 comments · Fixed by #80
Closed
4 of 6 tasks

Release management for VGCN images #78

kysrpex opened this issue Oct 19, 2023 · 16 comments · Fixed by #80
Assignees

Comments

@kysrpex
Copy link
Contributor

kysrpex commented Oct 19, 2023

Edited by @mira-miracoli

Tasks:

  • create python script using argparse (usegalaxy-eu/vgcn#80)
  • automatic requirements installation/check/remove with conda
  • publishing images to OpenStack and static/vgcn with flags
  • creates filenames the following way (see last comments): vgcn~<provisioning>~<os>~<date>~<seconds>~<branch>~<hash>~<comment>
  • tested on own Laptop and on Jenkins with <comment> = "test"
  • images tested for functioning with HTCondor

Original post from @kysrpex
At the moment, our publicly released images are named like this: "vggp-v60-j324-67edcc87f400-main" (see https://usegalaxy.eu/static/vgcn/ for more examples). Images uploaded to OpenStack have similar names.

The names are constructed from a build tag

# Construct build tag
GIT_COMMIT_SHORT=`git log --format="%H" -n 1 | sed -e 's/^\(.\{12\}\).*/\1/g'`
echo "GIT_COMMIT_SHORT=$GIT_COMMIT_SHORT"
VG_BUILD=`cat ansible-roles/group_vars/all.yml | grep '^vg_build:' | sed 's/vg_build: //g'`
echo "VG_BUILD=$VG_BUILD"
NICE_BRANCH=`git name-rev --name-only HEAD | sed 's|remotes/origin/||g'`
echo "NICE_BRANCH=$NICE_BRANCH"
# continue with the same build01 numbering
BN=`expr $BUILD_NUMBER + 205`
echo "BN=$BN"
BUILD_TAG="v$VG_BUILD-j$BN-$GIT_COMMIT_SHORT-$NICE_BRANCH"
echo "BUILD_TAG=$BUILD_TAG"

that is used to generate the release tag (the final name)

function release_tag {
    local flavour=$1
    local tag
    if [[ "$flavour" == "vgcn-bwcloud-gpu"* ]]; then
        tag="vggp-gpu-$BUILD_TAG"
    elif [[ "$flavour" == "vgcn-bwcloud-"* ]]; then
        tag="vggp-$BUILD_TAG"
    else
        tag="$flavour-$BUILD_TAG"
    fi
    echo $tag    
}

by calling the release_tag function using $flavour as an input.

release_tag $flavour

The good choices I see in our current approach are:

  • We include the commit hash in the image name.
  • We include the branch name in the image name.

The bad choices are:

  • We have defined some meaning for the version number (e.g. v60 includes features A and B), but it is not clear under which conditions this number should change and how should it change.
  • We have made the mistake of resetting the build number at some point, meaning that images with a higher build number are not necessarily more recent.
  • We do not include any hint of the operating system on which the images are based (e.g. Rocky Linux 9.2, AlmaLinux 8.8).

Neutral choices:

  • What does "vggp" mean?

The bad choices make it difficult to "compare" two image versions (determining which one is newer when it makes sense), or quickly inferring what is in them (fortunately the branch name and commit hash can still be used for this purpose).

We need a naming scheme (and possibly a simple branching model) that addresses those problems. Let's think of what information is needed or would be useful to put on a version tag for an image.

  • Commit date.
  • Commit time.
  • Git branch, assuming it conveys behavior differences from the main branch (e.g. internal images that connect to the secondary HTCondor cluster, or the patched GPU images that run a specific kernel version).
  • Provisioning (a.k.a. flavors).
  • Commit hash.
  • Operating system.
  • Extra information that conveys the image may still be different from what one would get building it from the git repository (e.g. local changes).

I think we could leave out:

  • Attempting to include a number (e.g. v60) that conveys the features of the image. We are failing at doing this extra effort already and thus not getting anything valuable out of it.
  • Build numbers. The "same image" (we are not aiming here at real reproducible builds) is supposed to be produced from the same commit hash.

A possible naming scheme could be vgcn-<date>-<seconds>-<branch>~<provisioning>~<os>~<hash>-<comment>, where:

  • Using vgcn could immediately help distinguish the old naming scheme from the new one (if there is not a good reason for the name vggp).
  • <date> is the commit date in YYYY-MM-DD format. For example: 2023-10-19.
  • <seconds> is the commit time measured in the number of seconds elapsed since the start of the commit date (i.e. since YYYY-MM-DDT00:00:00). For example: 58706.
  • <branch> is the name of the Git branch. Git branches should differ from main only in small details and be temporary, just for patchwork.
  • <provisioning> are the playbooks that have been used to provision the image. It could be a + separated list of playbook names, for example generic+workers+internal+gpu. Optionally we could allow the use of at most one alias (the string would start with +) to have shorter names and avoid confusing people. For example, +external could be an alias for generic+workers+external, and the alias could be combined with more playbooks, for example +external+gpu.
  • <os> is the name of the Packer build without the source (e.g. centos-8.5.2111-x86_64).
  • <hash> is the Git commit hash.
  • -<comment> is any extra string to identify deviations from the Git commit that was checked out at the moment of building the image. Jenkins would omit this part. If you build an image locally with any deviation from the checked out commit and share it with others, it would be nice if you include something here.

A sample name using this scheme could be vgcn-2023-10-19-58706-main~+internal~centos-8.5.2111~x86_64-687c70f-local_build_different_motd.

@kysrpex kysrpex self-assigned this Oct 19, 2023
@kysrpex
Copy link
Contributor Author

kysrpex commented Oct 19, 2023

@sanjaysrikakulam I think you were also interested in having the release names changed. Just pinging you so that you are aware that the issue exists.

@mira-miracoli
Copy link
Contributor

mira-miracoli commented Oct 27, 2023

Thanks for the detailed thoughts to this, I fully agree that it needs to change and I think I agree to most of what you said. Here are a few ideas I have:

  1. Should we maybe start with the OS name? That would give a better overview (assuming people want to get a specific OS as no 1 priority)
  2. Maybe that could be followed by the provisioning, because I think that might also be a high search priority.
    That would lead to:
vgcn-<OS>-<provisioning>-<date>-<seconds>-<branch>-<commit>-<comment>
I created an example list with an AI of my 'trust'
vgcn-centos-8.6-generic-2023-10-29-11111-development-abcdef
vgcn-centos-8.6-generic-2023-11-01-77777-development-yz1234
vgcn-centos-8.6-generic-2023-11-04-40404-development-ijklmn
vgcn-centos-8.6-gpu-2023-10-30-33333-main-ghijkl
vgcn-centos-8.6-gpu-2023-11-02-99999-main-567890
vgcn-centos-8.6-gpu-2023-11-04-30303-main-pqrstuv
vgcn-centos-8.6-internal-2023-10-28-54321-testing-mnopqr
vgcn-centos-8.6-internal-2023-10-31-55555-testing-stuvwx
vgcn-centos-8.6-internal-2023-11-02-88888-testing-abcdefgh
vgcn-centos-8.6-internal-2023-11-03-20202-testing-wxyz12
vgcn-centos-8.6-internal-2023-11-05-60606-main-345678
vgcn-centos-8.6-internal-2023-11-06-80808-testing-9abcde
vgcn-rocky-9.2-generic-2023-10-27-12345-development-abc123
vgcn-rocky-9.2-generic-2023-10-30-44444-development-jkl012
vgcn-rocky-9.2-generic-2023-11-03-10101-development-stu901
vgcn-rocky-9.2-gpu-2023-10-28-98765-main-def456
vgcn-rocky-9.2-gpu-2023-10-31-66666-main-mno345
vgcn-rocky-9.2-internal-2023-10-29-22222-testing-ghi789
vgcn-rocky-9.2-internal-2023-11-02-88888-testing-pqr678
vgcn-rocky-9.2-internal-2023-11-05-50505-testing-vwx234

@sanjaysrikakulam
Copy link
Member

I like the idea. I somehow missed the ping. Sorry!

The following would be nice from Mira's and yours; I think seconds is unnecessary.

vgcn-<OS>-<provisioning>-<date>-<branch>-<commit>-<comment>

This name would be cool, and we can add a section in the readme of VGCN explaining the same.

@mira-miracoli
Copy link
Contributor

Do we all agree on @sanjaysrikakulam 's version, or is it maybe better to keep the seconds so it is easier to select the latest build?

@kysrpex
Copy link
Contributor Author

kysrpex commented Nov 6, 2023

Do we all agree on @sanjaysrikakulam 's version, or is it maybe better to keep the seconds so it is easier to select the latest build?

I'd keep the seconds to easily select the latest build. I think that now leaves the decision up to you :)

  1. Should we maybe start with the OS name? That would give a better overview (assuming people want to get a specific OS as no 1 priority)
  2. Maybe that could be followed by the provisioning, because I think that might also be a high search priority.

After thinking it twice I would switch what I initially proposed to vgcn~<provisioning>~<os>~<date>~<seconds>~<branch>~<hash>~<comment>. Note that it gives higher priority to the OS name than to the date as you suggest. However, the provisioning may be of higher priority than the OS?

@mira-miracoli
Copy link
Contributor

mira-miracoli commented Nov 6, 2023

I think we can continue with this final version @kysrpex suggested
And I can start to implement that and do changes in @usegalaxy-eu/jenkins-scripts

@kysrpex
Copy link
Contributor Author

kysrpex commented Nov 6, 2023

Ok then now what remains is to change vgcn-build.sh to accomodate the changes.

@mira-miracoli mira-miracoli self-assigned this Nov 6, 2023
@mira-miracoli
Copy link
Contributor

Shall we create the timestamp before starting the build or after?

@kysrpex
Copy link
Contributor Author

kysrpex commented Nov 6, 2023

Shall we create the timestamp before starting the build or after?

The timestamp refers to the commit time. I assume that is why @sanjaysrikakulam objected, because you need to run some git commands to compute it (more work).

  • <seconds> is the commit time measured in the number of seconds elapsed since the start of the commit date (i.e. since YYYY-MM-DDT00:00:00). For example: 58706.

@mira-miracoli
Copy link
Contributor

I think that should do it:

SECONDS=`echo $(( $(git log --format=%ci -n 1 | cut -d ' ' -f2 | xargs -I {} \
    date -d "{}" +%s) - $(date +%s)))`

@mira-miracoli
Copy link
Contributor

Regarding our discussion if we should leave out generic and only mention it with a "-" prefix if it is missing – here is my proposition:

PROVISIONING_STR=`if [[ " $PROVISIONING " =~ .*\ generic\ .* ]]; \
    then echo "${PROVISIONING[*]/'generic'}" | tr ' ' '+' | \
    sed -e 's/\(+\)*//;s/+$//'; else echo "${PROVISIONING[*]}" | \
    tr ' ' '+' | sed -e 's/\(+\)*//;s/+$//' | awk '{print $1"-generic"}'; fi`

if generic is present it will be cut off, otherwise "-generic" is appended

@kysrpex
Copy link
Contributor Author

kysrpex commented Nov 6, 2023

Regarding our discussion if we should leave out generic and only mention it with a "-" prefix if it is missing – here is my proposition:

PROVISIONING_STR=`if [[ " $PROVISIONING " =~ .*\ generic\ .* ]]; \
    then echo "${PROVISIONING[*]/'generic'}" | tr ' ' '+' | \
    sed -e 's/\(+\)*//;s/+$//'; else echo "${PROVISIONING[*]}" | \
    tr ' ' '+' | sed -e 's/\(+\)*//;s/+$//' | awk '{print $1"-generic"}'; fi`

if generic is present it will be cut off, otherwise "-generic" is appended

That sounds good, I just thought that we need a different character from - for exclusion, because I think having dashes in playbook names is desirable. Maybe !generic is ok?

@mira-miracoli
Copy link
Contributor

or, the verbose way: literally say "+no-generic"

@kysrpex
Copy link
Contributor Author

kysrpex commented Nov 6, 2023

or, the verbose way: literally say "+no-generic"

I prefer !generic tbh.

@mira-miracoli
Copy link
Contributor

@kysrpex and I had a discussion about the Jekins build script and we had two outcomes:

  • it should be in the VGCN repo, in order to be able to do the release and upload progress without depending on Jenkins
  • it should be a Python script with command line arguments and help section

kysrpex added a commit to kysrpex/usegalaxy-eu-vgcn-infrastructure that referenced this issue Nov 16, 2023
Rename `htcondor-secondary` image to follow naming conventions from usegalaxy-eu/vgcn#78
@kysrpex kysrpex linked a pull request Dec 15, 2023 that will close this issue
@mira-miracoli
Copy link
Contributor

We need to change the column width here:
https://usegalaxy.eu/static/vgcn/

kysrpex added a commit to mira-miracoli/vgcn that referenced this issue Dec 20, 2023
Compute commit time in `assemble_timestamp` instead of build time as agreed in usegalaxy-eu#78.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants