Fix docker bake tagging #782
Conversation
…ternal devops builds
pschork
force-pushed
the
pschork/fix_docker_bake_tagging
branch
from
a8a9b64 to
12e4f78
Compare
pschork
force-pushed
the
pschork/fix_docker_bake_tagging
branch
from
49a1567 to
e73cdf5
Compare
docker-bake.hcl
Outdated
| variable "GIT_SHA" { | ||
| default = "$GIT_SHA" | ||
| } | ||
|
|
||
| variable "GIT_SHORT_SHA" { | ||
| default = "$GIT_SHORT_SHA" |
There was a problem hiding this comment.
These defaults are designed to fail (via invalid tag format) if someone tries to bake a release target without exporting GIT_SHA or GIT_SHORT_SHA env variables
docker buildx bake internal-release
[+] Building 0.1s (1/1) FINISHED docker:desktop-linux
=> [internal] load local bake definitions 0.0s
=> => reading docker-bake.hcl 5.14kB / 5.14kB 0.0s
ERROR: invalid tag "ghcr.io/eigenda-traffic-generator:$GIT_SHA": invalid reference format
without a default, the build will still fail, but error message is harder to grok what the issue is.
docker buildx bake internal-release
[+] Building 0.1s (1/1) FINISHED docker:desktop-linux
=> [internal] load local bake definitions 0.0s
=> => reading docker-bake.hcl 5.14kB / 5.14kB 0.0s
ERROR: invalid tag "ghcr.io/eigenda-traffic-generator:": invalid reference format
There was a problem hiding this comment.
I'd prob add this as a comment above the variables themselves for documentation. Otherwise I can see myself reading this and being super confused wondering where/how this variable is defined and used.
| SEMVER := $(shell basename $(CURDIR)) | ||
| else | ||
| GITCOMMIT := $(shell git rev-parse --short HEAD) | ||
| GITDATE := $(shell git log -1 --format=%cd --date=unix) | ||
| GITSHA := $(shell git rev-parse HEAD) | ||
| BRANCH := $(shell git rev-parse --abbrev-ref HEAD | sed 's/[^[:alnum:]\.\_\-]/-/g') |
There was a problem hiding this comment.
Note: this sanitizes the branch name to conform with docker tagging syntax rules.
docker-bake.hcl
Outdated
|
|
||
| # Internal devops builds | ||
| group "internal-release" { | ||
| targets = ["node-internal", "batcher-release", "disperser-release", "encoder-release", "retriever-release", "churner-release", "dataapi-release", "traffic-generator-release"] |
There was a problem hiding this comment.
Why is it node-internal and not node-release?
There was a problem hiding this comment.
node-release was already used for public node releases and is incompatible with internal release targets.
There was a problem hiding this comment.
I'll rename node-internal-release
There was a problem hiding this comment.
also confused. is it because node-release is already something separate which builds with the arm target?
Should we call all the other internal-release targets xxx-internal instead of release then? Especially if those things are only internal, why call them release?
There was a problem hiding this comment.
also confused. is it because node-release is already something separate which builds with the arm target?
it's because the existing node-release is incompatible with out internal release tagging.
target "node-release" {
inherits = ["node", "_release"]
tags = ["${REGISTRY}/${REPO}/opr-node:${BUILD_TAG}"]
}
target "node-internal-release" {
inherits = ["node"]
tags = ["${REGISTRY}/eigenda-node:${BUILD_TAG}",
"${REGISTRY}/eigenda-node:${GIT_SHA}",
"${REGISTRY}/eigenda-node:sha-${GIT_SHORT_SHA}",
]
}
Should we call all the other internal-release targets xxx-internal instead of release then? Especially if those things are only internal, why call them release?
Definitely agree that "release" is way overloaded. I'll rename
Why are these changes needed?
Checks