CI: add gpMgmt Behave test - #1639
Draft
tuhaihe wants to merge 23 commits into
Draft
Conversation
tuhaihe
marked this pull request as draft
March 25, 2026 08:15
tuhaihe
force-pushed
the
add-behave-test-ci
branch
3 times, most recently
from
June 24, 2026 08:00
0e824bd to
c0fd832
Compare
tuhaihe
force-pushed
the
add-behave-test-ci
branch
9 times, most recently
from
September 11, 2026 23:12
3de58e8 to
e617f85
Compare
Run the gpMgmt Behave suite on Rocky Linux 9, split by management command so that a failure in one command's feature file does not mask or pollute the others. The workflow builds an RPM once and fans out to a per-command matrix, each entry creating its own demo cluster. It is kept separate from the main build/installcheck workflow so that matrix expansion, environment setup and result parsing for Behave can evolve without disturbing the primary CI path. gpcheckperf is deliberately not in the matrix. Every scenario in gpcheckperf.feature is tagged @concourse_cluster -- it needs a multi-host cluster -- so a single-host entry filtering on "--tags ~@concourse_cluster" selects nothing and reports success without having run a single scenario. Greenplum's pipeline has the same single-host gpcheckperf job and it is empty there too; a green check that tests nothing is worse than no check at all. It can be added back when there is CI that can host a real multi-node cluster. demo_cluster.sh needs an absolute TRUSTED_SHELL path: the generated cluster config is later sourced by gpinitsystem, where $0 is no longer demo_cluster.sh and the relative path no longer resolves.
demo_cluster.sh writes the cluster config with
TRUSTED_SHELL="$(dirname "$0")/lalshell"
left unexpanded, so the path is resolved when the file is sourced rather
than when it is written. gpinitsystem sources it, and by then $0 is
gpinitsystem, not demo_cluster.sh, so TRUSTED_SHELL points at a lalshell
that is not there.
Nothing noticed while the demo cluster was only ever created by
demo_cluster.sh itself, but the Behave suite runs
gpinitsystem -a -c ../gpAux/gpdemo/clusterConfigFile
directly in a dozen scenarios. Expand the path at write time.
Back-port from Greenplum a check Cloudberry was missing. mix_distribution_policy reports tables whose distribution policy mixes legacy and non-legacy hash opclasses, and cross-checks the result against the gp_use_legacy_hashops GUC so the operator is told which of the two states is inconsistent. Add the SQL fixtures the corresponding Behave scenarios load. Greenplum's companion ao_lastrownums check is deliberately not brought over: it reads pg_attribute_encoding.lastrownums, a column Cloudberry's catalog does not have, so the query errors out on every run and gpcheckcat reports failure on a freshly created database.
CREATE_QES_MIRROR seeds the mirror with pg_basebackup, which copies the primary's postgresql.conf verbatim -- including the primary's port. The mirror was then started with START_QE before that port was corrected, so on a single host it either bound the wrong port or collided with its own primary. Rewrite postgresql.conf before starting the segment. Greenplum has the same SED_PG_CONF call but places it after START_QE, which leaves the same window open there. Also emit explicit 127.0.0.1/32 and ::1/128 trust entries in the coordinator's pg_hba.conf. The hostname-based entry alone is not enough when the loopback address the client picks depends on the host's IPv4 and IPv6 resolution order.
Cloudberry keeps the promote_trigger_file GUC that PostgreSQL 16 removed, because gpactivatestandby's force path promotes a standby coordinator by creating the trigger file and then starting the server in utility mode. CheckForStandbyTrigger no longer honoured the GUC, so that path never promoted. Restore the check, and close the race it exposes in the postmaster: with hot_standby off, PM_STATUS_STANDBY is reported as soon as recovery starts, which "pg_ctl -w" treats as ready. gpstart would then connect before promotion finished and fail with "the database system is not accepting connections". Extend the existing promotion_requested guard to cover a configured trigger file that is already present, so pg_ctl waits for PM_STATUS_READY.
minirepro asks gp_dump_query_oids which objects a query touches, and for a query over a view the answer is the view alone. The resulting dump cannot reproduce the plan, because the base tables and -- more to the point -- their statistics are missing. Expand view dependencies through pg_rewrite so the tables behind a view are dumped alongside it, and build the SQL literal by doubling quotes instead of calling Escape(), which mangled non-ASCII query text.
PyGreSQL's escape_string is a C function that encodes its argument as ASCII, so it raises UnicodeEncodeError on any table or schema name containing non-ASCII characters, and analyzedb exits with status 2 on a database it can otherwise analyze perfectly well. With standard_conforming_strings on -- the default -- doubling single quotes is the whole of the escaping a string literal needs, so build the literal directly. Exercised by the "analyzedb can handle the table name with special utf-8 characters" Behave scenario.
The Behave suite had drifted from the one it was forked from: scenarios added upstream for gprecoverseg, gpmovemirrors, gpaddmirrors, gpexpand, gpcheckcat, gpstop and minirepro were missing, several existing ones had diverged, and a number of step definitions carried only one of the @given/@when/@then decorators Greenplum registers, so features using "And" could not match them and Behave reported the steps as undefined even though the implementation was right there. Bring the feature files and their step definitions back in line with github.com/greenplum-db/gpdb-archive, keeping the Cloudberry-specific adjustments where the two products genuinely differ. Differential recovery is the one place they differ outright: Cloudberry has no "gprecoverseg --differential", so those scenarios cannot pass here. Tag them @Differential and filter them out of the gprecoverseg matrix entry, splitting the Examples tables that mix differential with full and incremental so the other variants keep running. The scenarios stay in place, aligned with Greenplum, ready to be enabled with the feature.
pg_basebackup and pg_rewind both copy the source segment's postgresql.conf, port setting included, so a mirror recovered by gprecoverseg came back configured for its primary's port. On a multi-host cluster the wrong port is merely wrong; on a single host it collides with the primary that is still running there. Rewrite the port before starting the segment, on both the full and the incremental path, using the existing ModifyConfSetting command -- its perl expression is anchored at the start of the line, so it touches the port setting and nothing else. Greenplum does the same in the same two places. Fixes the eight gprecoverseg Behave scenarios that assert "port value in postgresql.conf of <host> is incorrect".
Each recovering segment writes pg_basebackup or pg_rewind output to gpAdminLogs/<tool>.<timestamp>.dbid<N>.out, and gprecoverseg deleted that file as soon as the segment came back successfully. Deleting it on success is backwards: those files are what gpstate reads to report recovery progress, and what an operator reads afterwards to see what a recovery actually did. Remove stale progress files for the segments about to be recovered before starting, and leave the run's own files in place, the way Greenplum does. Clearing beforehand is also what makes the file a reliable record of the current run rather than an append of every run that came before. Fixes the Behave scenarios asserting gpAdminLogs holds "pg_basebackup*" and "pg_rewind*" files after a recovery.
A full resynchronisation rebuilds a mirror from its primary in place. That is not what -r asks for -- rebalance returns segments to their preferred roles -- nor what -p asks for, which is to rebuild onto a different host. gprecoverseg accepted both combinations and silently did something other than what was asked. Reject them, with the messages Greenplum uses.
Four scenarios in the matrix test behaviour the product does not have.
They are kept in the tree, aligned with Greenplum, so they can be
enabled with the feature -- but tagged and filtered out, because a job
that ships permanently red teaches everyone to ignore it.
@not_implemented, for behaviour that was never built:
- "should drop existing slot on full recovery" and the full variant of
"should not try to drop slot if slot does not exist" expect
gprecoverseg to check and drop internal_wal_replication_slot during a
full recovery. Greenplum only does that on the differential path;
FullRecovery there just hands the slot name to pg_basebackup, exactly
as Cloudberry does.
- "recovers segment for valid max-rate options" needs gprecoverseg
--max-rate, which Cloudberry does not have.
- the two "config file contains hostname" scenarios need gprecoverseg's
-i file to accept a leading hostname. Greenplum's _parseConfigFile()
takes 3, 4 or 5 parts and cross-checks hostname against address;
Cloudberry's takes 3, so the run stops at "expected 3 parts on failed
segment group, obtained 4".
@known_failure, a separate tag for behaviour that exists and is broken:
- "gpinitsystem should print FQDN in pg_hba.conf for standby when
HBA_HOSTNAMES=1" never gets a standby. CREATE_STANDBY_QD runs
gpinitstandby -s localhost --hba-hostnames and it gives up in under a
second against a coordinator that is up and answering queries,
leaving no standby data directory while gpinitsystem still exits 0.
simple_main_locked() prints the option parser's help when a
ProgramArgumentValidationException asks for it, but the parser was never
passed in -- the function set a local 'parser = None' instead. Every such
error therefore died with
AttributeError: 'NoneType' object has no attribute 'print_help'
raised from inside the except block, so the handler never reached its
'exit_status = 2' and the tool exited 1 with a stack trace. Pass the
parser down from simple_main_internal(), as Greenplum does.
Visible as 'gprecoverseg xyz' returning 1 instead of 2.
Two log lines Greenplum emits around recover_mirrors() were dropped:
- the [ERROR] 'gprecoverseg failed. Please check the output for more
details.' that follows a failed recovery. Without it a failed run
ends on the per-segment errors alone, and callers that scrape the
output -- gpmovemirrors among them -- cannot tell a failure from a
partial success.
- the notice that the pg_basebackup/pg_rewind progress files this run
wrote are left behind and a later gprecoverseg may remove them.
Asserted by gpmovemirrors.feature:136 and gprecoverseg.feature:58.
Escape() calls pgdb.escape_string(), whose str overload encodes with the
ASCII codec, so any non-ASCII identifier or value raises
UnicodeEncodeError. minirepro dies half way through writing a dump:
File ".../gppylib/utils.py", line 508, in Escape
return pgdb.escape_string(query_str)
UnicodeEncodeError: 'ascii' codec can't encode character '\xe4'
Pass UTF-8 bytes instead. libpq escapes those byte-for-byte, so the
result is identical for ASCII input and decodes back to the original
text otherwise.
Exercised by minirepro.feature:270.
getFuncs() called selectDumpableFunction() and then immediately called selectDumpableObject() on the same object, which overwrites dobj->dump with the parent namespace's dump_contains. --relation-oids sets every namespace to DUMP_COMPONENT_NONE (see selectDumpableNamespace), so a function named by --function-oids was dropped from the dump whenever --relation-oids was also given -- which is how minirepro always calls pg_dump: pg_dump -sxO db --relation-oids 0 --function-oids 17021 emitted no CREATE FUNCTION at all. Drop the second call and let selectDumpableFunction() decide, as Greenplum does. It now takes the archive so it can still give extension membership the last word, and assigns DumpComponents bitmasks rather than a bool. Exercised by minirepro.feature:282.
build_exclude_list() assembled the EXCLUDE clauses into a buffer of its own, and BaseBackup() then freed that buffer without ever putting it in the BASE_BACKUP command -- so -E/--exclude and --exclude-from were silently ignored and every excluded path was copied anyway. Worse than the wasted copying: gpinitstandby passes -E ./promote, and 'promote' is the name of PostgreSQL's promotion signal file. Copying the coordinator's promote directory into the standby left a 'promote' entry there, CheckPromoteSignal()'s stat() succeeds on a directory, and the brand new standby coordinator promoted itself the moment it reached a consistent state -- picking a new timeline instead of streaming, so pg_stat_replication on the coordinator stayed empty. Append the options to the command buffer with AppendStringCommandOption() instead. The server has always understood a repeated EXCLUDE (parse_basebackup_options collects them into one hash table); only the client half was missing. Exercised by gpinitstandby.feature:33 'gpinitstandby exclude dirs'.
TableMainColumn maps a catalog to the column that identifies the object
an entry belongs to. Greenplum has a row for pg_description and one for
pg_shdescription; Cloudberry has only the first. Without it,
processMissingDuplicateEntryResult() falls through to getOidFromPK(),
which looks the entry up by
SELECT oid FROM pg_shdescription WHERE ...
and pg_shdescription has no oid column, so missing_extraneous died with
Execution error: ERROR: column "oid" does not exist
LINE 4: SELECT oid FROM pg_shdescription
Also drop the two ao_lastrownums scenarios: that check was removed
earlier in this series because it reads pg_attribute_encoding.lastrownums,
a column Greenplum has and Cloudberry does not.
And tag 'validate session GUC passed with -x is set' @not_implemented --
its first half needs a postmaster started with gp_role=utility to refuse
non-utility connections ("System was started in single node mode - only
utility mode connections are allowed"), which Cloudberry's InitPostgres()
no longer does, so gpcheckcat connects fine and returns 0.
The loop waits for every segment to leave mode 'n'/status 'd', but the
call that asks FTS to look -- Greenplum ends each iteration with
select gp_request_fts_probe_scan()
-- is missing, so all sixty iterations re-read an unchanged
gp_segment_configuration in a couple of seconds and give up long before
gp_fts_probe_interval elapses. gpinitsystem then logs
[WARN]:-Failed to start Cloudberry instance; please review
gpinitsystem log to determine failure.
and returns 0 with all three mirrors still down. That is also why the
FATAL was commented out with a 'should support notify fts' TODO: the
function could never succeed, so the error it raised had to go instead.
Restore the probe call. Leaving the FATAL commented out for now -- that
is a separate decision about how hard gpinitsystem should fail.
Fixes the three gpinitsystem.feature scenarios that begin 'all the
segments are running' after a gpinitsystem run.
add_operator.sql declares my_pk_schema.!# with LEFTARG only -- a postfix
operator, which PostgreSQL removed in 14:
ERROR: operator right argument type must be specified
DETAIL: Postfix operators are not supported.
psql without ON_ERROR_STOP still exits 0, so the scenario's 'psql should
return a return code of 0' passed and the failure only showed up two
steps later: with no pg_operator row to delete there is nothing for
missing_extraneous to find, and gpcheckcat returned 0 where the scenario
wants 3.
RIGHTARG gives a prefix operator, which is still a pg_operator entry
identified by a non-oid primary key -- which is what 'gpcheckcat should
report and repair extra entries with non-oid primary keys' is about.
Every gpinitsystem run that is not given -e/-f logs
[WARN]:-No ETCD cluster host config provided, use default configuration.
[WARN]:-No FTS cluster host config provided, use default configuration.
and its own SCAN_LOG_FILE then greps the log for \[WARN\], so every run
ends with
[WARN]:-Scan of log file indicates that some warnings or errors
[WARN]:-were generated during the array creation
'Log file scan check passed' is therefore unreachable, and the two
gpinitsystem.feature scenarios that assert it can never pass. Neither
message reports a problem -- they say which configuration is in use --
so log them at INFO, which the scan already ignores.
Scenarios routinely begin by emptying gpAdminLogs, so by the time a feature finishes the logs belonging to the scenario that failed have been deleted several times over and the CI artifact says nothing about it. Chasing an intermittent gprecoverseg failure through three CI runs turned up exactly one surviving recovery log, from a scenario that had passed. On failure, copy gpAdminLogs, gp_segment_configuration and every segment's postgresql.conf into build-logs/failed-scenarios/<line>.<name>/ while they still describe the failure. Wrapped so that a problem collecting diagnostics can never turn one failure into a different one.
tuhaihe
force-pushed
the
add-behave-test-ci
branch
from
September 12, 2026 01:39
56b073c to
0d5e452
Compare
'check segment conf: postgresql.conf' rsyncs every segment's
postgresql.conf to a local path named after the segment's host. On a
single-host demo cluster all of them are 'cdw', so all eight land on one
file -- and rsync's quick check skips a transfer whose size and mtime
match the destination. Segments initialised by the same gpinitsystem run
share an mtime, and their port lines are the same length (port=7003 vs
port=7006), so the transfer is skipped and the check reads the previous
segment's file:
port value in postgresql.conf of cdw is incorrect.
Expected:'7006', given:'7003'
The snapshot this series now takes when a scenario fails shows every
segment holding its own correct port moments later, dbid6 included -- the
file was never wrong, the check was reading the wrong one. It looked
intermittent because it depends on two segments' files agreeing on size
and mtime.
Greenplum has the same step, but runs it on a multi-host concourse
cluster where the hostnames differ and the collision cannot happen.
Name the copy after the dbid as well, and say which dbid in the error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions