Use git sha1 instead of counter tagging

[RuoqingHe]

Summary of the PR

• docker.sh: Reuse code as much as possible

• docker.sh: Replace counter tagging with git sha1:

  We were using vXX counter tagging till v49 for x86_64 and aarch64,
  and v49-riscv for riscv64. And we have separated riscv64 from the
  other two to another job due to insufficient Github Action time limit
  (if the job failed to complete in 6 hours, it will be canceled), which
  end up of having two jobs. If the x86_64 and aarch64 job are finished
  and images are published in vN, while riscv64 job fails in that run,
  restarting the riscv64 job will result an absence of vN and a presence
  of v(N+1). So we decided to move on to git sha1 tagging strategy.

  Remove latest function and vXX counter tagging, and use output of

  git show -s --format=%h

  as the tag of images built per PR for publishing.

• docker.sh: Introduce RISC-V support for manual operations

• ci: Add latest tag for ease of use

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

• All commits in this PR have Signed-Off-By trailers (with
  git commit -s), and the commit message has max 60 characters for the
  summary and max 75 characters for each description line.
• All added/changed functionality has a corresponding unit/integration
  test.
• All added/changed public-facing functionality has entries in the "Upcoming
  Release" section of CHANGELOG.md (if no such section exists, please create one).
• Any newly added unsafe code is properly documented.

[stefano-garzarella]

Yeah, thanks for implementing my suggestion of using sha1 instead of counter.
I suggest putting some context in the PR description and also in the commit description where we change from vXX to gYYYYY, especially to explain the reason of the changes, otherwise can be hard for reviewer to understand what we are fixing here.

What about updating the README.md too, where we used v38 as an example?

Maybe we can also add a section to mention that we used vXX till v49/v50, then switched to gYYYYY (where YYYYY=$(git show -s --format=%h))

[RuoqingHe]

What about updating the README.md too, where we used v38 as an example?

Would it be better to use latest tag in our README.md for developers who are going to use that image 🤔

[stefano-garzarella]

What about updating the README.md too, where we used v38 as an example?

Would it be better to use latest tag in our README.md for developers who are going to use that image 🤔

This is a good idea, is latest automatically handled by dockerhub, or we need to push that tag every time?
BTW, I'd mention both gYYYYY and latest, mentioning the latest should not be used in rust-vmm-ci for the reasons we discussed on slack

[RuoqingHe]

What about updating the README.md too, where we used v38 as an example?

Would it be better to use latest tag in our README.md for developers who are going to use that image 🤔

This is a good idea, is latest automatically handled by dockerhub, or we need to push that tag every time? BTW, I'd mention both gYYYYY and latest, mentioning the latest should not be used in rust-vmm-ci for the reasons we discussed on slack

Yes, gYYYY for rust-vmm-ci and latest for users, but I didn't find latest tag on our dockerhub. I'll dive to that action to see how to get it working 🙂

[stefano-garzarella]

LGTM! @RuoqingHe I'd just suggest fixing also the README and add a little section that we switched from vXX to gYYYYY.

[RuoqingHe]

LGTM! @RuoqingHe I'd just suggest fixing also the README and add a little section that we switched from vXX to gYYYYY.

Thanks for reviewing, I gather there were something wrongly configured in my workflow, I force pushed to see if it's addressed 🙂

[RuoqingHe]

LGTM! @RuoqingHe I'd just suggest fixing also the README and add a little section that we switched from vXX to gYYYYY.

Looks like that's now working, I will write them into README.md and document this change

[stefano-garzarella]

@RuoqingHe LGTM! thanks for this work!

@roypat can you take a look?

[roypat]

looks good, just a small comment on the wording in the readme :)

Is the segfault in the github action just some intermittent thing?

[roypat]

Hmm, it looks like builds for x86_64 and arm64 are really broken, not just flakes 😱

Ooo, but it can't be related, right? A segfault while building serde-json is strange, since we just touched how we tag container images.

mhh, we don't pin dependency versions in the docker file, so whenever we rebuild we just pull in the newest version of everything. cargo-audit had a new release 10 days ago, so might be that this new version has this issue? We can try pinning it to an older version (and maybe cut an issue to the appropriate location? maybe rustc?)

[stefano-garzarella]

Hmm, it looks like builds for x86_64 and arm64 are really broken, not just flakes 😱

Ooo, but it can't be related, right? A segfault while building serde-json is strange, since we just touched how we tag container images.

mhh, we don't pin dependency versions in the docker file, so whenever we rebuild we just pull in the newest version of everything. cargo-audit had a new release 10 days ago, so might be that this new version has this issue? We can try pinning it to an older version (and maybe cut an issue to the appropriate location? maybe rustc?)

yep, I agree. @RuoqingHe can you take a look when you have time? I know it's Chinese new year (happy new year!!!) and you will travel for FOSDEM, so take your time, it's not urgent!

[RuoqingHe]

Could @stefano-garzarella @roypat restart the failed job for me when convenient, my local builds start to pass, looks like our upstream have this problem addressed 😯

[RuoqingHe]

Okay, it seems I was right that I tested locally even with arm64 build:

I'm now confused 😵‍💫, the line added is used to address dpkg: error processing package libc-bin (--configure) reported while trying to install libc-bin, that's a problem on my setup but not a problem on our workers, how strange

[stefano-garzarella]

@RuoqingHe what is the current state?
Should we mention that we are updating to ubuntu 24 ?

[stefano-garzarella]

@RuoqingHe what is the current state? Should we mention that we are updating to ubuntu 24 ?

Also, why it's needed? I mean we are just changing the versioning. If it's a fix for something else, IMHO we should fix in another PR.

[RuoqingHe]

@RuoqingHe what is the current state? Should we mention that we are updating to ubuntu 24 ?

Also, why it's needed? I mean we are just changing the versioning. If it's a fix for something else, IMHO we should fix in another PR.

Never mind, I was just trying if upgrading to 24.04 helps addressing this problem. I'm not planning to propose upgrading to 24.04 in this PR 😉

I couldn't reproduce this problem (buildx fails on arm64) in my setup, I'm just running out of ideas 😭

[stefano-garzarella]

@RuoqingHe what is the current state? Should we mention that we are updating to ubuntu 24 ?

Also, why it's needed? I mean we are just changing the versioning. If it's a fix for something else, IMHO we should fix in another PR.

Never mind, I was just trying if upgrading to 24.04 helps addressing this problem. I'm not planning to propose upgrading to 24.04 in this PR 😉

I couldn't reproduce this problem (buildx fails on arm64) in my setup, I'm just running out of ideas 😭

Do you think it's related to our changes?

[RuoqingHe]

@RuoqingHe what is the current state? Should we mention that we are updating to ubuntu 24 ?

Also, why it's needed? I mean we are just changing the versioning. If it's a fix for something else, IMHO we should fix in another PR.

Never mind, I was just trying if upgrading to 24.04 helps addressing this problem. I'm not planning to propose upgrading to 24.04 in this PR 😉
I couldn't reproduce this problem (buildx fails on arm64) in my setup, I'm just running out of ideas 😭

Do you think it's related to our changes?

I hardly think so, this PR haven't touched Dockerfile or build_container.sh, docker.sh only executes before docker is invoked.

How about we restart workflow of our latest commit to find out, but that would result v51 image in our dockerhub 😂

[stefano-garzarella]

@RuoqingHe what is the current state? Should we mention that we are updating to ubuntu 24 ?

Also, why it's needed? I mean we are just changing the versioning. If it's a fix for something else, IMHO we should fix in another PR.

Never mind, I was just trying if upgrading to 24.04 helps addressing this problem. I'm not planning to propose upgrading to 24.04 in this PR 😉
I couldn't reproduce this problem (buildx fails on arm64) in my setup, I'm just running out of ideas 😭

Do you think it's related to our changes?

I hardly think so, this PR haven't touched Dockerfile or build_container.sh, docker.sh only executes before docker is invoked.

How about we restart workflow of our latest commit to find out, but that would result v51 image in our dockerhub 😂

Maybe we can open a draft PR with minimal changes just to start the CI, but I'd not start the one on main to avoid new version published

[RuoqingHe]

@RuoqingHe what is the current state? Should we mention that we are updating to ubuntu 24 ?

Also, why it's needed? I mean we are just changing the versioning. If it's a fix for something else, IMHO we should fix in another PR.

Never mind, I was just trying if upgrading to 24.04 helps addressing this problem. I'm not planning to propose upgrading to 24.04 in this PR 😉
I couldn't reproduce this problem (buildx fails on arm64) in my setup, I'm just running out of ideas 😭

Do you think it's related to our changes?

I hardly think so, this PR haven't touched Dockerfile or build_container.sh, docker.sh only executes before docker is invoked.

How about we restart workflow of our latest commit to find out, but that would result v51 image in our dockerhub 😂

Maybe we can open a draft PR with minimal changes just to start the CI, but I'd not start the one on main to avoid new version published

Noted, I will raise one 👍

[RuoqingHe]

Green light, are we good to go @roypat @stefano-garzarella 🫠

[stefano-garzarella]

@RuoqingHe great!

@roypat do you think we should open a separated PR for the CI fix (last patch)? TBH I'm fine also merging this PR with the fix.

[roypat]

Wohooo, great work!!

@roypat do you think we should open a separated PR for the CI fix (last patch)? TBH I'm fine also merging this PR with the fix.

I'm fine merging it through here as well :)