Skip to content

Commit

Permalink
[spm]: Clean up timers auto generation logic. (#3523)
Browse files Browse the repository at this point in the history
Config Reload Enhancements PR sonic-net/SONiC#1203 does not completely remove TIMERs from SONiC Package Manager infra. This PR is intended to complete the original changes.

`Systemd` TIMERs infra was replaced by `hostcfgd` service management.
That was done to improve reliability of service management.

#### What I did
* Removed redundant TIMERs infra

#### How I did it
* Updated SPM auto generation logic

#### How to verify it
1. Install application extension
```bash
spm install --from-tarball <app_ext_path>
```
2. Make sure `delayed` flag is set
```bash
docker image inspect <app_ext_image> | jq '.[].Config.Labels["com.azure.sonic.manifest"]' | python -c 'import sys,ast; print(ast.literal_eval(sys.stdin.read()))' | jq .service.delayed
true
```
3. Check no TIMERs were generated
  • Loading branch information
nazariig authored Sep 15, 2024
1 parent 2cb8cc6 commit 5fc0ee6
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 76 deletions.
19 changes: 0 additions & 19 deletions sonic-utilities-data/templates/timer.unit.j2

This file was deleted.

20 changes: 1 addition & 19 deletions sonic_package_manager/service_creator/creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@


SERVICE_FILE_TEMPLATE = 'sonic.service.j2'
TIMER_UNIT_TEMPLATE = 'timer.unit.j2'

SYSTEMD_LOCATION = '/usr/lib/systemd/system'
ETC_SYSTEMD_LOCATION = '/etc/systemd/system'
Expand Down Expand Up @@ -305,7 +304,7 @@ def generate_service_mgmt(self, package: Package):
log.info(f'generated {script_path}')

def generate_systemd_service(self, package: Package):
""" Generates systemd service(s) file and timer(s) (if needed) for package.
""" Generates systemd service(s) file for package.
Args:
package: Package object to generate service for.
Expand Down Expand Up @@ -333,23 +332,6 @@ def generate_systemd_service(self, package: Package):
render_template(template, output_file, template_vars)
log.info(f'generated {output_file}')

if package.manifest['service']['delayed']:
template_vars = {
'source': get_tmpl_path(TIMER_UNIT_TEMPLATE),
'manifest': package.manifest.unmarshal(),
'multi_instance': False,
}
output_file = os.path.join(SYSTEMD_LOCATION, f'{name}.timer')
template = os.path.join(TEMPLATES_PATH, TIMER_UNIT_TEMPLATE)
render_template(template, output_file, template_vars)
log.info(f'generated {output_file}')

if package.manifest['service']['asic-service']:
output_file = os.path.join(SYSTEMD_LOCATION, f'{name}@.timer')
template_vars['multi_instance'] = True
render_template(template, output_file, template_vars)
log.info(f'generated {output_file}')

def update_generated_services_conf_file(self, package: Package, remove=False):
""" Updates generated_services.conf file.
Expand Down
3 changes: 1 addition & 2 deletions sonic_package_manager/service_creator/feature.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,7 @@ def update(self,
old_manifest: Manifest,
new_manifest: Manifest):
""" Migrate feature configuration. It can be that non-configurable
feature entries have to be updated. e.g: "delayed" for example if
the new feature introduces a service timer or name of the service has
feature entries have to be updated. e.g: name of the service has
changed, but user configurable entries are not changed).
Args:
Expand Down
1 change: 0 additions & 1 deletion tests/sonic_package_manager/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,6 @@ def sonic_fs(fs):
fs.create_dir(SERVICE_MGMT_SCRIPT_LOCATION)
fs.create_file(GENERATED_SERVICES_CONF_FILE)
fs.create_file(os.path.join(TEMPLATES_PATH, SERVICE_FILE_TEMPLATE))
fs.create_file(os.path.join(TEMPLATES_PATH, TIMER_UNIT_TEMPLATE))
fs.create_file(os.path.join(TEMPLATES_PATH, SERVICE_MGMT_SCRIPT_TEMPLATE))
fs.create_file(os.path.join(TEMPLATES_PATH, DOCKER_CTL_SCRIPT_TEMPLATE))
fs.create_file(os.path.join(TEMPLATES_PATH, DEBUG_DUMP_SCRIPT_TEMPLATE))
Expand Down
35 changes: 0 additions & 35 deletions tests/sonic_package_manager/test_service_creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,20 +137,6 @@ def read_file(name):
assert not sonic_fs.exists(os.path.join(ETC_SYSTEMD_LOCATION, 'test@2.service.d'))


def test_service_creator_with_timer_unit(sonic_fs, manifest, service_creator):
entry = PackageEntry('test', 'azure/sonic-test')
package = Package(entry, Metadata(manifest))
service_creator.create(package)

assert not sonic_fs.exists(os.path.join(SYSTEMD_LOCATION, 'test.timer'))

manifest['service']['delayed'] = True
package = Package(entry, Metadata(manifest))
service_creator.create(package)

assert sonic_fs.exists(os.path.join(SYSTEMD_LOCATION, 'test.timer'))


def test_service_creator_with_debug_dump(sonic_fs, manifest, service_creator):
entry = PackageEntry('test', 'azure/sonic-test')
package = Package(entry, Metadata(manifest))
Expand Down Expand Up @@ -396,27 +382,6 @@ def test_feature_update(mock_sonic_db, manifest):
], any_order=True)


def test_feature_registration_with_timer(mock_sonic_db, manifest):
manifest['service']['delayed'] = True
mock_connector = Mock()
mock_connector.get_entry = Mock(return_value={})
mock_sonic_db.get_connectors = Mock(return_value=[mock_connector])
mock_sonic_db.get_initial_db_connector = Mock(return_value=mock_connector)
feature_registry = FeatureRegistry(mock_sonic_db)
feature_registry.register(manifest)
mock_connector.set_entry.assert_called_with('FEATURE', 'test', {
'state': 'disabled',
'auto_restart': 'enabled',
'high_mem_alert': 'disabled',
'set_owner': 'local',
'has_per_asic_scope': 'False',
'has_global_scope': 'True',
'delayed': 'True',
'check_up_status': 'False',
'support_syslog_rate_limit': 'False',
})


def test_feature_registration_with_non_default_owner(mock_sonic_db, manifest):
mock_connector = Mock()
mock_connector.get_entry = Mock(return_value={})
Expand Down

0 comments on commit 5fc0ee6

Please sign in to comment.