From 011b25d8038e0f0bd3272fa57b0c7e068feb130c Mon Sep 17 00:00:00 2001 From: Nitzan Mordechai Date: Sun, 7 Dec 2025 09:06:14 +0000 Subject: [PATCH] test/encoding/readable: Add backward incompat checks The readable.sh script has forward incompat checks, but no backward incompat checks. This fix will: 1. Add check for backward_incompat directory for each type for specific objects or all objects with the same type and skip those objects from being tested. 2. Add version comparison helper functions (version_lt, version_le, version_ge, versions_span) for robust version handling 3. Replace 'sort -n' with 'sort -V' for proper version number sorting 4. Add CORPUS_PATH environment variable to allow teuthology tests to execute this script 5. Improve readability of the script The difference between backward and forward incompat: - forward_incompat: Marks objects from older versions that newer ceph-dencoder versions cannot read. Example: Version 19.2.x objects marked incompat at version 20.2.x means ceph-dencoder v20.2.x+ can't decode them. Skip when testing old objects with a new ceph-dencoder. - backward_incompat: Marks objects from newer versions that older ceph-dencoder versions cannot read. Example: Version 19.2.x objects marked backward_incompat at v19.2.x means ceph-dencoder < v19.2.x can't decode them. Skip when testing new objects with an old ceph-dencoder. Fixes: https://tracker.ceph.com/issues/74074 Signed-off-by: Nitzan Mordechai --- src/test/encoding/readable.sh | 179 +++++++++++++++++++++++++--------- 1 file changed, 131 insertions(+), 48 deletions(-) diff --git a/src/test/encoding/readable.sh b/src/test/encoding/readable.sh index 230043a9630..b05b3e55c93 100755 --- a/src/test/encoding/readable.sh +++ b/src/test/encoding/readable.sh @@ -22,80 +22,164 @@ else CEPH_DENCODER=ceph-dencoder fi -myversion=`$CEPH_DENCODER version` +myversion=$($CEPH_DENCODER version) +echo "Using ceph-dencoder version $myversion" +if [ -z "$myversion" ]; then + echo "Failed to get version from $CEPH_DENCODER" + exit 1 +fi DEBUG=0 WAITALL_DELAY=.1 debug() { if [ "$DEBUG" -gt 0 ]; then echo "DEBUG: $*" >&2; fi } +version_le() { + [ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | head -n1)" = "$1" ] +} + +version_lt() { + version_le "$1" "$2" && [ "$1" != "$2" ] +} + +version_ge() { + [ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | tail -n1)" = "$1" ] +} + +versions_span() { + local left=$1 + local right=$2 + local pivot=$3 + + if version_le "$left" "$pivot" && version_ge "$right" "$pivot"; then + return 0 + fi + if version_le "$right" "$pivot" && version_ge "$left" "$pivot"; then + return 0 + fi + return 1 +} + test_object() { local type=$1 local output_file=$2 local failed=0 local numtests=0 - tmp1=`mktemp /tmp/test_object_1-XXXXXXXXX` - tmp2=`mktemp /tmp/test_object_2-XXXXXXXXX` + tmp1=$(mktemp /tmp/test_object_1-XXXXXXXXX) + tmp2=$(mktemp /tmp/test_object_2-XXXXXXXXX) rm -f $output_file if $CEPH_DENCODER type $type 2>/dev/null; then #echo "type $type"; echo " $vdir/objects/$type" - # is there a fwd incompat change between $arversion and $version? - incompat="" - incompat_paths="" + # Check for forward and backward incompat changes + forward_versions="" + forward_paths="" + backward_incompat="" + backward_incompat_paths="" sawarversion=0 - for iv in `ls $dir/archive | sort -n`; do + for iv in $(ls "$dir/archive" | sort -V); do if [ "$iv" = "$arversion" ]; then sawarversion=1 fi - if [ $sawarversion -eq 1 ] && [ -e "$dir/archive/$iv/forward_incompat/$type" ]; then - incompat="$iv" - - # Check if we'll be ignoring only specified objects, not whole type. If so, remember - # all paths for this type into variable. Assuming that this path won't contain any - # whitechars (implication of above for loop). - if [ -d "$dir/archive/$iv/forward_incompat/$type" ]; then - if [ -n "`ls $dir/archive/$iv/forward_incompat/$type/ | sort -n`" ]; then - incompat_paths="$incompat_paths $dir/archive/$iv/forward_incompat/$type" + # Record forward incompatibility markers + marker_path="$dir/archive/$iv/forward_incompat/$type" + if [ -e "$marker_path" ]; then + if [ -d "$marker_path" ]; then + if [ -n "$(ls "$marker_path"/ 2>/dev/null )" ]; then + forward_paths="$forward_paths $iv:$marker_path" else echo "type $type directory empty, ignoring whole type instead of single objects" - fi; + forward_versions="$forward_versions $iv" + fi + else + forward_versions="$forward_versions $iv" fi fi - if [ "$iv" = "$version" ]; then - rm -rf $tmp1 $tmp2 - break + # Check for backward incompatibility (affects this arversion) + # backward_incompat at version X means decoders < X can't decode objects from X onwards + # Check for backward_incompat in versions UP TO AND INCLUDING arversion + if [ $sawarversion -eq 0 ] || [ "$iv" = "$arversion" ]; then + if [ -e "$dir/archive/$iv/backward_incompat/$type" ]; then + backward_incompat="$iv" + + # Check if we'll be ignoring only specified objects, not whole type + if [ -d "$dir/archive/$iv/backward_incompat/$type" ]; then + if [ -n "$(ls $dir/archive/$iv/backward_incompat/$type/ 2>/dev/null )" ]; then + backward_incompat_paths="$backward_incompat_paths $dir/archive/$iv/backward_incompat/$type" + else + echo "type $type directory empty, ignoring whole type instead of single objects" + fi; + fi + fi fi + done - if [ -n "$incompat" ]; then - if [ -z "$incompat_paths" ]; then - echo "skipping incompat $type version $arversion, changed at $incompat < code $myversion" - rm -rf $tmp1 $tmp2 - return - else - # If we are ignoring not whole type, but objects that are in $incompat_path, - # we don't skip here, just give info. - echo "postponed skip one of incompact $type version $arversion, changed at $incompat < code $myversion" - fi; + # Skip if forward incompatibility places archive and decoder on opposite sides of a change + if [ -n "$forward_versions" ]; then + for forward_version in $forward_versions; do + if versions_span "$myversion" "$arversion" "$forward_version"; then + if version_lt "$myversion" "$forward_version"; then + echo "skipping forward incompat $type version $arversion, requires decoder >= $forward_version, current decoder is $myversion" + else + echo "skipping forward incompat $type version $arversion, decoder >= $forward_version incompatible with objects < $forward_version (current decoder is $myversion)" + fi + rm -f $tmp1 $tmp2 + return + fi + done + fi + + # Skip if our decoder is too old for backward compatibility requirement + # Only skip whole type if NO per-object markers exist (like forward_incompat) + if [ -n "$backward_incompat" ] && [ -z "$backward_incompat_paths" ]; then + # Use sort -V for proper version comparison (handles 19.5 vs 19.10 correctly) + if version_lt "$myversion" "$backward_incompat"; then + echo "skipping backward incompat $type version $arversion, requires decoder >= $backward_incompat, current decoder is $myversion" + rm -f $tmp1 $tmp2 + return + fi fi - for f in `ls $vdir/objects/$type`; do + for f in $(ls "$vdir/objects/$type"); do skip=0; # Check if processed object $f of $type should be skipped (postponed skip) - if [ -n "$incompat_paths" ]; then - for i_path in $incompat_paths; do + # Check both forward_incompat and backward_incompat paths + if [ -n "$forward_paths" ]; then + for entry in $forward_paths; do + marker_version=${entry%%:*} + marker_path=${entry#*:} + if versions_span "$myversion" "$arversion" "$marker_version"; then + if [ -L "$marker_path/$f" ]; then + if version_lt "$myversion" "$marker_version"; then + echo "skipping object $f of type $type (forward incompat requires decoder >= $marker_version, current is $myversion)" + else + echo "skipping object $f of type $type (forward incompat for decoder >= $marker_version, current is $myversion)" + fi + skip=1 + break + fi + fi + done + fi; + + if [ -n "$backward_incompat_paths" ]; then + # Only skip individual objects marked as backward_incompat if decoder is too old + # Use sort -V for proper version comparison + if version_lt "$myversion" "$backward_incompat"; then + for b_path in $backward_incompat_paths; do # Check if $f is a symbolic link and if it's pointing to existing target - if [ -L "$i_path/$f" ]; then - echo "skipping object $f of type $type" + if [ -L "$b_path/$f" ]; then + echo "skipping object $f of type $type (backward incompat)" skip=1 break fi; done; + fi fi; if [ $skip -ne 0 ]; then @@ -108,13 +192,13 @@ test_object() { pid2="$!" #echo "\t$vdir/$type/$f" if ! wait $pid1; then - echo "**** failed to decode $vdir/objects/$type/$f ****" + echo "**** failed to decode type $type from archive $arversion object $f (path $vdir/objects/$type/$f) ****" failed=$(($failed + 1)) rm -f $tmp1 $tmp2 - continue + continue fi if ! wait $pid2; then - echo "**** failed to decode+encode+decode $vdir/objects/$type/$f ****" + echo "**** failed to decode+encode+decode type $type from archive $arversion object $f (path $vdir/objects/$type/$f) ****" failed=$(($failed + 1)) rm -f $tmp1 $tmp2 continue @@ -126,9 +210,9 @@ test_object() { # nothing. if ! $CEPH_DENCODER type $type is_deterministic; then echo " sorting json output for nondeterministic object" - for f in $tmp1 $tmp2; do - sort $f | sed 's/,$//' > $f.new - mv $f.new $f + for tmpfile in $tmp1 $tmp2; do + sort $tmpfile | sed 's/,$//' > $tmpfile.new + mv $tmpfile.new $tmpfile done fi @@ -138,7 +222,7 @@ test_object() { failed=$(($failed + 1)) fi numtests=$(($numtests + 1)) - rm -f $tmp1 $tmp2 + rm -f $tmp1 $tmp2 done else echo "skipping unrecognized type $type" @@ -197,17 +281,17 @@ do_join() { # Using $MAX_PARALLEL_JOBS jobs if defined, unless the number of logical # processors -if [ `uname` == FreeBSD -o `uname` == Darwin ]; then - NPROC=`sysctl -n hw.ncpu` +if [ $(uname) == FreeBSD -o $(uname) == Darwin ]; then + NPROC=$(sysctl -n hw.ncpu) max_parallel_jobs=${MAX_PARALLEL_JOBS:-${NPROC}} else max_parallel_jobs=${MAX_PARALLEL_JOBS:-$(nproc)} fi -output_file=`mktemp /tmp/output_file-XXXXXXXXX` +output_file=$(mktemp /tmp/output_file-XXXXXXXXX) running_jobs=0 -for arversion in `ls $dir/archive | sort -n`; do +for arversion in $(ls $dir/archive | sort -V); do vdir="$dir/archive/$arversion" #echo $vdir @@ -215,7 +299,7 @@ for arversion in `ls $dir/archive | sort -n`; do continue; fi - for type in `ls $vdir/objects`; do + for type in $(ls $vdir/objects); do test_object $type $output_file.$running_jobs & pids="$pids $!" running_jobs=$(($running_jobs + 1)) @@ -243,4 +327,3 @@ if [ $numtests -eq 0 ]; then fi echo "passed $numtests tests." - -- 2.47.3