Conversation
maximthomas
left a comment
There was a problem hiding this comment.
praise: The join reads membership from the server's own state instead of from dsreplication's exit codes, and the root password stays off every command line.
is_member(bootstrap/join.sh:135-140) needs both theBASE_DNdomain in cn=config and the cn=admin data registration, so the two meanings of exit 5 no longer decide anything.- Every tool reads
--bindPasswordFilefrommktemp -p /dev/shm "opendj-join.$ADMIN_PORT.XXXXXX"(join.sh:100),run.sh:45removes only the leftovers for this container'sADMIN_PORT, andbuild.yml:593greps the scripts statically for a password flag. - The anchored self-recognition (
join.sh:111-116) replaces the/etc/hostsgrep that took opendj-1 for opendj-10.
issue (blocking): A failed dsreplication initialize is reported as success, so the container turns healthy without the topology's data.
opendj-packages/opendj-docker/bootstrap/join.sh:198-208
rc=$? right after if initialize_from …; then …; fi holds the exit status of the if itself, which is 0 when the condition failed and there is no else. After the last attempt, return $rc returns 0, and joined() then runs cleanup_departed and touches $BOOTSTRAP_COMPLETE. So a replica whose every initialize failed reports itself healthy while it holds only its bootstrap entries. This happens when the source is unreachable, or when each attempt is killed as described in the third comment. Every retry line also logs "exited with 0". A bash probe of the same shape prints attempt 1 rc=0, attempt 2 rc=0, g returned 0.
initialize_from "$source"
rc=$?
if [ "$rc" -eq 0 ]; then
rm -f "$INITIALIZE_PENDING"
# ... generation ID cross-check as now ...
return 0
fi
[ "$i" -eq "$REPLICATION_RETRY_COUNT" ] && return $rcissue (blocking): On a restart, run.sh writes the health marker even when $INITIALIZE_PENDING is still on the volume.
opendj-packages/opendj-docker/run.sh:142
The gate only looks for ds-cfg-replication-domain in config.ldif. On a first start where the enable succeeded but the initialize failed or was killed, the container is correctly unhealthy. The next start is healthy immediately, with bootstrap-only or half-imported data, while join.sh re-initializes in the background. If no peer answers generation_id, ensure_initialized "" returns 1 (join.sh:182-195) and the server stays healthy but unreplicated. The README ("healthy only once the join succeeded - across restarts too") and the comment at run.sh:136-141 promise the opposite.
if ! join_requested || { grep -q "ds-cfg-replication-domain" ./data/config/config.ldif && [ ! -f "$INITIALIZE_PENDING" ]; }; thenWith this change, under OrderedReady a -0 that still carries the marker waits for a peer. That is the honest state for a server that never received the topology's data, and it deserves a line in the README.
issue (blocking): dsreplication initialize is killed after REPLICATION_ATTEMPT_TIMEOUT (120 s by default), so a directory whose total update takes longer than that can never be initialized.
opendj-packages/opendj-docker/bootstrap/join.sh:160-165, opendj-packages/opendj-docker/README.md:196
A total update is a full import of BASE_DN. If it needs more than 120 s, every attempt is killed before its task finishes, and the next attempt requests another full import, so no attempt can ever succeed. $INITIALIZE_PENDING is never cleared and every start imports again. Combined with the first comment, the container turns healthy after 30 failures; combined with the second, on the next restart. The README justifies the bound with a hanging dsreplication enable, while the base replicate.sh ran initialize without a bound. With the defaults, scaling up a StatefulSet over any non-trivial directory hits this.
initialize_from() {
dsreplication initialize --baseDN "$BASE_DN" \
--adminUID admin --adminPasswordFile "$PASSWORD_FILE" \
--hostSource "$1" --portSource "$ADMIN_PORT" \
--hostDestination "$MYHOSTNAME" --portDestination "$ADMIN_PORT" -X -n
}Or: add a separate REPLICATION_INITIALIZE_TIMEOUT, unset by default, and make README:196 say which commands the attempt timeout covers.
issue (blocking): cleanup_departed leaves a departed server in the cn=schema (and cn=admin data) replication domains, so the scale-down check cannot pass.
opendj-packages/opendj-docker/bootstrap/join.sh:263-279, .github/workflows/build.yml:694
join.sh passes no --noSchemaReplication, so dsreplication enable also configures the cn=schema domain with the same replication-server list, on every server in the topology. On the first enable it does the same for the cn=admin data domain (ReplicationCliMain :4761-4767, :4850-4859, :6329-6391). The cleanup only prunes the replication-server entry and the BASE_DN domain. After the scale-down, dj-0 and dj-1 therefore keep dj-2:8989 in their cn=schema domain. The check at build.yml:694 greps every replication-domain entry for dj-2, so it loops until timeout 2m fails the step. Outside CI, the survivors keep dialling the removed server. The docker jobs have not run at this head yet, and scale-down is the one scenario missing from the description's "Verified locally" list.
search localhost --baseDN "cn=config" --searchScope sub "(objectClass=ds-cfg-replication-domain)" cn \
| awk '/^cn: /{print substr($0,5)}' \
| while read -r domain; do
search localhost --baseDN "cn=config" --searchScope sub \
"(&(objectClass=ds-cfg-replication-domain)(cn=$domain))" ds-cfg-replication-server \
| awk '/^ds-cfg-replication-server: /{print $2}' \
| while read -r value; do
host=${value%:*}
if ! in_peers "$host" && ! is_self "$host"; then
echo "join: removing departed replication server $value from replication domain $domain"
dsconfig set-replication-domain-prop --provider-name "Multimaster Synchronization" \
--domain-name "$domain" --remove "replication-server:$value" \
--hostname localhost --port "$ADMIN_PORT" --bindDN "$ROOT_USER_DN" \
--bindPasswordFile "$PASSWORD_FILE" --trustAll --no-prompt || true
fi
done
doneissue (blocking): The build-docker-alpine "Docker test replication" step still exercises the simple path of replicate.sh, which run.sh no longer calls.
.github/workflows/build.yml:1115-1120, :1142
Only build-docker's step was rewritten. In build-docker-alpine, test_replica runs with MASTER_SERVER=dj-master OPENDJ_REPLICATION_TYPE=simple, and run.sh now hands that to join.sh. The step then waits 5 minutes for "Will sleep for a bit", which only replicate.sh:82 prints. The wait exits 124 on every run. The alpine image also gets no coverage of join.sh, and its password grep at :1098 does not list join.sh either. Port build.yml:581-708 into the alpine job, or move the step into a script under .github/scripts/ that both jobs call with their own image.
issue (non-blocking): is_member never finds the seed's cn=admin data entry when hostname -f is longer than the listed name, which is the case on Kubernetes.
opendj-packages/opendj-docker/bootstrap/join.sh:138-139
The seed never runs an enable of its own. A peer's enable registers it under the name that peer connected with (ServerDescriptor :648, HOST_NAME = conn.getHostPort().getHost()), which is <sts>-0.<svc>, while hostname -f in the pod is the FQDN. So on every restart -0 fails is_member, runs REPLICATION_RETRY_COUNT rounds of enables that exit 5, never runs cleanup_departed, and leaves through the seed branch. Health is not affected. CI hides the problem because --hostname dj-N equals the listed name.
is_member() {
local host
search localhost --baseDN "cn=config" --searchScope sub \
"(&(objectClass=ds-cfg-replication-domain)(ds-cfg-base-dn=$BASE_DN))" 1.1 | grep -q "^dn:" || return 1
for host in $(search localhost --baseDN "cn=Servers,cn=admin data" --searchScope one "(objectClass=*)" hostname \
| awk '/^hostname: /{print $2}'); do
is_self "$host" && return 0
done
return 1
}issue (non-blocking): The top-level docker logs X 2>&1 | grep -q … checks run under pipefail and can fail even after a match.
.github/workflows/build.yml:617, :625, :628, :666, :667
shell: bash runs as bash -eo pipefail. grep -q exits on the first match, so the next frame docker logs writes gets SIGPIPE (141), and the pipeline is non-zero although the line was found. Checks :617, :625, :628 and :666 then fail spuriously, and the negative check at :667 passes silently exactly when the seed line is present. The polls inside timeout … bash -c are not affected, because that inner bash has no pipefail. The failure rate on the runner was not measured.
docker logs dj-0 2>&1 | grep "seeding it with this server's data" >/dev/null || { echo "::error::dj-0 did not seed the topology"; false; }issue (non-blocking): On the Alpine image, timeout is BusyBox's, and it kills only the sh launcher of dsreplication, not the JVM.
opendj-packages/opendj-docker/bootstrap/join.sh:143, :161, opendj-packages/opendj-docker/Dockerfile-alpine:60
Dockerfile-alpine installs bash "$JDK" and no coreutils. BusyBox timeout signals only the PID it execs, and _mixed-script.sh:69 starts java without exec. A timed-out attempt therefore leaves its JVM running under the container's memory limit while the next attempt starts, so the per-attempt bound does not hold on Alpine. This was not probed in the image.
&& apk add bash coreutils "$JDK" \issue (non-blocking): generation_id can return another server's generation ID, because (connected-to=*) does not single out the domain's own monitor entry.
opendj-packages/opendj-docker/bootstrap/join.sh:152-158
DataServerHandler (:244) and ServerHandler (:559, :591) put connected-to, domain-name and generation-id on the replication server's entry for every connected directory server. awk … exit takes the first match, so the cross-checks at :174-178 and :200-204 may compare the wrong entry. replayed-updates is published only by ReplicationMonitor (:79). The comment at :152-153 needs the same correction.
"(&(domain-name=$BASE_DN)(replayed-updates=*))" generation-id \issue (non-blocking): The README says plain docker run passes the container names, but that only works when --hostname equals --name.
opendj-packages/opendj-docker/README.md:77-80, opendj-packages/opendj-docker/bootstrap/join.sh:65
MYHOSTNAME defaults to hostname -f, which is the container ID when --hostname is not given. So in a container named dj-0, is_self dj-0 is false: the container never takes the seed rule and tries to enable with itself under a second name. CI passes --hostname "$1" (build.yml:606). What that self-enable leaves behind was not run.
no registry; plain `docker run` passes the container names, each container started with
`--hostname` equal to its `--name` (or with `MYHOSTNAME` set to it). `MASTER_SERVER` keeps workingquestion (non-blocking): Is a parallel first start (Compose, podManagementPolicy: Parallel) meant to be supported?
opendj-packages/opendj-docker/bootstrap/join.sh:319-321, opendj-packages/opendj-docker/run.sh:179
run.sh marks every bootstrapped volume, including the first peer's, and only the seed branch (join.sh:335-337) removes that marker. When the peers start together, B's enable through A makes A a member. A then initializes itself from B while B initializes from A, and the topology ends up with whichever import lands last, not with the first peer's data as the README says. If the bootstrap data is identical everywhere, the only cost is duplicate total updates. If parallel starts are out of scope, this is minor, and the README should say the first start has to be ordered. If they are supported, it is a major issue.
question (non-blocking): Was dropping IP and alias recognition of MASTER_SERVER intended?
opendj-packages/opendj-docker/bootstrap/join.sh:111-116
The base replicate.sh treated any /etc/hosts line containing MASTER_SERVER (an IP, hostAliases, --add-host) as "I am the master". is_self compares only with hostname -f and its first label. A master started with OPENDJ_REPLICATION_TYPE=simple and its own IP or an alias as MASTER_SERVER now tries to enable against itself, cannot take the seed rule, and stays unhealthy, where the old image was healthy. If this was intended, a compatibility line in the README (list the hostname, or set MYHOSTNAME) covers it.
suggestion (non-blocking): Pin the initialize with data that never went through the changelog. The log line and the current data checks both pass without any initialize.
.github/workflows/build.yml:626-631, :666-668
"initializing from dj-N" is printed before the call (join.sh:197). ou=replicated and ou=replicated2 were written over LDAP, so dj-1's changelog replays them to a server that was never initialized, since two fresh volumes share a generation ID. A no-op initialize_from that returns 0 keeps the step green (not run).
# dj-0 bootstraps with -e SAMPLE_DATA=10 in place of ADD_BASE_ENTRY (start_node takes the extra -e)
wait_healthy dj-1
docker exec dj-1 /opt/opendj/bin/ldapsearch --hostname localhost --port 1636 --bindDN "cn=Directory Manager" --bindPassword "$ROOT_PASSWORD" --useSsl --trustAll --baseDN "uid=user.0,ou=People,dc=example,dc=com" --searchScope base "(objectClass=*)" 1.1Pin: dj-1 must serve an entry that the seed only ever imported; the no-op mutant fails the search.
suggestion (non-blocking): Hold the "dj-x is not healthy" assertions over a probe cycle. A single inspect right after the failure line reads "starting" whatever join.sh did.
.github/workflows/build.yml:653, :657
The health status changes only after a probe that starts once the marker exists (the 5 s start interval plus the probe's ldapsearch). So a join.sh that wrote $BOOTSTRAP_COMPLETE on its failure path would pass most runs (not run). The restart check does catch the base behaviour, which wrote the marker after the upgrade, but not a marker written at the moment of failure.
for _ in $(seq 1 12); do
[ "$(docker inspect --format='{{.State.Health.Status}}' dj-x)" != healthy ] || { echo "::error::dj-x reports itself healthy although its join failed"; false; }
sleep 5
donesuggestion (non-blocking): Keep the pins on run.sh's removal of opendj-replicate.$ADMIN_PORT.* and on replicate.sh's retry on exit 8. The rewritten step dropped both.
.github/workflows/build.yml:643-647, :700
The step now plants only opendj-join.* files, and dj-sdsr runs without a network disconnect. The alpine copy still pins both behaviours, but it fails earlier (see above). Deleting the opendj-replicate glob from run.sh:45, or breaking retry 8, would stay green.
docker exec dj-shm sh -c ': >/dev/shm/opendj-join.4444.killed && : >/dev/shm/opendj-replicate.4444.killed && : >/dev/shm/opendj-join.5444.other'
docker run -d --memory="512m" --network test_replication --ipc=container:dj-shm --name dj-shm-probe --hostname dj-shm-probe -e ROOT_PASSWORD="$ROOT_PASSWORD" "$IMAGE"
timeout 1m bash -c 'while docker exec dj-shm sh -c "ls /dev/shm/opendj-*.4444.* >/dev/null 2>&1"; do sleep 2; done'suggestion (non-blocking): No step in CI pins "membership from what is there, not the exit code", although the comment at build.yml:619-620 says this one does.
.github/workflows/build.yml:619-625, opendj-packages/opendj-docker/bootstrap/join.sh:317-319
A mutant enable_through "$peer" && is_member stays green. While dj-0 is off the network the enable fails without membership, afterwards it exits 0 with membership, and restarted members never call enable. A deterministic case of exit 5 with membership needs two enables of the same pair. Failing that, narrow the comment to the retry the step actually pins.
# a joining server tries again while its peer is unreachablesuggestion (non-blocking): is_self and in_peers treat any two names with the same first label as one server.
opendj-packages/opendj-docker/bootstrap/join.sh:115, :219-226
With the same StatefulSet name in two namespaces (opendj-0.opendj.east…, opendj-0.opendj.west…), west-0 skips east-0 and counts the first peer as itself, so it may seed a second topology. With an IPv4 list and MYHOSTNAME set to an IP, every name collapses to "10" and every server seeds at once. The documented list shapes never collide, but a guard would stop the other shapes from forking silently.
[ "$peer" = "$host" ] && return 0
case $peer in *[!0-9.]*) ;; *) return 1 ;; esac # an IPv4 address has no host label
[ "${peer%%.*}" = "${host%%.*}" ]nitpick (non-blocking): dj-sdsr still runs with --rm, so the ERR trap cannot print its log if its server exits.
.github/workflows/build.yml:700
cleanup() already removes the container.
docker run -d --memory="512m" --network test_replication --name dj-sdsr --hostname dj-sdsr -e ROOT_PASSWORD="$ROOT_PASSWORD" -e MASTER_SERVER=dj-0 -e OPENDJ_REPLICATION_TYPE=sdsr "$IMAGE"…gy's data, and clean every replication list Review round 1 of OpenIdentityPlatform#1115: - join.sh: a failed dsreplication initialize is no longer taken for a success (the exit status was that of an if without else), and it is bounded by its own REPLICATION_INITIALIZE_TIMEOUT, none by default; REPLICATION_ATTEMPT_TIMEOUT bounds the enable only, and the bound sits on the dsreplication call, as timeout cannot run a shell function. - join.sh publishes pending/ready in the local entry cn=Docker Join,cn=config; a pending server enables and initializes only through a ready peer, the first peer seeds at once when every other one is pending and after the retries only while no peer that could hold the data answers. Fresh servers started together no longer take each other's bootstrap data for the topology's. - The cleanup prunes departed servers from every replication list (replication server, BASE_DN, cn=schema, cn=admin data), and the lists get every listed, registered peer they lack; dsconfig inside the read loops reads /dev/null. - Self-recognition: a name equal to hostname -f or cut from it at a dot, and the container's own addresses and their /etc/hosts names, compared whole; is_member checks every registered hostname; the generation ID comes from the domain's own monitor entry. - run.sh stays unhealthy on a restart while $INITIALIZE_PENDING is on the volume. - Dockerfile-alpine installs coreutils, whose timeout signals the whole process group. - CI: both image jobs run .github/scripts/docker-test-replication.sh, which adds the initialize pin, held unhealthy checks, the full-list scale-down check, three fresh servers started at once, a master named by its own address, and the replicate.sh pins on the /dev/shm glob and the retry on exit 8. - README documents all of the above.
|
Round 1 is in 89627af. All five blocking issues were real, and so were the non-blocking ones; two of them are fixed differently from the snippet, and both questions got an answer in code rather than in the README. Point by point: Blocking
Non-blocking
Verified locally with the branch's |
|
Follow-up to the round above, in a82819f:
Verification status, stated plainly: the scenarios after that pin - the master named by its own address and the three servers started at once - have not completed a run yet on either image. The later local runs could not tell anything: the host was saturated by unrelated builds (load average up to ~300 on 8 cores), and the containers missed their timeouts from the first seed on. The docker jobs of this head are the first clean run of the full script; I will report on them here. |
maximthomas
left a comment
There was a problem hiding this comment.
praise: Every round-1 blocking issue is fixed where it lived, and the pending/ready state gives the join a real answer to "whose data is this".
join.sh:551-552reads the exit status ofinitialize_fromdirectly, andREPLICATION_INITIALIZE_TIMEOUT(join.sh:88, used at:287) no longer shares the enable's 120 s bound.replication_server_lists(join.sh:349-360) runs one search over everyds-cfg-replication-serverandds-cfg-replication-domainentry, socleanup_departedprunescn=schemaandcn=admin dataas well.is_self/names_matchcompare names whole and addresses only for equality, replacing the unanchored/etc/hostsgrep.
issue (blocking): Neither image job checks out .github/scripts, so the "Docker test replication" step exits 127 before any check runs.
.github/workflows/build.yml:585, :966, checkouts at :479-481, :859-861
Both jobs check out only sparse-checkout: .github/benchmark, and .github/scripts is outside that cone. In run 36587186866 at a82819f, job 109537652639 logs .github/scripts/docker-test-replication.sh: No such file or directory and Process completed with exit code 127, and the alpine job fails the same way. No scenario of the script has run in CI at this head, and merging would turn both cells red on master.
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
sparse-checkout: |
.github/benchmark
.github/scriptsissue (blocking): A volume whose first bootstrap failed or was killed has no $INITIALIZE_PENDING, so on its next start it publishes ready, joins, and turns healthy with bootstrap-only data.
opendj-packages/opendj-docker/run.sh:170-182, opendj-packages/opendj-docker/bootstrap/join.sh:482-485, :314-321
run.sh touches the marker only when BOOTSTRAPPED=true. setup.sh can exit 1 after ./data/config already exists (create-backend, makeldif and import-ldif all end in || exit 1), and a container killed mid-import never reaches :181. The restart road finds no domain and starts join.sh. join.sh sees no marker, which it reads as "never bootstrapped by run.sh", so it publishes ready, enables through a peer, and joined() writes the health marker over a partial userRoot that was never initialized. A later pending peer listed ahead of the other ready ones then initializes from it in trusted_source. README:59-60 says a failed bootstrap never reports healthy. Not run: dsreplication enable over a partially imported backend (it needs the configured base DN, not the entries).
BOOTSTRAPPED=true
# marked first, so that a bootstrap that fails or is killed half-way never
# leaves a volume that counts as holding the data of the topology
if join_requested; then
touch "$INITIALIZE_PENDING"
fi
if ! sh "${BOOTSTRAP}"; thenThen drop the touch at run.sh:180-182.
issue (blocking): On the non-pending road the first peer seeds once its retries run out, even while a ready peer answers.
opendj-packages/opendj-docker/bootstrap/join.sh:516-519, twin guard at :581
:516 runs if [ "$FIRST" = yes ]; then seed … with no any_other_trusted check. The pending road's last rule (:581) has that check, and so does the rule as the header (:54-57), :576-578 and README:114-116 state it. Take a -0 with an unmarked volume that is not a member (a restored volume, or the failed-bootstrap volume above) and a live, ready dj-1. If every enable through dj-1 fails for all rounds (killed at REPLICATION_ATTEMPT_TIMEOUT, a TLS or admin-port mismatch, or exit 5 on a userRoot the bootstrap never created), dj-0 seeds, writes .bootstrap-complete, and serves unreplicated data behind the same Service as the live topology.
if [ "$FIRST" = yes ] && ! any_other_trusted; then
seed "no topology found after $REPLICATION_RETRY_COUNT attempts"
exit 0
fiissue (blocking): A seed that no peer joined has no replication domain, so every restart makes its health wait on join.sh binding with the bootstrap ROOT_PASSWORD. Once the root password is changed, it never turns healthy again.
opendj-packages/opendj-docker/run.sh:144, opendj-packages/opendj-docker/bootstrap/join.sh:463-467, :212-214, :470-476
seed() runs no dsreplication, so config.ldif never gets ds-cfg-replication-domain, and run.sh:144 leaves the marker to join.sh. join.sh's server_up binds as ROOT_USER_DN with the env password. After 150 failures it logs "the server did not come up, giving up" and exits 1, even though the server is up. This hits a StatefulSet at replicas: 1, or an upgraded old-image master that never had a replica, once its root password has been changed over LDAP. README:34-36 says ROOT_PASSWORD is only the initial password, which is why #1092 took the health check off it. At BASE the same restart was healthy right after upgrade -n.
# join.sh, seed()
touch "$SEEDED"
# run.sh
export SEEDED=${SEEDED:-/opt/opendj/data/.replication-seeded}
if ! join_requested || { { grep -q "ds-cfg-replication-domain" ./data/config/config.ldif || [ -f "$SEEDED" ]; } && [ ! -f "$INITIALIZE_PENDING" ]; }; thenissue (non-blocking): seed() ignores a failed publish_state, so a seed whose state never reached its peers still reports itself healthy.
opendj-packages/opendj-docker/bootstrap/join.sh:463-467, :219-224
If both the replace and the --defaultAdd on the local cn=config fail during the pending road's seed (:568-570), pending stays on record. trusted() rejects it, so every other peer skips the seed and exits 1 unhealthy until the seed restarts, while the seed itself is healthy.
seed() { # <why>
echo "join: $1, seeding it with this server's data"
publish_state ready || return 1
rm -f "$INITIALIZE_PENDING"
touch "$BOOTSTRAP_COMPLETE"
}At the callers (:508, :517, :562, :569, :582), write seed "…" || exit 1.
issue (non-blocking): With REPLICATION_PEERS set, a fresh first peer seeds while peers still on the previous image answer as absent and hold the data.
opendj-packages/opendj-docker/bootstrap/join.sh:245-249, :581
any_other_trusted is built on trusted(), which rejects absent when the peers are explicit. The final seed rule therefore treats an answering old-image peer as "no peer that could hold the data", which README:113-116 says it is not. Distrusting absent as an initialize source is right, because a new-image peer answers absent while it bootstraps. Only the seed guard needs a separate predicate: "answers and is not pending". The window is narrow: pod-0 on a fresh volume while the others still run the old image.
issue (non-blocking): The deprecated rg path now calls MASTER_SERVER on the local $ADMIN_PORT, so an rg replica whose admin port differs from its master's never turns healthy.
opendj-packages/opendj-docker/bootstrap/replicate.sh:28-29 and the rg branch
At BASE the rg self call on 4444 was unchecked, and the script's exit came from the master call on 4444. Now that call goes to MASTER_SERVER:$ADMIN_PORT, fails 30 times, and run.sh sets BOOTSTRAPPED=false. README:133 says the one-shot types keep their previous behaviour. Either say there that they, like simple, need one ADMIN_PORT on every server, or keep 4444 for the master call.
issue (non-blocking): If /dev/shm is not writable, the || mktemp fallback writes the root password to /tmp/tmp.*, which run.sh's sweep never removes.
opendj-packages/opendj-docker/bootstrap/join.sh:116-118, opendj-packages/opendj-docker/run.sh:45-46
A join killed by a stop never runs its EXIT trap, so each killed start leaves one more password file in the container's writable layer. The comment at join.sh:113-115 says the opposite. setup.sh already uses a swept /tmp template.
PASSWORD_FILE=$(mktemp -p /dev/shm "opendj-join.$ADMIN_PORT.XXXXXX" 2>/dev/null || mktemp "/tmp/opendj-join.$ADMIN_PORT.XXXXXX")Also add /tmp/opendj-join."$ADMIN_PORT".* to the rm -f in run.sh:45-46.
issue (non-blocking): Moving from MASTER_SERVER=<ip> to a name-based REPLICATION_PEERS makes cleanup_departed deregister the live master.
opendj-packages/opendj-docker/bootstrap/join.sh:382-402
Under MASTER_SERVER=10.0.0.5, the replica's enable registered the master as hostname=10.0.0.5 and listed 10.0.0.5:8989, and names_match never equates an address with a name. At the replica's first start with REPLICATION_PEERS=dj-0,dj-1, the replica deletes the master's cn=Servers entry (the delete replicates everywhere) and removes it from every list. It heals at the master's next start. README:90-92 warns only about the reverse case. A migration note would do: rename the master to its DNS name and restart it first.
issue (non-blocking): A server that the survivors pruned and that is scaled back up on its retained volume can pass is_member on its stale local cn=admin data.
opendj-packages/opendj-docker/bootstrap/join.sh:264-272
If the replicated delete of its cn=Servers entry arrives after join.sh's is_member search, the join logs "already a member" and repairs nothing. run.sh:144 has already marked the server healthy. It then stays unregistered and missing from the survivors' lists until its own next restart, when the enable re-registers it. Not run: the outcome depends on when the delete arrives relative to the search, and needs a docker run of scale-down, survivor restart, and scale-up on the retained volume.
issue (non-blocking): An empty replication_server_lists result still runs one dsconfig per registered peer with an empty domain name.
opendj-packages/opendj-docker/bootstrap/join.sh:436-452
The here-string feeds one empty line, and [ -n "$value" ] && continue does not skip it. After a failed local search, the result is a log line "adding replication server X to the list of " and one failing JVM per peer, swallowed by || true.
lists=$(replication_server_lists)
[ -n "$lists" ] || return 0suggestion (non-blocking): Nothing pins "a member is ready again right after a restart, without waiting for its peers": both members restart together under a 480 s wait.
.github/scripts/docker-test-replication.sh:200-204
A mutant that gates the marker on any one peer answering stays green, at run.sh:144-145 and at join.sh's already-member exit (:486-490). That gate is the OrderedReady deadlock the run.sh:136-143 comment describes.
# a member is ready again right after a restart, without waiting for its peers
docker stop dj-1 >/dev/null
docker restart dj-0 >/dev/null
wait_until 120 "dj-0 is healthy while dj-1 is down" is_healthy dj-0
docker start dj-1 >/dev/null
wait_healthy dj-1Pin: this kills the peer-gated mutants. run.sh:145 itself is pinned only by a restart after the root password was changed over LDAP.
suggestion (non-blocking): No case makes dsreplication initialize fail, so the round-1 fix (keep the marker, stay unhealthy on a failed initialize) is unpinned.
opendj-packages/opendj-docker/bootstrap/join.sh:551-559
Every initialize in the script succeeds on 10 sample entries. initialize_from "$source" || true; rc=0 runs identically in CI and stays green.
docker run -d --memory="512m" --network $NETWORK --name dj-fi --hostname dj-fi \
-e ROOT_PASSWORD="$ROOT_PASSWORD" -e ADD_BASE_ENTRY="--addBaseEntry" \
-e OPENDJ_REPLICATION_TYPE=simple -e REPLICATION_PEERS=dj-0,dj-fi \
-e REPLICATION_INITIALIZE_TIMEOUT=1 \
-e REPLICATION_RETRY_COUNT=2 -e REPLICATION_RETRY_INTERVAL=3 "$IMAGE" >/dev/null
wait_until 300 "the initialize of dj-fi is cut off" logs_have dj-fi "initialize from dj-0 exited with 124"
stays_unhealthy dj-fi "its initialize failed"
docker exec dj-fi test -f /opt/opendj/data/.replication-initialize-pending \
|| fail "dj-fi dropped its pending marker after a failed initialize"Pin: a one-second bound cuts off the dsreplication JVM before it connects. The mutant then removes the marker and turns healthy, so stays_unhealthy and the test -f go red.
suggestion (non-blocking): The dj-sdsr exit-8 pin depends on a race of about 5 s.
.github/scripts/docker-test-replication.sh:289-291
The test detects "Will sleep for a bit" with a 2 s poll and only then disconnects dj-0. BASE polled every 0.2 s. replicate.sh:82-84 sleeps 5 s before its first enable. On a loaded runner the enable can reach dj-0 first, and then both cells go red even though the code is correct. Not measured. A deterministic order: disconnect dj-0 before starting dj-sdsr, wait for "exited with 8, trying again", then reconnect it.
…ry start of the Docker image
…gy's data, and clean every replication list Review round 1 of OpenIdentityPlatform#1115: - join.sh: a failed dsreplication initialize is no longer taken for a success (the exit status was that of an if without else), and it is bounded by its own REPLICATION_INITIALIZE_TIMEOUT, none by default; REPLICATION_ATTEMPT_TIMEOUT bounds the enable only, and the bound sits on the dsreplication call, as timeout cannot run a shell function. - join.sh publishes pending/ready in the local entry cn=Docker Join,cn=config; a pending server enables and initializes only through a ready peer, the first peer seeds at once when every other one is pending and after the retries only while no peer that could hold the data answers. Fresh servers started together no longer take each other's bootstrap data for the topology's. - The cleanup prunes departed servers from every replication list (replication server, BASE_DN, cn=schema, cn=admin data), and the lists get every listed, registered peer they lack; dsconfig inside the read loops reads /dev/null. - Self-recognition: a name equal to hostname -f or cut from it at a dot, and the container's own addresses and their /etc/hosts names, compared whole; is_member checks every registered hostname; the generation ID comes from the domain's own monitor entry. - run.sh stays unhealthy on a restart while $INITIALIZE_PENDING is on the volume. - Dockerfile-alpine installs coreutils, whose timeout signals the whole process group. - CI: both image jobs run .github/scripts/docker-test-replication.sh, which adds the initialize pin, held unhealthy checks, the full-list scale-down check, three fresh servers started at once, a master named by its own address, and the replicate.sh pins on the /dev/shm glob and the retry on exit 8. - README documents all of the above.
… out of command substitutions set -E hands the ERR trap to every $(...): a command that failed there, as the dsreplication enable the replicate.sh pin expects to fail, dumped the container logs into the captured output and removed the containers under the running test. The trap now acts in the main shell only. join.sh waits for the peers to register with one search a round instead of one per peer: every ldapsearch starts a JVM.
…p, keep a seed ready across restarts, and take turns to enable Review round 2 of OpenIdentityPlatform#1115: - both image jobs check out .github/scripts, so the replication test runs at all - run.sh marks the volume pending before the bootstrap, so a bootstrap that failed or was killed is initialized from the topology rather than published ready - on a restart the volume is ready unless it is pending; a seed that no peer joined has no replication domain, and the join cannot bind once the root password was changed - the first peer seeds after its retries on either road only while no other peer answers that may hold the data: ready, or publishing no state but replicating BASE_DN - the state reaches the peers before the pending marker goes, or the join fails - a server the survivors removed while it was away takes its replication configuration down and enables anew: dsreplication enable does not register a server whose replicated cn=admin data already matches the peer's - servers registered by an address are not removed as departed - the /tmp fallback of the password files has a name run.sh removes - an empty list of replication servers changes nothing - joins through one peer take turns, with a lock entry in the peer's cn=config: two enable runs at once leave the loser a member only in part, for good - README: one ADMIN_PORT for the one-shot types, address-registered servers, the lock - CI: a member ready while its peer is down, a seed ready after its root password changed, a failed bootstrap and a failed initialize, a scale-up on a retained volume, no lock left after the parallel start, and sdsr's exit 8 without a race: the replica starts on a network of its own rather than dj-0 leaving the topology's, which left dj-0's replication server a dead connection of its own directory server to route the replica's initialize into
a82819f to
df083d9
Compare
|
Round 2 is in df083d9, rebased onto the current master (3295ece). All four blocking issues were real, and so were the non-blocking ones; three are fixed differently from the snippet, and the local runs turned up two more faults, fixed here as well. Point by point: Blocking
Non-blocking
Suggestions
Found in the local runs
Verification: |
Fixes #1086.
The container joined its replication topology once, during the first bootstrap only, through a single
MASTER_SERVERit recognised by an unanchoredgrepof/etc/hosts, and a join that failed turned into a healthy, unreplicated server on the next restart, because a restart wrote the health marker right afterupgrade -n. A StatefulSet cannot rely on any of that (#1086, discussion #1079).The join becomes a background step next to the server, which stays PID 1 of the container (#1085):
bootstrap/join.sh(new) runs in the background on every start and servesOPENDJ_REPLICATION_TYPE=simple.REPLICATION_PEERS=host1,host2,…(DNS names), which a chart derives from the StatefulSet ordinals;MASTER_SERVERkeeps working as a one-element list. The server recognises itself by a name equal tohostname -for tohostname -fcut at a dot (a pod listed as<sts>-N.<svc>has the FQDN<sts>-N.<svc>.<ns>.svc.…), and by its own addresses and the names/etc/hostsgives them — compared whole. Thegrepof/etc/hostsit replaces tookopendj-1foropendj-10, a replica for its master when an--add-hostnamed the master, and a master that lost its volume for a replica of itself.dsreplication enablecovers "already replicated" and "BASE_DNnot found on one of the servers" alike, so the step instead checks the replication domain forBASE_DNincn=configand the registration incn=admin data, and until both are there it tries again, a bounded, configurable number of times, eachenableunder its owntimeout(REPLICATION_ATTEMPT_TIMEOUT), since it can hang on a peer that stops mid-operation.dsreplication initialize— a full import — has a bound of its own,REPLICATION_INITIALIZE_TIMEOUT, none by default.dsreplication enableruns through the same server at once break each other — both create the replication server a seed does not have yet, the loser fails half-way (exit 17) and every later enable exits 5 without ever completing its membership. An enable therefore holds a lock on the peer it runs through, the entrycn=Docker Join Lock,cn=config, which only one add creates; a lock left by a killed join is broken once it is older thanREPLICATION_ATTEMPT_TIMEOUTplus a minute, by a delete that asserts the value it read.ADD_BASE_ENTRY/SAMPLE_DATA), and two freshly bootstrapped volumes even share a generation ID, so "doesBASE_DNhave entries" decides nothing.run.shmarks a volume before it bootstraps it, and the join publishes that to the peers in a local, non-replicated entry,cn=Docker Join,cn=config:pendinguntil the volume received the data of the topology,readyafterwards — published before the marker goes, so the two never tell different stories. A pending server joins and initializes only through a ready peer — however many restarts that takes — and cross-checks the generation IDs. So servers may also start together (Compose,podManagementPolicy: Parallel) without one taking another's bootstrap data for the topology's, and a bootstrap that failed or was killed half-way is initialized from the topology rather than taken for its data. Where onlyMASTER_SERVERis set, a peer that publishes no state (an older image) counts as ready.REPLICATION_PEERSmay declare that there is no topology and seed it with its own data — at once when every other peer answers and is pending, otherwise only once its retries are exhausted while no other peer answers that may hold the data: a ready one, or one that publishes no state but replicatesBASE_DN(a server of an earlier image). Anyone else stays unhealthy, which surfaces a lost topology instead of forking it. A-0that lost its volume therefore rejoins through the survivors and takes the data of the topology back, instead of bootstrapping an empty, unreplicated server behind the same Service. The residual risk — every other server down and the first peer's volume lost — is documented in the README.REPLICATION_PEERSset explicitly, a joined server removes every server registered incn=admin databut no longer listed, and prunes it from everyreplication-serverlist it holds — that of its replication server and those of the domains ofBASE_DN,cn=schemaandcn=admin data. A server registered by an address (a master thatMASTER_SERVER=<address>named) is left alone, as nothing tells it from a listed name. It also adds every listed, registered peer its lists lack, so joins that ran at the same time cannot leave two replication servers unaware of each other.dsreplication disablein apreStophook could not do this: it fires on every termination — rolling update, drain — and changes only the servers it can reach, while OpenDJ 4 has no cleanup subcommand for a dead one. With onlyMASTER_SERVERset nothing is removed or added, so servers joined by hand stay.dsreplication enablebetween two servers whosecn=admin datais replicated registers nobody. When a peer that answers does not register it, it takes its own replication configuration down (dsreplication disable --disableAll) and enables again, which registers it.run.sh: on a restart the health marker follows the upgrade unless the volume is still pending — a server whose volume holds the data of the topology is ready as soon as it serves. Gating it on its peers would deadlock a whole-cluster restart underOrderedReady, and gating it on the join would keep a seed that no peer joined yet (it has no replication domain) unready once its root password was changed, since the join binds withROOT_PASSWORD. A volume whose bootstrap, join or initialize never completed still carries the marker and waits for the join, so none of these turns into a healthy server with bootstrap data only.bootstrap/replicate.shkeeps the one-shotsrs,sdsrandrgpaths as they are (deprecated in the README), with$ADMIN_PORT/$REPLICATION_PORTin place of the hardcoded4444/8989— so, likesimple, they need oneADMIN_PORTon every server.Dockerfile-alpineinstallscoreutils: thetimeoutof BusyBox signals only the shell script that starts java, that of coreutils the whole process group..github/scripts/docker-test-replication.shwith their own image (and check out.github/scriptsfor it). It covers the seed decision, the retry while the peer is unreachable, the initialize (entries the seed only ever imported), a member ready again while its peer is down, a seed ready again after its root password changed, a bootstrap that failed after its import, an initialize that failed, a failed join staying unhealthy across a restart, the seed losing its volume, a scale-up, a scale-down after which the survivors have dropped the removed server from every list, the removed server scaled up again on its volume, three fresh servers started at once (and no join lock left behind), a master named by its own address, the shared-/dev/shmhygiene of a Kubernetes pod, and the deprecatedsdsrpath with its retry on exit 8.BASE_DN,ROOT_USER_DN,ROOT_PASSWORD,ADMIN_PORT,REPLICATION_PORT), how a server recognises itself, the retry and timeout knobs, the published state, the join lock, the seed rule with its residual risk, and the cleanup.No password reaches a command line (#1084): the tools read it from a file on
/dev/shm(or on/tmpunder a namerun.shremoves), andrun.shremoves what a killed join orreplicate.shleaves there, by theADMIN_PORTin the name, keeping the files of the other containers of the pod.Verified locally by running
.github/scripts/docker-test-replication.shin full over Debian and Alpine images built the way the image jobs build them, from a server package of the branch: both passed every scenario (on the base before #1116/#1117/#1118, which change no Docker file; the docker jobs of this head run the same script on the current base).