Updated mode: http2 support #4004
Conversation
paasta_tools/kubernetes_tools.py
Outdated
| @@ -1371,7 +1371,7 @@ def get_liveness_probe( | |||
| timeout_seconds=timeout_seconds, | |||
| ) | |||
|
|
|||
| if mode == "http" or mode == "https": | |||
| if mode in ["http", "https", "http2"]: | |||
| path = self.get_healthcheck_uri(service_namespace_config) | |||
| probe.http_get = V1HTTPGetAction( | |||
| path=path, port=self.get_container_port(), scheme=mode.upper() | |||
There was a problem hiding this comment.
mode.upper() will lead to LivenessProbe w/ scheme=HTTP which isn't supported , so we need to do some conversion here
There was a problem hiding this comment.
I am handling this in get_healthcheck_mode func as most of the places we are relying on right conversion of healthcheck_mode so fixing that and making http as default value for http2 service if healthcheck_mode is not defined explicitly will take care of this case.
paasta_tools/cli/cmds/info.py
Outdated
| @@ -37,7 +37,7 @@ | |||
| ) | |||
|
|
|||
| # modes that depend on smartstack port cannot be tested via paasta proxies, so we exclude those | |||
| TESTABLE_SERVICE_MODES = {"http", "https"} | |||
| TESTABLE_SERVICE_MODES = {"http", "https", "http2"} | |||
There was a problem hiding this comment.
get_smartstack_endpoints() will probably need some special handling since that passes through the mode as the uri scheme - but the http2 scheme is still http
There was a problem hiding this comment.
and i think the same needs to happen in get_deployments_strings()?
There was a problem hiding this comment.
handling this with url_scheme even though service is http2 we will use http in endpoint url. Fixed in both cases.
paasta_tools/cli/cmds/local_run.py
Outdated
| @@ -168,7 +168,7 @@ def run_healthcheck_on_container( | |||
| healthcheck_result = perform_cmd_healthcheck( | |||
| docker_client, container_id, healthcheck_data, timeout | |||
| ) | |||
| elif healthcheck_mode == "http" or healthcheck_mode == "https": | |||
| elif healthcheck_mode in ["http", "https", "http2"]: | |||
There was a problem hiding this comment.
non-blocking suggestion: might be nice to have a constant for HTTP_MODES = {"http", "https", "http2"} since we have this list in multiple places
| @@ -424,7 +424,7 @@ def get_healthcheck_for_instance( | |||
| mode = service_manifest.get_healthcheck_mode(smartstack_config) | |||
| hostname = socket.getfqdn() | |||
|
|
|||
| if mode == "http" or mode == "https": | |||
| if mode in ["http", "https", "http2"]: | |||
| path = service_manifest.get_healthcheck_uri(smartstack_config) | |||
| healthcheck_command = "%s://%s:%d%s" % (mode, hostname, random_port, path) | |||
There was a problem hiding this comment.
(same as above re: http2 having a uri scheme of http)
Support for HTTP2 mode & updated SmartStack schema.