Skip to content

Xdb 400 7.3 systemd#175

Merged
MarkSh1 merged 4 commits into
owtech:ow-fork-7.3from
MarkSh1:XDB-400-7.3-systemd
Oct 8, 2025
Merged

Xdb 400 7.3 systemd#175
MarkSh1 merged 4 commits into
owtech:ow-fork-7.3from
MarkSh1:XDB-400-7.3-systemd

Conversation

@MarkSh1
Copy link
Copy Markdown

@MarkSh1 MarkSh1 commented Oct 8, 2025

  1. Fixed a backup_agent path error in the multi-version server package.
  2. Added an FDB read/write test to test-deploy.
  3. Refactored the test-deploy script.
  4. Used a single "foundationdb.service" for both RPM and DEB packages, eliminating init.d calls.

@MarkSh1 MarkSh1 requested review from Copilot and oleg68 October 8, 2025 08:27
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors FoundationDB's packaging system to use systemd consistently across both RPM and DEB packages, eliminating the need for init.d scripts. It also fixes a backup_agent path error and enhances the test-deploy script with better error handling and FDB read/write testing.

  • Unified systemd service file usage across all package types (RPM, DEB, and multi-version)
  • Fixed backup_agent path from /usr/lib/foundationdb/backup_agent/backup_agent to /usr/bin/backup_agent
  • Enhanced test-deploy script with systemd container support, structured error handling, and FDB read/write validation

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
packaging/rpm/scripts/postserver.sh Added conditional restart logic and systemd service management
packaging/rpm/buildrpms.sh Changed to use unified foundationdb.service file
packaging/multiversion/server/postinst-deb Removed DEB-specific postinstall script
packaging/multiversion/server/postinst Simplified systemctl commands by removing /usr/bin/ prefix
packaging/multiversion/clients/postinst Fixed backup_agent path from lib directory to bin directory
packaging/deb/DEBIAN-foundationdb-server/postinst Removed init.d fallback logic, keeping only systemd
packaging/deb/DEBIAN-foundationdb-server/conffiles Removed init.d script from configuration files
cmake/InstallLayout.cmake Unified to single postinst script and foundationdb.service file
build-scripts/for-linux/test-deploy.bash Complete refactor with systemd support and FDB read/write testing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

DORESTART=1

if test -f /etc/foundationdb/owtech.conf; then
value=`grep -E "^RestartWhenUpdate" /etc/foundationdb/owtech.conf | awk -F "=" '{ print $2 }' | tr -d " " | tr "a-z" "A-Z"`
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This complex pipeline could be simplified and made more readable by using a single awk command or breaking it into multiple lines with intermediate variables.

Suggested change
value=`grep -E "^RestartWhenUpdate" /etc/foundationdb/owtech.conf | awk -F "=" '{ print $2 }' | tr -d " " | tr "a-z" "A-Z"`
value=$(awk -F= '/^RestartWhenUpdate/ { gsub(/ /, "", $2); print toupper($2) }' /etc/foundationdb/owtech.conf)

Copilot uses AI. Check for mistakes.
systemctl start foundationdb.service
else
/etc/init.d/foundationdb start
update-rc.d foundationdb defaults
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The update-rc.d command is inside an else block that corresponds to non-systemd systems, but the surrounding systemd-only logic suggests init.d support may no longer be intended. This could cause confusion about the intended behavior.

Suggested change
update-rc.d foundationdb defaults

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source code contains foundationdb-init files to support older systems.

local TEST_CLIENT="getent passwd foundationdb"
TEST_CLIENT_WITH="$TEST_CLIENT_WITH $TEST_CLIENT"

local TEST_WRITE_READ='fdbcli --exec "writemode on; set key testvalue; get key" | grep -E "key.*is.*testvalue"'
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The grep pattern key.*is.*testvalue may be too broad and could match unintended output. Consider using a more specific pattern like ^.*key.*is.*testvalue$ to ensure exact matching.

Suggested change
local TEST_WRITE_READ='fdbcli --exec "writemode on; set key testvalue; get key" | grep -E "key.*is.*testvalue"'
local TEST_WRITE_READ='fdbcli --exec "writemode on; set key testvalue; get key" | grep -E "^.*key.*is.*testvalue$"'

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

@MarkSh1 MarkSh1 Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fdbcli --exec "writemode on; set key testvalue; get key
returns

`key' is `testvalue'

@MarkSh1 MarkSh1 force-pushed the XDB-400-7.3-systemd branch from 9c34692 to 4f30551 Compare October 8, 2025 10:08
@owtech owtech deleted a comment from Copilot AI Oct 8, 2025
Copy link
Copy Markdown

@oleg68 oleg68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the same PR for main?

@MarkSh1 MarkSh1 merged commit b735f82 into owtech:ow-fork-7.3 Oct 8, 2025
MarkSh1 added a commit to MarkSh1/foundationdb that referenced this pull request Oct 8, 2025
* XDB-400 using one systemd service file for RPM and DEB
* XDB-400-7.3 removed code duplication, added systemd support for Debian container
* XDB-400 refactoring, added systemd for ol8
* XDB-400-7.3 fixed rpm post, multiversion server,client post, added WR test in test-deploy
MarkSh1 added a commit to MarkSh1/foundationdb that referenced this pull request Oct 8, 2025
* XDB-400 using one systemd service file for RPM and DEB
* XDB-400-7.3 removed code duplication, added systemd support for Debian container
* XDB-400 refactoring, added systemd for ol8
* XDB-400-7.3 fixed rpm post, multiversion server,client post, added WR test in test-deploy
MarkSh1 added a commit to MarkSh1/foundationdb that referenced this pull request Oct 10, 2025
* XDB-400 using one systemd service file for RPM and DEB
* XDB-400-7.3 removed code duplication, added systemd support for Debian container
* XDB-400 refactoring, added systemd for ol8
* XDB-400-7.3 fixed rpm post, multiversion server,client post, added WR test in test-deploy
MarkSh1 added a commit that referenced this pull request Oct 13, 2025
* Create build container with zlib for main branch (#173)
* XDB-394 fix. Restore original osx packaging files
* added zlib and set archive repos for no longer supported Debian 10
* renamed variable for readability and added sudo to install
* fix "Unknown error" when  configuring regions
* XDB-328 cherry-pick from 7.3
* XDB-394 fix path to fdbmonitor and backup_agent (#167)
* XDB-436 added fdbconvert to package install targets (#174)
* Xdb 400 7.3 systemd (#175)
* XDB-400 using one systemd service file for RPM and DEB
* XDB-400-7.3 removed code duplication, added systemd support for Debian container
* XDB-400 refactoring, added systemd for ol8
* XDB-400-7.3 fixed rpm post, multiversion server,client post, added WR test in test-deploy
* XDB-394 fix. Restore original osx packaging files (#170)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants