Skip to content

K8s : add settings to the PM config - #602

Merged
MRiganSUSX merged 10 commits into
developfrom
wanyunSu/k8s-PMconf
Oct 10, 2025
Merged

K8s : add settings to the PM config#602
MRiganSUSX merged 10 commits into
developfrom
wanyunSu/k8s-PMconf

Conversation

@wanyunSu

@wanyunSu wanyunSu commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

Description

Make use of process manager configuration for k8s settings

Type of change

  • Documentation (non-breaking change that adds or improves the documentation)
  • New feature (non-breaking change which adds functionality)
  • Optimization (non-breaking, back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (whatever its nature)

Key checklist

  • All tests pass (eg. python -m pytest)
  • Pre-commit hooks run successfully (eg. pre-commit run --all-files)

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added or an issue has been opened to tackle that in the future.
    (Indicate issue here: # (issue))

@wanyunSu wanyunSu mentioned this pull request Oct 2, 2025
8 tasks
@wanyunSu wanyunSu self-assigned this Oct 2, 2025
@wanyunSu
wanyunSu marked this pull request as ready for review October 9, 2025 09:38
@wanyunSu
wanyunSu requested a review from MRiganSUSX October 9, 2025 14:35
@MRiganSUSX
MRiganSUSX requested a review from Copilot October 9, 2025 14:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 the K8s process manager configuration to make use of a structured settings configuration instead of hardcoded values and flat configuration access. The changes introduce a hierarchical settings structure that organizes configuration parameters into logical groups (labels, connection_server, pod_management, cleanup, checking) and makes several timeout and retry values configurable.

Key changes:

  • Restructured configuration access to use nested settings groups instead of flat key-value pairs
  • Made hardcoded sleep values configurable through the settings
  • Updated connection URL logging to use pod IP instead of hostname

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/drunc/process_manager/k8s_process_manager.py Refactored configuration loading to use structured settings and made sleep/timeout values configurable
src/drunc/data/process_manager/k8s.json Added structured settings configuration with organized parameter groups

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

Comment thread src/drunc/process_manager/k8s_process_manager.py Outdated
Comment thread src/drunc/data/process_manager/k8s.json Outdated
Comment thread src/drunc/data/process_manager/k8s.json Outdated
wanyunSu and others added 3 commits October 9, 2025 17:19
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@MRiganSUSX

Copy link
Copy Markdown
Contributor

Hi @wanyunSu,
I have two comments:

  • one is as mentioned by copilot above: if we know that a configuration variable is an integer (/float/whatever), we should set it as that in the json, and the load it as it in the pm. So, for example:
"restart_time": "10",
...
self.restart_cleanup_time = float(cleanup.get("restart_time", "10"))

should instead be

"restart_time": 10,
...
self.restart_cleanup_time = cleanup.get("restart_time", 10)
  • the second point is naming consistency. We should normalize the naming between the json and python, if possible. Ie:
self._host_cache_expiry = checking.get("host_verification_cache_expiry", 300)
could be simply
self._host_cache_expiry = checking.get("host_cache_expiry", 300)

@MRiganSUSX MRiganSUSX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM !

@MRiganSUSX
MRiganSUSX merged commit b8bde82 into develop Oct 10, 2025
3 checks passed
@MRiganSUSX
MRiganSUSX deleted the wanyunSu/k8s-PMconf branch October 10, 2025 12:57
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.

5 participants