Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions sonic_package_manager/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,10 @@ def install_from_source(self,
self.database.update_package(package.entry)
self.database.commit()

# Ensure all files (plugins, services, database) are synced to disk
# This prevents data loss or corruption after power cycle
os.sync()

@under_lock
def update(self,
name: str,
Expand Down Expand Up @@ -546,6 +550,10 @@ def uninstall(self, name: str,
package.entry.version = None
self.database.update_package(package.entry)
self.database.commit()

# Ensure database update is synced to disk
os.sync()

manifest_path = os.path.join(MANIFESTS_LOCATION, name)
edit_path = os.path.join(MANIFESTS_LOCATION, name + ".edit")
if os.path.exists(manifest_path):
Expand Down Expand Up @@ -692,6 +700,10 @@ def upgrade_from_source(self,
new_package_entry.version = new_version
self.database.update_package(new_package_entry)
self.database.commit()

# Ensure all files are synced to disk after upgrade
os.sync()

if update_only:
manifest_path = os.path.join(MANIFESTS_LOCATION, name)
edit_path = os.path.join(MANIFESTS_LOCATION, name + ".edit")
Expand Down
63 changes: 54 additions & 9 deletions tests/sonic_package_manager/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ def mock_run_command():


def test_installation_not_installed(package_manager):
package_manager.install('test-package')
package = package_manager.get_installed_package('test-package')
assert package.installed
assert package.entry.default_reference == '1.6.0'
with patch('sonic_package_manager.manager.os.sync') as mock_sync:
package_manager.install('test-package')
package = package_manager.get_installed_package('test-package')
assert package.installed
assert package.entry.default_reference == '1.6.0'
# Verify os.sync() is called after installation to ensure data persistence
mock_sync.assert_called()


def test_installation_already_installed(package_manager):
Expand Down Expand Up @@ -179,7 +182,8 @@ def test_installation_multiple_cli_plugin(package_manager, fake_metadata_resolve
manifest['cli']= {'show': ['/cli/plugin.py', '/cli/plugin2.py']}
with patch('sonic_package_manager.manager.get_cli_plugin_directory') as get_dir_mock, \
patch('os.remove') as remove_mock, \
patch('os.path.exists') as path_exists_mock:
patch('os.path.exists') as path_exists_mock, \
patch('sonic_package_manager.manager.os.sync') as mock_sync:
get_dir_mock.return_value = '/'
package_manager.install('test-package')
package_manager.docker.extract.assert_has_calls(
Expand All @@ -189,6 +193,9 @@ def test_installation_multiple_cli_plugin(package_manager, fake_metadata_resolve
],
any_order=True,
)
# Verify os.sync() is called after installation
install_sync_count = mock_sync.call_count
assert install_sync_count >= 1, "os.sync() should be called after installation"

package_manager._set_feature_state = Mock()
package_manager.uninstall('test-package', force=True)
Expand All @@ -199,6 +206,9 @@ def test_installation_multiple_cli_plugin(package_manager, fake_metadata_resolve
],
any_order=True,
)
# Verify os.sync() is called after uninstallation
uninstall_sync_count = mock_sync.call_count
assert uninstall_sync_count > install_sync_count, "os.sync() should be called after uninstallation"


def test_installation_cli_plugin_skipped(package_manager, fake_metadata_resolver, anything):
Expand Down Expand Up @@ -328,10 +338,13 @@ def test_manager_upgrade(package_manager, sonic_fs, mock_run_command):
package_manager.install('test-package-6=1.5.0')
package = package_manager.get_installed_package('test-package-6')

package_manager.install('test-package-6=2.0.0')
upgraded_package = package_manager.get_installed_package('test-package-6')
assert upgraded_package.entry.version == Version.parse('2.0.0')
assert upgraded_package.entry.default_reference == package.entry.default_reference
with patch('sonic_package_manager.manager.os.sync') as mock_sync:
package_manager.install('test-package-6=2.0.0')
upgraded_package = package_manager.get_installed_package('test-package-6')
assert upgraded_package.entry.version == Version.parse('2.0.0')
assert upgraded_package.entry.default_reference == package.entry.default_reference
# Verify os.sync() is called after upgrade to ensure data persistence
mock_sync.assert_called()

mock_run_command.assert_has_calls(
[
Expand Down Expand Up @@ -627,3 +640,35 @@ def attrs(self):
# Get the package from the database and verify the tag was set to the image ID
package = package_manager.database.get_package('test-package')
assert package.docker_image_reference == 'Azure/docker-test:1.6.0'

def test_sync_called_after_database_commit(package_manager):
"""Test that os.sync() is called after database.commit() to ensure transactional consistency"""

call_order = []

# Mock both database.commit() and os.sync() to track call order
original_commit = package_manager.database.commit

def mock_commit():
call_order.append('commit')
original_commit()

def mock_sync():
call_order.append('sync')

with patch.object(package_manager.database, 'commit', side_effect=mock_commit), \
patch('sonic_package_manager.manager.os.sync', side_effect=mock_sync):

package_manager.install('test-package')

# Verify that both commit and sync were called
assert 'commit' in call_order, "database.commit() should be called"
assert 'sync' in call_order, "os.sync() should be called"

# Find the last commit and verify sync comes after it
last_commit_idx = len(call_order) - 1 - call_order[::-1].index('commit')
last_sync_idx = len(call_order) - 1 - call_order[::-1].index('sync')

# sync should be called after the final commit
assert last_sync_idx > last_commit_idx, \
f"os.sync() should be called after database.commit(), but call order was: {call_order}"
Loading