Repository navigation
gpdemo: write an absolute TRUSTED_SHELL into the generated config - #2177
Merged
tuhaihe merged 2 commits intoOct 28, 2026
Merged
Conversation
This was referenced Oct 24, 2026
Draft
tuhaihe
approved these changes
Oct 26, 2026
Alena0704
reviewed
Oct 26, 2026
Alena0704
reviewed
Oct 28, 2026
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.
Co-authored-by: Alena Rybakina <58230554+Alena0704@users.noreply.github.com>
tuhaihe
force-pushed
the
fix/gpdemo-absolute-trusted-shell
branch
from
October 28, 2026 09:51
5b15fc6 to
7fc39e5
Compare
tuhaihe
enabled auto-merge (squash)
October 28, 2026 09:54
MisterRaindrop
added a commit
to MisterRaindrop/cloudberry
that referenced
this pull request
Oct 29, 2026
cluster config held a relative TRUSTED_SHELL, so gpinitsystem could not find lalshell from the directory the scenarios run in. apache#2177 made it absolute, so remove those tags. Two of them, the banner scenarios, are tagged @backup_restore_bashrc because they append to ~/.bashrc. before_scenario() returns early for the gpinitsystem feature before it backs ~/.bashrc up, while after_scenario() still restores it, and with no backup to restore it deletes ~/.bashrc. Back it up before that early return. Alena Rybakina found this in review. The mirror port scenario also waits for the gpinitsystem fixes in apache#2178 and keeps its tag until that is merged.
MisterRaindrop
added a commit
to MisterRaindrop/cloudberry
that referenced
this pull request
Oct 29, 2026
cluster config held a relative TRUSTED_SHELL, so gpinitsystem could not find lalshell from the directory the scenarios run in. apache#2177 made it absolute, so remove those tags. Two of them, the banner scenarios, are tagged @backup_restore_bashrc because they append to ~/.bashrc. before_scenario() returns early for the gpinitsystem feature before it backs ~/.bashrc up, while after_scenario() still restores it, and with no backup to restore it deletes ~/.bashrc. Back it up before that early return. Alena Rybakina found this in review. The mirror port scenario also waits for the gpinitsystem fixes in apache#2178 and keeps its tag until that is merged.
MisterRaindrop
added a commit
to MisterRaindrop/cloudberry
that referenced
this pull request
Oct 30, 2026
cluster config held a relative TRUSTED_SHELL, so gpinitsystem could not find lalshell from the directory the scenarios run in. apache#2177 made it absolute, so remove those tags. Two of them, the banner scenarios, are tagged @backup_restore_bashrc because they append to ~/.bashrc. before_scenario() returns early for the gpinitsystem feature before it backs ~/.bashrc up, while after_scenario() still restores it, and with no backup to restore it deletes ~/.bashrc. Back it up before that early return. Alena Rybakina found this in review. The mirror port scenario also waits for the gpinitsystem fixes in apache#2178 and keeps its tag until that is merged.
tuhaihe
pushed a commit
to MisterRaindrop/cloudberry
that referenced
this pull request
Oct 31, 2026
cluster config held a relative TRUSTED_SHELL, so gpinitsystem could not find lalshell from the directory the scenarios run in. apache#2177 made it absolute, so remove those tags. Two of them, the banner scenarios, are tagged @backup_restore_bashrc because they append to ~/.bashrc. before_scenario() returns early for the gpinitsystem feature before it backs ~/.bashrc up, while after_scenario() still restores it, and with no backup to restore it deletes ~/.bashrc. Back it up before that early return. Alena Rybakina found this in review. The mirror port scenario also waits for the gpinitsystem fixes in apache#2178 and keeps its tag until that is merged.
tuhaihe
pushed a commit
that referenced
this pull request
Nov 1, 2026
cluster config held a relative TRUSTED_SHELL, so gpinitsystem could not find lalshell from the directory the scenarios run in. #2177 made it absolute, so remove those tags. Two of them, the banner scenarios, are tagged @backup_restore_bashrc because they append to ~/.bashrc. before_scenario() returns early for the gpinitsystem feature before it backs ~/.bashrc up, while after_scenario() still restores it, and with no backup to restore it deletes ~/.bashrc. Back it up before that early return. Alena Rybakina found this in review. The mirror port scenario also waits for the gpinitsystem fixes in #2178 and keeps its tag until that is merged.
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