test(maas): add MaaS API and controller component health tests#1170
Merged
dbasunag merged 10 commits intoopendatahub-io:mainfrom Mar 5, 2026
Merged
test(maas): add MaaS API and controller component health tests#1170dbasunag merged 10 commits intoopendatahub-io:mainfrom
dbasunag merged 10 commits intoopendatahub-io:mainfrom
Conversation
SB159
requested review from
Raghul-M,
brettmthompson,
mwaykole and
threcc
as code owners
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/build-push-pr-image', '/lgtm', '/wip', '/hold', '/cherry-pick', '/verified'} |
for more information, see https://pre-commit.ci
Contributor
📝 WalkthroughWalkthroughAdds two new pytest modules introducing health checks for MaaS API and MaaS controller: DSC managementState and ModelsAsServiceReady validations, CRD existence checks, deployment availability waits, pod readiness checks, and a parameterized subscription stack readiness test. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py (1)
18-28: Duplicate test: identical totest_maas_condition_in_dscintest_maas_api_health.py.Both test classes verify the same
ModelsAsServiceReadyDSC condition with identical logic. Consider consolidating into a single test or creating a shared test base/mixin to avoid duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py` around lines 18 - 28, The test function test_maas_condition_in_dsc in this file duplicates an identical test in tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_api_health.py; either remove this duplicate test or extract the assertion into a shared helper/mixin (e.g., a helper named assert_models_as_service_ready_in_dsc or a TestMixin class) and call that from both test modules, ensuring the unique check for ModelsAsServiceReady on dsc_resource.instance.status.conditions remains implemented once and reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py`:
- Around line 18-28: The test function test_maas_condition_in_dsc in this file
duplicates an identical test in
tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_api_health.py;
either remove this duplicate test or extract the assertion into a shared
helper/mixin (e.g., a helper named assert_models_as_service_ready_in_dsc or a
TestMixin class) and call that from both test modules, ensuring the unique check
for ModelsAsServiceReady on dsc_resource.instance.status.conditions remains
implemented once and reused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c770a8c7-d884-4e15-9851-c17223a5bf7a
📒 Files selected for processing (2)
tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_api_health.pytests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py
dbasunag
reviewed
Mar 4, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py`:
- Line 78: The test currently ignores wait_for_pods_running(...) return value
and checks all namespace pods; change the call in test_maas_controller_health.py
to assert the helper returns True (or raises on failure) and restrict the check
to only maas-controller pods by passing a label selector or using the
maas-controller deployment selector (e.g., filter by label "app=maas-controller"
or the deployment's selector) into wait_for_pods_running (or call a variant that
accepts label_selector) so the test only verifies maas-controller pod readiness
and fails when those pods are unhealthy.
- Line 95: The test currently calls request.getfixturevalue using an invalid
keyword "fixture_name"; change the call to use the correct parameter name
"argname" (i.e., replace request.getfixturevalue(fixture_name=resource_fixture)
with request.getfixturevalue(argname=resource_fixture)) so the pytest
FixtureRequest API is used correctly in test_maas_controller_health.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba4c1f09-92dc-4b2d-a3cf-3f2d284a7184
📒 Files selected for processing (2)
tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_api_health.pytests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_api_health.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py (1)
77-78:⚠️ Potential issue | 🟠 MajorPod health check is still too broad and not asserted.
On Line 78, the call result is not asserted, and the helper evaluates all namespace pods rather than only
maas-controllerpods. This can mask controller issues or fail due to unrelated workloads.Suggested fix
- wait_for_pods_running(admin_client=admin_client, namespace_name=applications_namespace) + assert wait_for_pods_running( + admin_client=admin_client, + namespace_name=applications_namespace, + ), f"Pods did not reach Running/Completed in namespace {applications_namespace}"Please also scope this check to
maas-controllerpods (e.g., selector-based helper) so the test matches its name and intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py` around lines 77 - 78, The test currently logs and calls wait_for_pods_running(admin_client=admin_client, namespace_name=applications_namespace) without asserting its result and it checks all pods in the namespace; change it to target only maas-controller pods using a selector (e.g., pass a label_selector or use a selector-aware helper) and assert the helper returns success: replace the generic call to wait_for_pods_running with the selector-scoped variant (or add label_selector="app=maas-controller") using admin_client and applications_namespace, then add an assertion that the call indicates pods are running (e.g., assert truthy/True) so failures are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py`:
- Around line 77-78: The test currently logs and calls
wait_for_pods_running(admin_client=admin_client,
namespace_name=applications_namespace) without asserting its result and it
checks all pods in the namespace; change it to target only maas-controller pods
using a selector (e.g., pass a label_selector or use a selector-aware helper)
and assert the helper returns success: replace the generic call to
wait_for_pods_running with the selector-scoped variant (or add
label_selector="app=maas-controller") using admin_client and
applications_namespace, then add an assertion that the call indicates pods are
running (e.g., assert truthy/True) so failures are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92c07c1b-4cea-4be8-8479-0f91e617e81a
📒 Files selected for processing (1)
tests/model_serving/model_server/maas_billing/maas_subscription/component_health/test_maas_controller_health.py
dbasunag
approved these changes
Mar 5, 2026
mwaykole
approved these changes
Mar 5, 2026
|
Status of building tag latest: success. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Adds component health tests for MaaS components.
These tests validate that MaaS components are deployed and healthy before running MaaS tests.
The following checks are added:
modelsAsServicemanagementState isMANAGEDin the DataScienceClusterModelsAsServiceReadycondition isTruemaas-controllerdeployment is Availablemaas-controllerpods are Runningmaas-apideployment is Availablemaas-apipods are RunningMaaSModel,MaaSAuthPolicy,MaaSSubscription) are ReadyRelated Issues
How it has been tested
Additional Requirements
Summary by CodeRabbit