From 539df5e8bb738ae3be7fd7c0aebe8ffcf727f1b5 Mon Sep 17 00:00:00 2001 From: Amanda Vialva <144278621+amandavialva01@users.noreply.github.com> Date: Tue, 22 Oct 2024 12:23:06 -0400 Subject: [PATCH] chore: update CLI commands to work with global APIs (#10089) --- .../tests/cluster/test_config_policies.py | 199 +++++++++++++----- .../config_policies/valid_experiment.json | 12 ++ .../{valid.yaml => valid_experiment.yaml} | 2 +- .../fixtures/config_policies/valid_ntsc.json | 8 + .../fixtures/config_policies/valid_ntsc.yaml | 4 + harness/determined/cli/config_policies.py | 113 ++++++---- 6 files changed, 245 insertions(+), 93 deletions(-) create mode 100644 e2e_tests/tests/fixtures/config_policies/valid_experiment.json rename e2e_tests/tests/fixtures/config_policies/{valid.yaml => valid_experiment.yaml} (76%) create mode 100644 e2e_tests/tests/fixtures/config_policies/valid_ntsc.json create mode 100644 e2e_tests/tests/fixtures/config_policies/valid_ntsc.yaml diff --git a/e2e_tests/tests/cluster/test_config_policies.py b/e2e_tests/tests/cluster/test_config_policies.py index 0adfa68fc2d..e7f083a9201 100644 --- a/e2e_tests/tests/cluster/test_config_policies.py +++ b/e2e_tests/tests/cluster/test_config_policies.py @@ -7,16 +7,21 @@ from tests import config as conf from tests import detproc +VALID_EXPERIMENT_YAML = "config_policies/valid_experiment.yaml" +VALID_EXPERIMENT_JSON = "config_policies/valid_experiment.json" +VALID_NTSC_YAML = "config_policies/valid_ntsc.yaml" +VALID_NTSC_JSON = "config_policies/valid_ntsc.json" + @pytest.mark.e2e_cpu def test_set_config_policies() -> None: - sess = api_utils.user_session() + sess = api_utils.admin_session() workspace_name = api_utils.get_random_string() create_workspace_cmd = ["det", "workspace", "create", workspace_name] detproc.check_call(sess, create_workspace_cmd) - # valid path with experiment type + # workspace valid path with experiment type YAML stdout = detproc.check_output( sess, [ @@ -24,55 +29,85 @@ def test_set_config_policies() -> None: "config-policies", "set", "experiment", - "--workspace", + conf.fixtures_path(VALID_EXPERIMENT_YAML), + "--workspace-name", workspace_name, - "--config-file", - conf.fixtures_path("config_policies/valid.yaml"), ], ) - with open(conf.fixtures_path("config_policies/valid.yaml"), "r") as f: - data = f.read() + data = f"Set experiment config policies for workspace {workspace_name}:\n" + with open(conf.fixtures_path(VALID_EXPERIMENT_YAML), "r") as f: + data += f.read() print(stdout) print(data) assert data.rstrip() == stdout.rstrip() - # valid path with experiment type + # workspace valid path with ntsc type YAML stdout = detproc.check_output( sess, [ "det", "config-policies", "set", - "experiment", - "--workspace", + "tasks", + conf.fixtures_path(VALID_NTSC_YAML), + "--workspace-name", workspace_name, - "--config-file", - conf.fixtures_path("config_policies/valid.yaml"), ], ) + data = f"Set tasks config policies for workspace {workspace_name}:\n" + with open(conf.fixtures_path(VALID_NTSC_YAML), "r") as f: + data += f.read() + assert data.rstrip() == stdout.rstrip() - with open(conf.fixtures_path("config_policies/valid.yaml"), "r") as f: - data = f.read() + # global valid path with experiment type JSON + stdout = detproc.check_output( + sess, + [ + "det", + "config-policies", + "set", + "experiment", + conf.fixtures_path(VALID_EXPERIMENT_JSON), + ], + ) + + data = "Set global experiment config policies:\n" + with open(conf.fixtures_path(VALID_EXPERIMENT_YAML), "r") as f: + data += f.read() + print(stdout) + print(data) assert data.rstrip() == stdout.rstrip() - # invalid path + # global valid path with ntsc type JSON + stdout = detproc.check_output( + sess, + [ + "det", + "config-policies", + "set", + "tasks", + conf.fixtures_path(VALID_NTSC_JSON), + ], + ) + data = "Set global tasks config policies:\n" + with open(conf.fixtures_path(VALID_NTSC_YAML), "r") as f: + data += f.read() + assert data.rstrip() == stdout.rstrip() + + # workload type not provided detproc.check_error( sess, [ "det", "config-policies", "set", - "experiment", - "--workspace", - workspace_name, - "--config-file", - conf.fixtures_path("config_policies/non-existent.yaml"), + conf.fixtures_path(VALID_NTSC_YAML), ], - "No such file or directory", + "argument workload_type", ) - # workspace name not provided + # invalid path detproc.check_error( sess, [ @@ -80,10 +115,11 @@ def test_set_config_policies() -> None: "config-policies", "set", "experiment", - "--config-file", - conf.fixtures_path("config_policies/valid.yaml"), + conf.fixtures_path("config_policies/non-existent.yaml"), + "--workspace-name", + workspace_name, ], - "the following arguments are required: --workspace", + "No such file or directory", ) # path not provided @@ -94,22 +130,22 @@ def test_set_config_policies() -> None: "config-policies", "set", "experiment", - "--workspace", + "--workspace-name", workspace_name, ], - "the following arguments are required: --config-file", + "the following arguments are required: config_file", ) @pytest.mark.e2e_cpu def test_describe_config_policies() -> None: - sess = api_utils.user_session() + sess = api_utils.admin_session() workspace_name = api_utils.get_random_string() create_workspace_cmd = ["det", "workspace", "create", workspace_name] detproc.check_call(sess, create_workspace_cmd) - # set config policies + # set workspace config policies detproc.check_call( sess, [ @@ -117,14 +153,25 @@ def test_describe_config_policies() -> None: "config-policies", "set", "experiment", - "--workspace", + conf.fixtures_path(VALID_EXPERIMENT_YAML), + "--workspace-name", workspace_name, - "--config-file", - conf.fixtures_path("config_policies/valid.yaml"), ], ) - # no specified return type + # set global config policies + detproc.check_call( + sess, + [ + "det", + "config-policies", + "set", + "experiment", + conf.fixtures_path(VALID_EXPERIMENT_YAML), + ], + ) + + # workspace no specified return type stdout = detproc.check_output( sess, [ @@ -132,15 +179,15 @@ def test_describe_config_policies() -> None: "config-policies", "describe", "experiment", - "--workspace", + "--workspace-name", workspace_name, ], ) - with open(conf.fixtures_path("config_policies/valid.yaml"), "r") as f: + with open(conf.fixtures_path(VALID_EXPERIMENT_YAML), "r") as f: data = f.read() assert data.rstrip() == stdout.rstrip() - # yaml return type + # global no specified return type. stdout = detproc.check_output( sess, [ @@ -148,16 +195,30 @@ def test_describe_config_policies() -> None: "config-policies", "describe", "experiment", - "--workspace", + ], + ) + with open(conf.fixtures_path(VALID_EXPERIMENT_YAML), "r") as f: + data = f.read() + assert data.rstrip() == stdout.rstrip() + + # workspace yaml return type + stdout = detproc.check_output( + sess, + [ + "det", + "config-policies", + "describe", + "experiment", + "--workspace-name", workspace_name, "--yaml", ], ) - with open(conf.fixtures_path("config_policies/valid.yaml"), "r") as f: + with open(conf.fixtures_path(VALID_EXPERIMENT_YAML), "r") as f: data = f.read() assert data.rstrip() == stdout.rstrip() - # json return type + # global yaml return type stdout = detproc.check_output( sess, [ @@ -165,39 +226,59 @@ def test_describe_config_policies() -> None: "config-policies", "describe", "experiment", - "--workspace", + "--yaml", + ], + ) + with open(conf.fixtures_path(VALID_EXPERIMENT_YAML), "r") as f: + data = f.read() + assert data.rstrip() == stdout.rstrip() + + # workspace json return type + stdout = detproc.check_output( + sess, + [ + "det", + "config-policies", + "describe", + "experiment", + "--workspace-name", workspace_name, "--json", ], ) - with open(conf.fixtures_path("config_policies/valid.yaml"), "r") as f: + with open(conf.fixtures_path(VALID_EXPERIMENT_YAML), "r") as f: data = f.read() original_data = yaml.load(data, Loader=yaml.SafeLoader) # check if output is a valid json containing original data assert original_data == json.loads(stdout) - # workspace name not provided - detproc.check_error( + # global json return type + stdout = detproc.check_output( sess, [ "det", "config-policies", "describe", "experiment", + "--json", ], - "the following arguments are required: --workspace", ) + with open(conf.fixtures_path(VALID_EXPERIMENT_YAML), "r") as f: + data = f.read() + original_data = yaml.load(data, Loader=yaml.SafeLoader) + # check if output is a valid json containing original data + assert original_data == json.loads(stdout) @pytest.mark.e2e_cpu def test_delete_config_policies() -> None: - sess = api_utils.user_session() + sess = api_utils.admin_session() workspace_name = api_utils.get_random_string() create_workspace_cmd = ["det", "workspace", "create", workspace_name] detproc.check_call(sess, create_workspace_cmd) - # set config policies + # workspace set config policies detproc.check_call( sess, [ @@ -205,13 +286,13 @@ def test_delete_config_policies() -> None: "config-policies", "set", "experiment", - "--workspace", + conf.fixtures_path(VALID_EXPERIMENT_YAML), + "--workspace-name", workspace_name, - "--config-file", - conf.fixtures_path("config_policies/valid.yaml"), ], ) + # workspace delete config policies stdout = detproc.check_output( sess, [ @@ -219,14 +300,26 @@ def test_delete_config_policies() -> None: "config-policies", "delete", "experiment", - "--workspace", + "--workspace-name", workspace_name, ], ) assert "Successfully deleted" in stdout - # workspace name not provided - detproc.check_error( + # global set config policies + detproc.check_call( + sess, + [ + "det", + "config-policies", + "set", + "experiment", + conf.fixtures_path(VALID_EXPERIMENT_YAML), + ], + ) + + # global delete config policies + stdout = detproc.check_output( sess, [ "det", @@ -234,5 +327,5 @@ def test_delete_config_policies() -> None: "delete", "experiment", ], - "the following arguments are required: --workspace", ) + assert "Successfully deleted" in stdout diff --git a/e2e_tests/tests/fixtures/config_policies/valid_experiment.json b/e2e_tests/tests/fixtures/config_policies/valid_experiment.json new file mode 100644 index 00000000000..8bb185fd6fc --- /dev/null +++ b/e2e_tests/tests/fixtures/config_policies/valid_experiment.json @@ -0,0 +1,12 @@ +{ + "constraints": { + "priority_limit": 10, + "resources": { + "max_slots": 4 + } + }, + + "invariant_config": { + "records_per_epoch": 100 + } +} diff --git a/e2e_tests/tests/fixtures/config_policies/valid.yaml b/e2e_tests/tests/fixtures/config_policies/valid_experiment.yaml similarity index 76% rename from e2e_tests/tests/fixtures/config_policies/valid.yaml rename to e2e_tests/tests/fixtures/config_policies/valid_experiment.yaml index 2f43b09c014..78aa64157ef 100644 --- a/e2e_tests/tests/fixtures/config_policies/valid.yaml +++ b/e2e_tests/tests/fixtures/config_policies/valid_experiment.yaml @@ -3,4 +3,4 @@ constraints: resources: max_slots: 4 invariant_config: - description: test + records_per_epoch: 100 diff --git a/e2e_tests/tests/fixtures/config_policies/valid_ntsc.json b/e2e_tests/tests/fixtures/config_policies/valid_ntsc.json new file mode 100644 index 00000000000..b2de1882452 --- /dev/null +++ b/e2e_tests/tests/fixtures/config_policies/valid_ntsc.json @@ -0,0 +1,8 @@ +{ + "constraints": { + "priority_limit": 10, + "resources": { + "max_slots": 4 + } + } +} diff --git a/e2e_tests/tests/fixtures/config_policies/valid_ntsc.yaml b/e2e_tests/tests/fixtures/config_policies/valid_ntsc.yaml new file mode 100644 index 00000000000..00f0ac54dc9 --- /dev/null +++ b/e2e_tests/tests/fixtures/config_policies/valid_ntsc.yaml @@ -0,0 +1,4 @@ +constraints: + priority_limit: 10 + resources: + max_slots: 4 diff --git a/harness/determined/cli/config_policies.py b/harness/determined/cli/config_policies.py index 0af08da20b3..67953d48d8e 100644 --- a/harness/determined/cli/config_policies.py +++ b/harness/determined/cli/config_policies.py @@ -8,52 +8,84 @@ def describe_config_policies(args: argparse.Namespace) -> None: sess = cli.setup_session(args) + workload_type = "EXPERIMENT" - if args.workload_type.upper() == "NTSC": + if args.workload_type.upper() == "TASKS": workload_type = "NTSC" - wksp = api.workspace_by_name(sess, args.workspace) - resp = bindings.get_GetWorkspaceConfigPolicies( - sess, workloadType=workload_type, workspaceId=wksp.id - ) + if args.workspace_name: + wksp = api.workspace_by_name(sess, args.workspace_name) + wksp_resp = bindings.get_GetWorkspaceConfigPolicies( + sess, workloadType=workload_type, workspaceId=wksp.id + ) + if args.json: + render.print_json(wksp_resp.configPolicies) + else: + print(util.yaml_safe_dump(wksp_resp.configPolicies, default_flow_style=False)) + return + + global_resp = bindings.get_GetGlobalConfigPolicies(sess, workloadType=workload_type) if args.json: - render.print_json(resp.configPolicies) + render.print_json(global_resp.configPolicies) else: - print(util.yaml_safe_dump(resp.configPolicies, default_flow_style=False)) + print(util.yaml_safe_dump(global_resp.configPolicies, default_flow_style=False)) def set_config_policies(args: argparse.Namespace) -> None: sess = cli.setup_session(args) + try: with open(args.config_file, "r") as f: data = f.read() except Exception as e: raise cli.errors.CliError(f"Error opening file: {e}") workload_type = "EXPERIMENT" - if args.workload_type.upper() == "NTSC": + if args.workload_type.upper() == "TASKS": workload_type = "NTSC" - wksp = api.workspace_by_name(sess, args.workspace) - body = bindings.v1PutWorkspaceConfigPoliciesRequest( - workloadType=workload_type, configPolicies=data, workspaceId=wksp.id + if args.workspace_name: + wksp = api.workspace_by_name(sess, args.workspace_name) + wksp_body = bindings.v1PutWorkspaceConfigPoliciesRequest( + workloadType=workload_type, configPolicies=data, workspaceId=wksp.id + ) + wksp_resp = bindings.put_PutWorkspaceConfigPolicies( + sess, workloadType=workload_type, workspaceId=wksp.id, body=wksp_body + ) + print(f"Set {args.workload_type} config policies for workspace {args.workspace_name}:") + print(util.yaml_safe_dump(wksp_resp.configPolicies, default_flow_style=False)) + return + + global_body = bindings.v1PutGlobalConfigPoliciesRequest( + workloadType=workload_type, configPolicies=data ) - resp = bindings.put_PutWorkspaceConfigPolicies( - sess, workloadType=workload_type, workspaceId=wksp.id, body=body + global_resp = bindings.put_PutGlobalConfigPolicies( + sess, workloadType=workload_type, body=global_body ) - print(util.yaml_safe_dump(resp.configPolicies, default_flow_style=False)) + print(f"Set global {args.workload_type} config policies:") + print(util.yaml_safe_dump(global_resp.configPolicies, default_flow_style=False)) def delete_config_policies(args: argparse.Namespace) -> None: sess = cli.setup_session(args) + workload_type = "EXPERIMENT" - if args.workload_type.upper() == "NTSC": + if args.workload_type.upper() == "TASKS": workload_type = "NTSC" - wksp = api.workspace_by_name(sess, args.workspace) - bindings.delete_DeleteWorkspaceConfigPolicies( - sess, workloadType=workload_type, workspaceId=wksp.id - ) - print(f"Successfully deleted {workload_type} config policies for workspace {args.workspace}.") + if args.workspace_name: + wksp = api.workspace_by_name(sess, args.workspace_name) + + bindings.delete_DeleteWorkspaceConfigPolicies( + sess, workloadType=workload_type, workspaceId=wksp.id + ) + print( + f"Successfully deleted {workload_type} config policies for workspace " + f"{args.workspace_name}" + ) + return + + bindings.delete_DeleteGlobalConfigPolicies(sess, workloadType=workload_type) + print(f"Successfully deleted global {workload_type} config policies") args_description: cli.ArgsDescription = [ @@ -65,19 +97,20 @@ def delete_config_policies(args: argparse.Namespace) -> None: cli.Cmd( "describe", describe_config_policies, - "describe config policies", + "display config policies", [ cli.Arg( "workload_type", type=str, - choices=["experiment", "ntsc"], - help="the type (Experiment or NTSC ) of config policies", + choices=["experiment", "tasks"], + help="the type (Experiment or Tasks) of config policies", ), cli.Arg( - "--workspace", + "-w", + "--workspace-name", type=str, - required=True, # change to false when adding --global - help="config policies for the given workspace", + required=False, + help="apply config policies to workspace", ), cli.Group(cli.output_format_args["json"], cli.output_format_args["yaml"]), ], @@ -91,20 +124,21 @@ def delete_config_policies(args: argparse.Namespace) -> None: cli.Arg( "workload_type", type=str, - choices=["experiment", "ntsc"], - help="the type (Experiment or NTSC ) of config policies", + choices=["experiment", "tasks"], + help="the type (Experiment or Tasks) of config policies", ), cli.Arg( - "--workspace", + "config_file", type=str, - required=True, # change to false when adding --global - help="config policies for the given workspace", + help="path to the YAML or JSON file containing defined config policies " + "(can be absolute or relative to the current directory)", ), cli.Arg( - "--config-file", + "-w", + "--workspace-name", type=str, - required=True, - help="path to the yaml file containing defined config policies", + required=False, + help="apply config policies to this workspace", ), ], ), @@ -116,14 +150,15 @@ def delete_config_policies(args: argparse.Namespace) -> None: cli.Arg( "workload_type", type=str, - choices=["experiment", "ntsc"], - help="the type (Experiment or NTSC ) of config policies", + choices=["experiment", "tasks"], + help="the type (Experiment or Tasks) of config policies", ), cli.Arg( - "--workspace", + "-w", + "--workspace-name", type=str, - required=True, # change to false when adding --global - help="config policies for the given workspace", + required=False, + help="apply config policies to workspace", ), ], ),