make dynamic lora sidecar health check parameters configurable and force reconcile #605

kaushikmitr · 2025-03-28T18:52:32Z

This pull request includes several enhancements and fixes to the dynamic-lora-sidecar tool, focusing on improving the Dockerfile, updating the README documentation, adding command-line argument support, and updating tests accordingly.

Dockerfile and Documentation Updates:

tools/dynamic-lora-sidecar/Dockerfile: Corrected the COPY command to ensure proper file copying.
tools/dynamic-lora-sidecar/README.md: Added sections for command-line arguments and an example deployment configuration.

Command-Line Argument Support:

tools/dynamic-lora-sidecar/sidecar/sidecar.py: Introduced argparse for handling command-line arguments, updated the LoraReconciler class to use these arguments, and modified related methods to utilize the new configuration parameters.

Test Updates:

tools/dynamic-lora-sidecar/sidecar/test_sidecar.py: Updated tests to include the new configuration parameters and validated the proper initialization of health check settings from command-line arguments.

Tested With:

      initContainers:
        - name: lora-adapter-syncer
          tty: true
          stdin: true 
          image: us-docker.pkg.dev/kaushikmitra-gke-dev/kaushikmitra-docker-repo/ora_sidecar_test
          command: ["python", "sidecar.py", "--health-check-timeout", "600", "--health-check-interval", "5", "--reconcile-trigger", "10"]
          restartPolicy: Always
          imagePullPolicy: Always
          env: 
            - name: DYNAMIC_LORA_ROLLOUT_CONFIG
              value: "/config/configmap.yaml"
          volumeMounts: # DO NOT USE subPath
          - name: config-volume
            mountPath:  /config

…rrce reconcile

k8s-ci-robot · 2025-03-28T18:52:42Z

Hi @kaushikmitr. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

netlify · 2025-03-28T18:52:47Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`ea87c17`
🔍 Latest deploy log	https://app.netlify.com/sites/gateway-api-inference-extension/deploys/67e70eefb75d34000893e091
😎 Deploy Preview	https://deploy-preview-605--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

ahg-g · 2025-03-28T19:02:32Z

/ok-to-test

ahg-g

Thanks!

ahg-g · 2025-03-28T19:04:52Z

tools/dynamic-lora-sidecar/sidecar/sidecar.py

+        timeout_seconds = config.get("healthCheckTimeoutSeconds", 300)
+        self.health_check_timeout = datetime.timedelta(seconds=timeout_seconds)
+
+        # Get health check interval from config with default of 15 seconds


default of 2 seconds

yes made this 2 and refresh 5 and timeout 300

ahg-g · 2025-03-28T19:06:05Z

tools/dynamic-lora-sidecar/sidecar/sidecar.py

@@ -217,6 +233,9 @@ def reconcile(self):
        logging.info(
            f"reconciling model server {self.model_server} with config stored at {CONFIG_MAP_FILE}"
        )
+        # Update health check settings from config before reconciliation
+        self._update_health_check_settings()


why do we need to call it here again?

ahg-g · 2025-03-28T19:09:22Z

tools/dynamic-lora-sidecar/sidecar/sidecar.py

+        interval_seconds = config.get("healthCheckIntervalSeconds", 2)
+        self.health_check_interval = datetime.timedelta(seconds=interval_seconds)
+
+        # Get reconciliation trigger interval from config with default of 5 seconds


default is 1

ahg-g · 2025-03-28T19:35:31Z

tools/dynamic-lora-sidecar/sidecar/sidecar.py

+        """Update health check settings from config"""
+        config = self.config
+        # Get health check timeout from config with default of 300 seconds
+        timeout_seconds = config.get("healthCheckTimeoutSeconds", 300)


Those are better as command line flags or env vars to the container, similar to https://github.com/kubernetes-sigs/gateway-api-inference-extension/pull/605/files#diff-e5ae211d8cba1ab987311c2506ed80add46fe1f9d8f8c4ad08084439bb135364R15

yes made them cmd line flags with defaults

kaushikmitr

made the new params cmd line arugments with defaults

ahg-g · 2025-03-28T21:34:16Z

/lgtm
/approve

k8s-ci-robot · 2025-03-28T21:34:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, kaushikmitr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ahg-g]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kaushikmitr added 6 commits March 21, 2025 18:55

update benchmarking guide with latest results with vllm v1

Loading
Loading status checks…

0e714fe

Merge branch 'kubernetes-sigs:main' into main

Loading
Loading status checks…

22cbdaf

update graph

Loading
Loading status checks…

f252457

Merge branch 'kubernetes-sigs:main' into main

560d0be

make dynamic lora sidecar health check parameters configurable and fo…

f2a6f44

…rrce reconcile

update screenshots

Loading
Loading status checks…

bc9ddb6

k8s-ci-robot added cncf-cla: yes needs-ok-to-test labels Mar 28, 2025

k8s-ci-robot requested review from danehans and Jeffwan March 28, 2025 18:52

k8s-ci-robot added the size/M label Mar 28, 2025

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Mar 28, 2025

ahg-g reviewed Mar 28, 2025

View reviewed changes

k8s-ci-robot added size/L and removed size/M labels Mar 28, 2025

kaushikmitr force-pushed the main branch from b479651 to e31169b Compare March 28, 2025 20:51

kaushikmitr commented Mar 28, 2025

View reviewed changes

make the health and refresh params in sidecar cmd line argument

Loading
Loading status checks…

ea87c17

kaushikmitr force-pushed the main branch from e31169b to ea87c17 Compare March 28, 2025 21:04

k8s-ci-robot assigned ahg-g Mar 28, 2025

k8s-ci-robot added the lgtm label Mar 28, 2025

k8s-ci-robot added the approved label Mar 28, 2025

k8s-ci-robot merged commit 5df6968 into kubernetes-sigs:main Mar 28, 2025
7 of 8 checks passed

ahg-g mentioned this pull request Mar 28, 2025

lora-syncer should block on startup until the server is ready #597

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make dynamic lora sidecar health check parameters configurable and force reconcile #605

make dynamic lora sidecar health check parameters configurable and force reconcile #605

kaushikmitr commented Mar 28, 2025 •

edited

Loading

k8s-ci-robot commented Mar 28, 2025

netlify bot commented Mar 28, 2025 •

edited

Loading

ahg-g commented Mar 28, 2025

ahg-g left a comment

ahg-g Mar 28, 2025

kaushikmitr Mar 28, 2025

ahg-g Mar 28, 2025 •

edited

Loading

ahg-g Mar 28, 2025

ahg-g Mar 28, 2025

kaushikmitr Mar 28, 2025

kaushikmitr left a comment

ahg-g commented Mar 28, 2025

k8s-ci-robot commented Mar 28, 2025

make dynamic lora sidecar health check parameters configurable and force reconcile #605

make dynamic lora sidecar health check parameters configurable and force reconcile #605

Conversation

kaushikmitr commented Mar 28, 2025 • edited Loading

k8s-ci-robot commented Mar 28, 2025

netlify bot commented Mar 28, 2025 • edited Loading

✅ Deploy Preview for gateway-api-inference-extension ready!

ahg-g commented Mar 28, 2025

ahg-g left a comment

Choose a reason for hiding this comment

ahg-g Mar 28, 2025

Choose a reason for hiding this comment

kaushikmitr Mar 28, 2025

Choose a reason for hiding this comment

ahg-g Mar 28, 2025 • edited Loading

Choose a reason for hiding this comment

ahg-g Mar 28, 2025

Choose a reason for hiding this comment

ahg-g Mar 28, 2025

Choose a reason for hiding this comment

kaushikmitr Mar 28, 2025

Choose a reason for hiding this comment

kaushikmitr left a comment

Choose a reason for hiding this comment

ahg-g commented Mar 28, 2025

k8s-ci-robot commented Mar 28, 2025

kaushikmitr commented Mar 28, 2025 •

edited

Loading

netlify bot commented Mar 28, 2025 •

edited

Loading

ahg-g Mar 28, 2025 •

edited

Loading