It was an inlined `go test` in the workflow, and the only thing in the build
that runs it. Every other gate there goes through make - make test, make
build, make checksilent - and `make test` is `go test ./...` in this module,
which cannot reach test/e2e-apporder because that is a module of its own.
So the one check that exercises installing an application was the one check a
developer had no command for, and the only place it could turn red was after
pushing.
The fourth registered driver, and the one that disagrees with the other three
about NULL. A suite that never pointed at it reported success for a migration
no SQL Server database could apply - the same shape as the PostgreSQL gap that
put the postgres service here, one driver further along.
SQL Server has no equivalent of POSTGRES_DB, so the database the DSN names is
created in a step before the tests. The test helper fails rather than skips
when CI is set and the variable is not, so dropping the service or renaming
the variable cannot quietly go green.
Cleaning up after a successful deployment never runs on the host that needs it.
The pull is the first thing in this script that needs space and it is where a
full disk stops it, so the run ends before reaching any cleanup - and so does
the next run, and the one after that. That is not hypothetical: a deployment
failed on the pull with no space left on the device, and rerunning the workflow
unchanged failed at the same place. The disk had to be cleared by hand before a
deployment could go through.
The pipeline is now a function called twice, before the pull and after the
health check, so the window is bounded on both sides.
Verified against a real docker daemon, in the function form rather than the
inlined one: with five images newer than the running one, so position alone no
longer protects it, it leaves three and does not select the live one; removing
the id exclusion from the same function does select it. With NAME pointing at a
container that does not exist it selects nothing - as it also does without the
explicit guard, which is there because grep -v on an empty id reads like the
opposite of what it does, not because it changes the outcome.
Every deployment pulls an image tagged with its commit and nothing removed the
previous one, so they only accumulated. 68 had built up when a deployment failed
on a pull with no space left on the device. That is the harmless place to fail -
the site kept serving the image it already had - but no later run would have
recovered on its own.
Three are kept so a release can be re-run by tag by hand. Only this repository's
images are listed, because the host runs other services. The image the new
container is on is excluded by id rather than by position, and rmi is called
without -f so an image a container still holds is refused rather than taken from
it.
Verified against a real docker daemon: with five images newer than the running
one, so position alone no longer protects it, the pipeline leaves three and does
not select the live one. Removing the id exclusion from the same pipeline does
select it, so that guard is load-bearing rather than decorative.
The rest of this package's tests run on SQLite, where dropping an index
through the migrator works. That is why a migration which failed on
every PostgreSQL database it was pointed at had a green suite: the
defect cannot occur on the backend being tested.
A test alone would not have helped either - without a service it skips,
and a test that never runs is the same as no test. So the workflow gains
a postgres service and the DSN, and the helper refuses to skip when CI
is set: a workflow that drops the service or renames the variable fails
rather than going quietly green, which is the shape of the original
defect.
Counter-proved by putting Migrator().DropIndex back, which reproduces
the statement verbatim:
DROP INDEX CURRENT_SCHEMA()."idx_sd_pg_user_deleted_at"
ERROR: syntax error at or near "CURRENT_SCHEMA" (SQLSTATE 42601)
The conversion test also checks the timestamp survives as a marker,
since a conversion that dropped it would bring deleted rows back live
while still passing a column-type assertion.
The comment at the top of this workflow says documentation-only changes
skip it, because a push to master pushes an image, runs the migrations
and restarts the demo container. The ignore list did not cover
scripts/k8s, so editing a manifest that the deploy never reads bought
the site an outage.
The pattern is scripts/k8s/** rather than scripts/** because
scripts/Dockerfile is a build input - go.yml builds the release image
from it on a tag.
This workflow file stays outside the list on purpose. paths-ignore skips
only when every changed path matches, so a change that edits the deploy
still runs it, which is the point.
Stopping this process takes drain + server + cleanup seconds: eight out of the
box, and more for anyone who configures a drain window. Three places decide
whether it gets that long, and none of them was written with it in mind.
The release workflow stopped the previous container with the default deadline,
which docker sets at ten seconds. The compose file - which the Makefile calls
the first way to run this - set no stop_grace_period, so it took the same ten.
Under either, a drain window over two seconds would have been cut off by
SIGKILL part-way through the cleanup callbacks: this project's own deployments
could not have run the capability it ships.
The third was worse. `make run` removed the previous container with `docker rm
-f`, and the force flag kills a running container outright - "uses SIGKILL", in
docker's own words - with no grace at all. Restarting locally cut every
shutdown short, so the drain window would never once have been reached on a
developer's machine. It now stops with a deadline and then removes, which
leaves what gets removed unchanged: on a container that has already stopped,
stop is a no-op.
So: --timeout 30 in the workflow, stop_grace_period: 30s on the compose
service, and stop --timeout 30 before the removal in the Makefile. --timeout
rather than --time, which docker still honours but has deprecated - it prints a
warning on every use, and a deploy log that always carries a warning is one
nobody reads.
The three remaining `docker rm -f` calls in the workflow are left alone. Two
remove containers that have already been stopped and one is the rollback path,
and nothing static can tell those apart from a container that is still running -
which is also why the check added next does not look at `rm -f` at all: a forced
removal has no deadline to compare against. What keeps that path honest is the
line above it, not a check.
Thirty will drift the first time somebody raises a budget. The next commit is
what notices, which is also why these comments name a check that does not exist
yet.
The rule that consumers are registered before the queue is started had no test
that could fail on the backend it exists for. Everything so far ran on the
memory queue, which is the default: queue.Memory's Register starts another
consumer goroutine whatever the state, so the wrong order passes there. A suite
that only exercises the default reports success for a queue that accepts no
consumers at all.
CI gets a redis service, and two tests build the queue the way setupQueue does -
through config.QueueConfig.Setup, so what is under test is the adapter this
repository actually gets, LegacyQueueAdapter included. They skip without
GO_ADMIN_TEST_REDIS_ADDR, so a developer with no server still gets a green run.
Registering first and starting second delivers the message. Starting first and
registering second is refused: no consumer group was created, so Append comes
back with storage.ErrNoHandler. Pinning that particular error rather than "some
error" is deliberate - the test is about the missing consumer, and a connection
failure that happened to error too would otherwise pass for it.
Running it corrected something written two commits ago. The claim there was
that a late registration loses consumers "with nothing said". Only half of that
holds: the registration is silent, because Register returns nothing, but every
publish afterwards fails loudly - ErrNoHandler, logged at error level by both
call sites in common/middleware - while the log rows are never written. The
symptom is missing rows plus a lot of noise, not a quiet nothing. That commit's
message and the contract doc both say so now.
The repository has 19 test files and nothing was running any of them. Both
workflows build with go build, which does not compile _test.go, the Makefile's
test target was commented out, and there is no pre-commit hook. Every test in
the tree, including the schema guards that exist precisely to catch a silent
breakage, only ran when someone remembered to type go test.
Enables the commented-out target and calls it from go.yml, the one workflow
that fires on every push and pull request. build.yml is left alone: it skips
documentation-only changes and deploys on master, so it is the wrong place for
a gate that should never be skipped.
Runs with -race. common/actions reuses model instances across concurrent
requests, so a Generate() that returns in place rather than a copy leaks data
between them, which a single-threaded run cannot see.
Verified locally: the suite passes under CGO_ENABLED=0 and under -race, and
make test exits non-zero when a test fails, so the step actually gates.
Claude-Session: https://claude.ai/code/session_01DJhM6LvhkNPej35wy9F7Aq
A push to master here does not just build: it pushes an image, runs the
migrations and restarts the demo container, so the site takes a short outage
each time. The last two merges were markdown only and both paid for it.
Beyond the waste, a deploy can fail for reasons unrelated to the change that
triggered it - a container that will not come up, a database that is briefly
unreachable - and a README edit should not be able to turn the demo red.
Only build.yml is filtered. go.yml still builds on every push and pull request,
so nothing loses its compile check, and the badge keeps reporting the same
workflow it reports today.
Closes#871.
The deploy did docker rm -f then docker run. Nothing ran migrations, so
new code met old tables, and nothing checked the result - a container
that exits immediately left the site down with a green deploy.
Now, in order: pull the image, run the migration with it, and only then
touch what is running. A failed migration stops there, leaving old code
with the old schema, which is at least self-consistent.
The running container is renamed rather than removed, so it can be
started again unchanged if the new one does not become healthy. Healthy
means both an HTTP response and a database connection in the log: the
captcha endpoint answers without touching the database, so it alone
would call a container healthy that cannot reach MySQL.
The path is not a credential, and the file it points at is 600 and owned
by root, so this is not what protects it. But the repository is public
and there is no reason to publish the server's directory layout next to
the deploy that uses it.
DEMO_CONFIG_PATH holds it instead. It has to be set before this merges,
or the deploy stops at the guard - which is the intended failure: better
that than falling back to the sqlite in the image.
The demo ran on the sqlite file baked into the image, so every deploy
reset it and nothing there resembled how anyone actually runs this.
The config is mounted from the host rather than taken from the image.
config/settings.demo.yml ships in a public repository and is copied into
a public image, so the connection string cannot live there; that copy
stays on sqlite, which is what a fresh clone should get.
The deploy refuses to start if the host config is missing, rather than
falling back to the image's sqlite and looking like it worked.
Two merges seconds apart raced. Both runs do docker rm -f then docker
run; the second removed the container the first had just created, and
the first's docker run failed on the name conflict:
Conflict. The container name "/go-admin-api" is already in use
The deploy went red and the demo stayed on the older image, which is the
worse half: a failure that leaves the wrong version running.
Grouping by ref serialises pushes to master while leaving pull request
runs independent, since those carry their own ref.
Every push mirrored to Gitee and GitLab. Neither mirror is wanted any
more, so the workflow goes rather than half of it.
The GITEE_KEY and GITLAB_KEY secrets are left in place; restoring the
mirror is a revert of this commit.
Replace version tags (@v1/@v2/@v3/@master) with pinned commit SHAs
across all workflow files to satisfy go-admin-team organization
security policy requiring immutable action references.
zhangwenjian