AntelopeIO / DUNES

Docker Utilities for Node Execution
Other
26 stars 20 forks source link

Local testing will use an out of date container. #138

Closed ScottBailey closed 1 year ago

ScottBailey commented 1 year ago

Problem: when automatic testing (i.e. pytest) is performed, the"latest" DUNE container is used. Unfortunately, there is no guarantee the container is truly the latest. This has resulted in tests failing that should not have as well as test passing that should not have.

Proposed solution:

Update the bootstrap process to contain a unique identifier and add a test to ensure the identifier exists. One UID option is the git commit hash

stephenpdeos commented 1 year ago

On the fence for v1.2.0

stephenpdeos commented 1 year ago

Requires a proposal for what our execution plan is before proceeding. Presently considering using the commit as the unique identifier. Fallback may be DUNE version if the release doesn't have a git commit.

ScottBailey commented 1 year ago

Given the following:

SCRIPT=$(readlink -f "$0")
SCRIPT_DIR=$(dirname "$SCRIPT")

And:

calc_dune_uuid() {

    if git --version && git -C ${SCRIPT_DIR} rev-parse HEAD; then
        DUNE_UUID=$(git -C ${SCRIPT_DIR} rev-parse HEAD)
    else
        DUNE_UUID=$(${SCRIPT_DIR}/dune --version)
    fi
}

calc_dune_uuid
echo ${DUNE_UUID}

DUNE_UUID can contain either the commit hash OR the dune version.

Examples: 206a07aae938658ea69cc362a36c7669a1bbf0c7 DUNE v1.1.0

This is a possibility for the UUID.

ScottBailey commented 1 year ago

Consider using git short hash when available with fallback to dune version.

We can do this by adding version_uuid() to dune versions and using 'dune:' + version_uuid() instead of dune:latest. And we can add a hidden --version-uuid flag to provide this value to bootstrap.sh.

ScottBailey commented 1 year ago

@mikelik Here's my proposal, please provide feedback.

bootstrap.sh names images based on git commit hash. We can rename them to dune:version (e.g. dune:1.1.0) and dune:latest later.

My proposal is that DUNE itself prefers images in this order:

  1. dune:latest if it exists locally - this must be downloaded with --upgrade.
  2. dune:commit-hash if it exists locally - normally it will not.
  3. dune:ver local.
  4. dune:ver downloaded.

For testing, we want to use dune:git-hash if we are testing from a git repo; if not, we want to use dune:ver.

So in testing, assert if:

  1. There is an active dune:latest container - let's not destroy the user's containers!
  2. This is a git repo and there is an active dune:version container.
  3. This is a git repo and dune:commit-hash is not built yet.
mikelik commented 1 year ago

Currently DUNE always uses dune:latest. If it doesn't exist it is downloaded. To simplify the process I propose to not change this.

Docker image can have several tags. My proposal is to tag an image when using bootstrap.sh with dune:commit-hash (if exists). Command would be docker tag dune:latest dune:commit-hash.

I see following scenarios:

  1. User is running tests on git repo and has downloaded latest image dune:latest In general we shouldn't run tests using downloaded dune:latest when working on git repo, because the image is old. It will work only if there was a new DUNE release, dune:latest has just been published and it is in sync with the git repo. In tests assert that user has executed bootstrap.sh beforehand. We can check that by confirming that image_id of dune:commit-hash is the same as image_id of dune:latest. If dune:commit-hash doesn't exists we should fail (because user has not executed bootstrap.sh.
  2. User is running tests on git repo and has built image with bootstrap.sh Checks as above.
  3. User is running tests on installed version and only downloaded dune:latest is available. I'm not sure we want to cover this scenario in this issue. For now it might be enough to assert that dune:commit-hash is not the same as image_id of dune:latest. To fully cover this scenario we would need to introduce dune:ver, a new tag which should be added when downloading DUNE docker image for the first time. dune:ver needs to be updated when dune --upgrade is called. Then in tests we should assert that dune:ver image id is the same as dune:latest.