bb.org: Add docker library cleanup on dependent failure

[grooverdan]

results appears to be BuildStep attribute per
master/buildbot/process/buildstep.py in the buildbot code.

The unused(broken?) ifStagingSucceeding uses:
step.build.results in ('SUCCESS', 'WARNINGS')

[cvicentiu]

Hi @grooverdan. I don't fully understand what the problem is, but I assume it's some cleanup that is not running. It would be good if you could document this failure a little bit better in the commit message, so we know the history behind it.

Also, why does this have to be a new builder? Why isn't this a simple step?
The current code, if I understand it correctly, assumes that the build will be run on the same worker and if at any point in the future we have two workers running docker_library builds the current setup won't work as the cleanup might just run on a different machine.

[fauust]

This looks OK to me. But I am not sure how to test this? Also I am wondering if we could not add a cleaning based on the until filter?
https://docs.podman.io/en/latest/markdown/podman-image-prune.1.html

This is maybe not needed but we have implemented a more drastic clean on docker workers, see: https://gitlab.com/mariadb/sysadmin/-/blob/master/ansible/roles/bb_worker_docker/tasks/docker.yml#L27-34

[fauust]

I am wondering if we should not use getopts more frequently for those script? It makes them easier to debug and above all more robust in case they are not called correctly:

  usage() {
    cat >&2 <<-EOF
    Usage : $0 -v <mariadb-version> -c <commit> -b <branch>
      -v mariadb version
      -c commit hash
      -b branch
      -h help
  EOF
  }

  typeset mariadb_version=""
  typeset commit=""
  typeset branch=""

  while getopts "v:c:b:h" OPTION; do
    case $OPTION in
      v)
        mariadb_version="$OPTARG"
        ;;
      c)
        commit="$OPTARG"
        ;;
      b)
        branch="$OPTARG"
        ;;
      h)
        usage
        exit 0
        ;;
      *)
        usage
        exit 1
        ;;
    esac
  done

  [[ $mariadb_version != "" ]] || {
    usage
    exit 1
  }
  [[ $commit != "" ]] || {
    usage
    exit 1
  }
  [[ $branch != "" ]] || {
    usage
    exit 1
  }

  mariadb_version=${mariadb_version#*-}
  master_branch=${mariadb_version%\.*}
  commit=${$commit:-}
  branch=${$branch:-${master_branch}}

[fauust]

command=["bash", "-xc", util.Interpolate("./docker_cleanup.sh -v \"%(prop:mariadb_version)s\" -c \"%(prop:revision)s\" -b \"%(prop:branch)s\"")]))

See above

[fauust]

Not mandatory, but good habit IMO:
[shellcheck] SC2086: Double quote to prevent globbing and word splitting. [Info]

[fauust]

for foo in bar; do

[grooverdan]

Hi @grooverdan. I don't fully understand what the problem is, but I assume it's some cleanup that is not running. It would be good if you could document this failure a little bit better in the commit message, so we know the history behind it.

Also, why does this have to be a new builder? Why isn't this a simple step?
The current code, if I understand it correctly, assumes that the build will be run on the same worker and if at any point in the future we have two workers running docker_library builds the current setup won't work as the cleanup might just run on a different machine.

Its adding a fail only step that triggers the cleanup script on a different worker. i.e. ppc64le-ubuntu-2004-deb-autobake fails to build, trigger the cleanup of the production of the image from the other workers (which as I realize now assumes this failure is the last step).

Better suggestions welcome.

[cvicentiu]

@grooverdan I still don't understand. Why is this necessary? If an image fails to build, what gets pushed to the registry that needs to be cleaned up?

And you still haven't answered what prevents you from running the cleanup script on the same worker?

[grooverdan]

commit message description updated.

[cvicentiu]

Hi @grooverdan ! Thank you for the updated commit message. This made it much clearer.

A better proposal coming up in a few hours.