Repository navigation
gpdemo: write an absolute TRUSTED_SHELL into the generated config - #2177
Open
MisterRaindrop wants to merge 1 commit into
Open
MisterRaindrop wants to merge 1 commit into
MisterRaindrop wants to merge 1 commit into
Conversation
demo_cluster.sh writes the cluster config through an unquoted here-doc,
so
TRUSTED_SHELL="$(dirname "$0")/lalshell"
is expanded as the file is written, not as it is sourced. gpAux/gpdemo's
Makefile invokes the script as ./demo_cluster.sh, which makes $0
relative and dirname "." -- so the generated config carries
TRUSTED_SHELL="./lalshell"
gpinitsystem sources that file from wherever it happens to be running
and looks for lalshell in that directory instead.
Nothing noticed while the demo cluster was only ever created by
demo_cluster.sh itself, from the directory lalshell sits in, but the
Behave suite runs
gpinitsystem -a -c ../gpAux/gpdemo/clusterConfigFile
directly in a dozen scenarios. Resolve the directory the script itself
lives in and write that, and keep the quotes so a path containing
whitespace survives being sourced.
This was referenced Oct 24, 2026
Draft
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
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.
What does this PR do?
demo_cluster.shwrites the cluster config through an unquoted here-doc, sois expanded when the file is written, not when it is sourced.
gpAux/gpdemo's Makefile and the Behave suite both run the script as./demo_cluster.sh, so$0is relative,dirnamegives., and the generated config saysgpinitsystem sources that file from wherever it is running and looks for
lalshellin that directory. That never mattered while onlydemo_cluster.shitself ran gpinitsystem fromgpAux/gpdemo, but the Behave suite runsgpinitsystem -c ../gpAux/gpdemo/clusterConfigFilefromgpMgmt, which fails withThe script now writes the absolute path of the directory it lives in, still quoted so a path containing whitespace survives being sourced.
Split out of #1639 (originally 767f126, by @tuhaihe).
Test Plan
Rocky 9 build image, built from #2170's tree (the Behave CI workflow), demo cluster created the way CI creates it, then
behave --tags ~@needs_fix test/behave/mgmt_utils/gpinitsystem.featurewith #2170's feature file from before it tagged the scenarios below:mainTRUSTED_SHELLin the generated config"./lalshell""/home/gpadmin/cloudberry/gpAux/gpdemo/lalshell"./lalshell: No such file or directoryin the runWith this change, these scenarios go from failing to passing: the three locale scenarios (
gpinitsystem.featurelines 302, 312, 320 in #2170), the two banner scenarios (335, 342),-D(355), the defaulttrusted_shell(362), and the three@extendedbackout-file scenarios (84, 99, 113). The@extendedones are excluded in CI, but they ran here.The one that still fails, "should create consistent port entry on segments postgresql.conf file" (348), now gets past gpinitsystem and fails on the next check: a mirror's
postgresql.confstill has its primary's port (expected15437, got15434). That is a separate gpinitsystem defect, fixed by b971d48 in #1639, which will come as its own PR. The changed-trusted_shellscenario (369) setsTRUSTED_SHELL=sshand does not uselalshell. It passes in CI and passed with this change here. In themainrun here it failed becausegpcreateseg.shexited on an INT/TERM signal, after the scenarios before it had failed; I have not looked further into that.#2170 tags these scenarios
@needs_fixand points them at this change. Whichever of the two PRs lands second removes the tags; 348 keeps its tag until b971d48 lands.Type of Change
Checklist