Skip to content

Commit 2da78d8

Browse files
committed
Add validation for CLI input
1 parent 57c7d6c commit 2da78d8

File tree

4 files changed

+5
-63
lines changed

4 files changed

+5
-63
lines changed

src/hyperpod_cli/commands/job.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,7 @@ def start_job(
662662
config["cluster"]["cluster_config"]["volumes"] = volume_mount
663663

664664
if label_selector is not None:
665-
config["cluster"]["cluster_config"]["label_selector"] = label_selector
665+
config["cluster"]["cluster_config"]["label_selector"] = json.loads(label_selector)
666666
elif deep_health_check_passed_nodes_only:
667667
config["cluster"]["cluster_config"]["label_selector"] = (
668668
DEEP_HEALTH_CHECK_PASSED_ONLY_NODE_AFFINITY_DICT
@@ -672,15 +672,14 @@ def start_job(
672672
NODE_AFFINITY_DICT
673673
)
674674

675-
label_selector = config["cluster"].setdefault("label_selector",{})
675+
label_selector = config["cluster"]["cluster_config"].setdefault("label_selector",{})
676676
required_labels = label_selector.setdefault("required", {})
677677

678678
if not required_labels.get(KUBERNETES_INSTANCE_TYPE_LABEL_KEY):
679679
required_labels[KUBERNETES_INSTANCE_TYPE_LABEL_KEY] = (
680680
[str(instance_type)]
681681
)
682682

683-
684683
if auto_resume:
685684
# Set max_retry default to 1
686685
if max_retry is None:

src/hyperpod_cli/validators/job_validator.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,6 @@ def validate_yaml_content(data):
192192
required_labels[KUBERNETES_INSTANCE_TYPE_LABEL_KEY] = (
193193
[str(instance_type)]
194194
)
195-
if instance_type not in required_labels[KUBERNETES_INSTANCE_TYPE_LABEL_KEY]:
196-
logger.error(
197-
f"Please ensure 'instance-type' in 'cluster' matches with 'instance-type' in 'label_selector.required.beta.kubernetes.io/instance-type' in config file"
198-
)
199-
return False
200195

201196
auto_resume = False
202197
max_retry = None

test/unit_tests/test_job.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,6 @@ def capture_yaml_dump(config, *args, **kwargs):
576576
if result.exception:
577577
print(f"Exception: {result.exception}")
578578

579-
580579
@mock.patch('subprocess.run')
581580
@mock.patch("yaml.dump")
582581
@mock.patch("os.path.exists", return_value=True)

test/unit_tests/validators/test_job_validator.py

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,8 +1087,7 @@ def test_validate_yaml_content_valid(self):
10871087
result = validate_yaml_content(mock_data)
10881088
self.assertTrue(result)
10891089

1090-
def test_validate_yaml_content_valid_required_instance_type_label(self):
1091-
# Test auto-populate label selector - "beta.kubernetes.io/instance_type"
1090+
def test_validate_yaml_content_required_instance_type_label(self):
10921091
expected_label_selector = {
10931092
"required": {
10941093
"beta.kubernetes.io/instance-type": [
@@ -1097,6 +1096,7 @@ def test_validate_yaml_content_valid_required_instance_type_label(self):
10971096
}
10981097
}
10991098

1099+
# User does not provide label_selector
11001100
mock_data = {
11011101
"cluster": {
11021102
"cluster_type": "k8s",
@@ -1124,6 +1124,7 @@ def test_validate_yaml_content_valid_required_instance_type_label(self):
11241124
}
11251125
}
11261126

1127+
# User provides label_selector without instance_type
11271128
mock_data = {
11281129
"cluster": {
11291130
"cluster_type": "k8s",
@@ -1146,59 +1147,7 @@ def test_validate_yaml_content_valid_required_instance_type_label(self):
11461147
self.assertEqual(
11471148
mock_data["cluster"]["cluster_config"]["label_selector"], expected_label_selector
11481149
)
1149-
1150-
expected_label_selector = {
1151-
"required": {
1152-
"beta.kubernetes.io/instance-type": [
1153-
"ml.g5.xlarge"
1154-
]
1155-
}
1156-
}
1157-
1158-
mock_data = {
1159-
"cluster": {
1160-
"cluster_type": "k8s",
1161-
"instance_type": "ml.g5.xlarge",
1162-
"cluster_config": {
1163-
"scheduler": "SageMaker",
1164-
"label_selector": {
1165-
"required": {
1166-
"beta.kubernetes.io/instance-type": [
1167-
"ml.g5.xlarge"
1168-
]
1169-
}
1170-
}
1171-
}
1172-
}
1173-
}
11741150

1175-
result = validate_yaml_content(mock_data)
1176-
self.assertTrue(result)
1177-
self.assertEqual(
1178-
mock_data["cluster"]["cluster_config"]["label_selector"], expected_label_selector
1179-
)
1180-
1181-
def test_validate_yaml_content_invalid_required_instance_type_label(self):
1182-
# Test resepect user selection label selector
1183-
mock_data = {
1184-
"cluster": {
1185-
"cluster_type": "k8s",
1186-
"instance_type": "ml.g5.xlarge",
1187-
"cluster_config": {
1188-
"scheduler": "SageMaker",
1189-
"label_selector": {
1190-
"required": {
1191-
"beta.kubernetes.io/instance-type": [
1192-
"ml.g5.2xlarge"
1193-
]
1194-
}
1195-
}
1196-
}
1197-
}
1198-
}
1199-
1200-
result = validate_yaml_content(mock_data)
1201-
self.assertFalse(result)
12021151

12031152
def test_validate_yaml_content_error_no_cluster(
12041153
self,

0 commit comments

Comments
 (0)