Skip to content

Commit d05f3b4

Browse files
authored
First wave of fixes for remote-run (#4033)
* Describe 403 code for remote-run apis * Since OPA can deny remote-run requests, it'd be nice to have the right code returned from the API server rather than 500 cause the code is not in the spec (this was actually raise by @Qmando in #4022 but I turned the suggestion down, mistakenly, sorry) * Add more output to remote-run cli * ... because it's way too silent now, and pod don't come online instantaneusly * Fix wrong kwarg in paasta_cleanup_remote_run_resources * This is just linters failing me (and too much mocking in unit tests) * Drop unneeded pod options for remote-run jobs * We do not want this to become a way for people to deploy services in prod, so in general these pods should not have routable IPs (it'll be a different story for toolbox containers as those will run sshd). * In my recent testing the job pod was failing to becomes ready due to hacheck. I think that may be due to some zookeeper locking which interfered with the existing replicas of the service, but at any rate, we don't want that, or any other sidecar, as this is effectively meant to be "local-run, but in a pod"
1 parent 0a9862e commit d05f3b4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

66 files changed

+114
-71
lines changed

paasta_tools/api/api_docs/oapi.yaml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
openapi: 3.0.0
33
info:
44
title: Paasta API
5-
version: 1.1.0
5+
version: 1.1.1
66
servers:
77
- url: "{scheme}://{host}/{basePath}"
88
variables:
@@ -1216,6 +1216,8 @@ paths:
12161216
schema:
12171217
$ref: '#/components/schemas/RemoteRunOutcome'
12181218
description: Successfully started remote-run sandbox
1219+
"403":
1220+
description: Unauthorized to remote-run this service
12191221
"404":
12201222
description: Service instance not found
12211223
"500":
@@ -1252,6 +1254,8 @@ paths:
12521254
schema:
12531255
$ref: '#/components/schemas/RemoteRunOutcome'
12541256
description: Remote run pod stopped
1257+
"403":
1258+
description: Unauthorized to remote-run this service
12551259
"404":
12561260
description: Service instance not found
12571261
"500":
@@ -1288,6 +1292,8 @@ paths:
12881292
schema:
12891293
$ref: '#/components/schemas/RemoteRunOutcome'
12901294
description: Pod status information
1295+
"403":
1296+
description: Unauthorized to remote-run this service
12911297
"404":
12921298
description: Service instance not found
12931299
"500":
@@ -1324,6 +1330,8 @@ paths:
13241330
schema:
13251331
$ref: '#/components/schemas/RemoteRunToken'
13261332
description: Token generated successfully
1333+
"403":
1334+
description: Unauthorized to remote-run this service
13271335
"404":
13281336
description: Service instance not found
13291337
"500":

paasta_tools/api/api_docs/swagger.json

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"swagger": "2.0",
33
"info": {
44
"title": "Paasta API",
5-
"version": "1.1.0"
5+
"version": "1.1.1"
66
},
77
"basePath": "/v1",
88
"schemes": [
@@ -856,6 +856,9 @@
856856
"$ref": "#/definitions/RemoteRunOutcome"
857857
}
858858
},
859+
"403": {
860+
"description": "Unauthorized to remote-run this service"
861+
},
859862
"404": {
860863
"description": "Deployment key not found"
861864
},
@@ -904,6 +907,9 @@
904907
"$ref": "#/definitions/RemoteRunOutcome"
905908
}
906909
},
910+
"403": {
911+
"description": "Unauthorized to remote-run this service"
912+
},
907913
"404": {
908914
"description": "Service instance not found"
909915
},
@@ -952,6 +958,9 @@
952958
"$ref": "#/definitions/RemoteRunOutcome"
953959
}
954960
},
961+
"403": {
962+
"description": "Unauthorized to remote-run this service"
963+
},
955964
"404": {
956965
"description": "Service instance not found"
957966
},
@@ -998,6 +1007,9 @@
9981007
"$ref": "#/definitions/RemoteRunToken"
9991008
}
10001009
},
1010+
"403": {
1011+
"description": "Unauthorized to remote-run this service"
1012+
},
10011013
"404": {
10021014
"description": "Service instance not found"
10031015
},

paasta_tools/cli/cmds/remote_run.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ def paasta_remote_run_start(
6161
print(f"Error from PaaSTA APIs while starting job: {start_response.message}")
6262
return 1
6363

64+
print(
65+
f"Triggered remote-run job for {args.service}. Waiting for pod to come online..."
66+
)
6467
start_time = time.time()
6568
while time.time() - start_time < args.timeout:
6669
poll_response = client.remote_run.remote_run_poll(
@@ -69,7 +72,9 @@ def paasta_remote_run_start(
6972
start_response.job_name,
7073
)
7174
if poll_response.status == 200:
75+
print("")
7276
break
77+
print(f"\rStatus: {poll_response.message}", end="")
7378
time.sleep(10)
7479
else:
7580
print("Timed out while waiting for job to start")
@@ -79,6 +84,8 @@ def paasta_remote_run_start(
7984
print("Successfully started remote-run job")
8085
return 0
8186

87+
print("Pod ready, establishing interactive session...")
88+
8289
token_response = client.remote_run.remote_run_token(
8390
args.service, args.instance, user
8491
)

paasta_tools/kubernetes/bin/paasta_cleanup_remote_run_resources.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def parse_args() -> argparse.Namespace:
7474
def main():
7575
args = parse_args()
7676
kube_client = KubeClient()
77-
age_limit = datetime.now(tzinfo=timezone.utc) - timedelta(seconds=args.max_age)
77+
age_limit = datetime.now(tz=timezone.utc) - timedelta(seconds=args.max_age)
7878
for namespace in get_all_managed_namespaces(kube_client):
7979
clean_namespace(kube_client, namespace, age_limit)
8080

paasta_tools/kubernetes_tools.py

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,6 +1436,7 @@ def get_kubernetes_containers(
14361436
aws_ebs_volumes: Sequence[AwsEbsVolume],
14371437
secret_volumes: Sequence[SecretVolume],
14381438
service_namespace_config: ServiceNamespaceConfig,
1439+
include_sidecars: bool = True,
14391440
) -> Sequence[V1Container]:
14401441
ports = [self.get_container_port()]
14411442
# MONK-1130
@@ -1471,11 +1472,13 @@ def get_kubernetes_containers(
14711472
projected_sa_volumes=self.get_projected_sa_volumes(),
14721473
),
14731474
)
1474-
containers = [service_container] + self.get_sidecar_containers( # type: ignore
1475-
system_paasta_config=system_paasta_config,
1476-
service_namespace_config=service_namespace_config,
1477-
hacheck_sidecar_volumes=hacheck_sidecar_volumes,
1478-
)
1475+
containers = [service_container]
1476+
if include_sidecars:
1477+
containers += self.get_sidecar_containers( # type: ignore
1478+
system_paasta_config=system_paasta_config,
1479+
service_namespace_config=service_namespace_config,
1480+
hacheck_sidecar_volumes=hacheck_sidecar_volumes,
1481+
)
14791482
return containers
14801483

14811484
def get_readiness_probe(
@@ -2052,11 +2055,15 @@ def format_kubernetes_job(
20522055
self,
20532056
job_label: str,
20542057
deadline_seconds: int = 3600,
2058+
keep_routable_ip: bool = False,
2059+
include_sidecars: bool = False,
20552060
) -> V1Job:
20562061
"""Create the config for launching the deployment as a Job
20572062
20582063
:param str job_label: value to set for the "job type" label
20592064
:param int deadline_seconds: maximum allowed duration for the job
2065+
:param bool keep_routable_ip: maintain routable IP annotation in pod template
2066+
:param bool include_sidecars: do not discard sidecar containers when building pod spec
20602067
:return: job object
20612068
"""
20622069
try:
@@ -2073,6 +2080,8 @@ def format_kubernetes_job(
20732080
git_sha=git_sha,
20742081
system_paasta_config=system_paasta_config,
20752082
restart_on_failure=False,
2083+
include_sidecars=include_sidecars,
2084+
force_no_routable_ip=not keep_routable_ip,
20762085
),
20772086
),
20782087
)
@@ -2205,6 +2214,8 @@ def get_pod_template_spec(
22052214
git_sha: str,
22062215
system_paasta_config: SystemPaastaConfig,
22072216
restart_on_failure: bool = True,
2217+
include_sidecars: bool = True,
2218+
force_no_routable_ip: bool = False,
22082219
) -> V1PodTemplateSpec:
22092220
service_namespace_config = load_service_namespace_config(
22102221
service=self.service, namespace=self.get_nerve_namespace()
@@ -2214,8 +2225,10 @@ def get_pod_template_spec(
22142225
)
22152226

22162227
hacheck_sidecar_volumes = system_paasta_config.get_hacheck_sidecar_volumes()
2217-
has_routable_ip = self.has_routable_ip(
2218-
service_namespace_config, system_paasta_config
2228+
has_routable_ip = (
2229+
"false"
2230+
if force_no_routable_ip
2231+
else self.has_routable_ip(service_namespace_config, system_paasta_config)
22192232
)
22202233
annotations: KubePodAnnotations = {
22212234
"smartstack_registrations": json.dumps(self.get_registrations()),
@@ -2239,6 +2252,7 @@ def get_pod_template_spec(
22392252
secret_volumes=self.get_secret_volumes(),
22402253
system_paasta_config=system_paasta_config,
22412254
service_namespace_config=service_namespace_config,
2255+
include_sidecars=include_sidecars,
22422256
),
22432257
share_process_namespace=True,
22442258
node_selector=self.get_node_selector(),

paasta_tools/paastaapi/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
88
No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator) # noqa: E501
99
10-
The version of the OpenAPI document: 1.1.0
10+
The version of the OpenAPI document: 1.1.1
1111
Generated by: https://openapi-generator.tech
1212
"""
1313

paasta_tools/paastaapi/api/autoscaler_api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
66
No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator) # noqa: E501
77
8-
The version of the OpenAPI document: 1.1.0
8+
The version of the OpenAPI document: 1.1.1
99
Generated by: https://openapi-generator.tech
1010
"""
1111

paasta_tools/paastaapi/api/default_api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
66
No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator) # noqa: E501
77
8-
The version of the OpenAPI document: 1.1.0
8+
The version of the OpenAPI document: 1.1.1
99
Generated by: https://openapi-generator.tech
1010
"""
1111

paasta_tools/paastaapi/api/remote_run_api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
66
No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator) # noqa: E501
77
8-
The version of the OpenAPI document: 1.1.0
8+
The version of the OpenAPI document: 1.1.1
99
Generated by: https://openapi-generator.tech
1010
"""
1111

paasta_tools/paastaapi/api/resources_api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
66
No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator) # noqa: E501
77
8-
The version of the OpenAPI document: 1.1.0
8+
The version of the OpenAPI document: 1.1.1
99
Generated by: https://openapi-generator.tech
1010
"""
1111

0 commit comments

Comments
 (0)