Address code review comments

tools/gke/kubernetes_api.py

@@ -31,9 +31,9 @@
 import requests
 import json
 
-REQUEST_TIMEOUT_SECS = 10
+_REQUEST_TIMEOUT_SECS = 10
 
-def make_pod_config(pod_name, image_name, container_port_list, cmd_list,
+def _make_pod_config(pod_name, image_name, container_port_list, cmd_list,
                     arg_list):
   """Creates a string containing the Pod defintion as required by the Kubernetes API"""
   body = {
@@ -69,12 +69,12 @@ def make_pod_config(pod_name, image_name, container_port_list, cmd_list,
   return json.dumps(body)
 
 
-def make_service_config(service_name, pod_name, service_port_list,
+def _make_service_config(service_name, pod_name, service_port_list,
                         container_port_list, is_headless):
   """Creates a string containing the Service definition as required by the Kubernetes API.
 
   NOTE:
-  This creates either a Headless* Service or 'LoadBalancer' service depending on
+  This creates either a Headless Service or 'LoadBalancer' service depending on
   the is_headless parameter. For Headless services, there is no 'type' attribute
   and the 'clusterIP' attribute is set to 'None'. Also, if the service is
   Headless, Kubernetes creates DNS entries for Pods - i.e creates DNS A-records
@@ -120,44 +120,44 @@ def make_service_config(service_name, pod_name, service_port_list,
   return json.dumps(body)
 
 
-def print_connection_error(msg):
+def _print_connection_error(msg):
   print('ERROR: Connection failed. Did you remember to run Kubenetes proxy on'
         'localhost (i.e kubectl proxy --port=<proxy_port>) ?. Error: %s' % msg)
 
-def do_post(post_url, api_name, request_body):
+def _do_post(post_url, api_name, request_body):
   """Helper to do HTTP POST.
 
   Note:
   1) On success, Kubernetes returns a success code of 201(CREATED) not 200(OK)
   2) A response code of 509(CONFLICT) is interpreted as a success code (since
   the error is most likely due to the resource already existing). This makes
-  do_pos() idempotent which is semantically desirable.
+  _do_post() idempotent which is semantically desirable.
   """
   is_success = True
   try:
-    r = requests.post(post_url, data=request_body, timeout=REQUEST_TIMEOUT_SECS)
+    r = requests.post(post_url, data=request_body, timeout=_REQUEST_TIMEOUT_SECS)
     if r.status_code == requests.codes.conflict:
       print('WARN: Looks like the resource already exists. Api: %s, url: %s' %
             (api_name, post_url))
-    elif r.status_code != requests.codes.created and r.status_code != requests.codes.ok:
+    elif r.status_code != requests.codes.created:
-      print('ERROR: %s API returned error. HTTP response: %s' %
+      print('ERROR: %s API returned error. HTTP response: (%d) %s' %
-            (api_name, r.text))
+            (api_name, r.status_code, r.text))
       is_success = False
   except(requests.exceptions.Timeout, requests.exceptions.ConnectionError) as e:
     is_success = False
-    print_connection_error(str(e))
+    _print_connection_error(str(e))
   return is_success
 
 
-def do_delete(del_url, api_name):
+def _do_delete(del_url, api_name):
   """Helper to do HTTP DELETE.
 
   Note: A response code of 404(NOT_FOUND) is treated as success to keep
-  do_delete() idempotent.
+  _do_delete() idempotent.
   """
   is_success = True
   try:
-    r = requests.delete(del_url, timeout=REQUEST_TIMEOUT_SECS)
+    r = requests.delete(del_url, timeout=_REQUEST_TIMEOUT_SECS)
     if r.status_code == requests.codes.not_found:
       print('WARN: The resource does not exist. Api: %s, url: %s' %
             (api_name, del_url))
@@ -167,20 +167,20 @@ def do_delete(del_url, api_name):
       is_success = False
   except(requests.exceptions.Timeout, requests.exceptions.ConnectionError) as e:
     is_success = False
-    print_connection_error(str(e))
+    _print_connection_error(str(e))
   return is_success
 
 
 def create_service(kube_host, kube_port, namespace, service_name, pod_name,
                    service_port_list, container_port_list, is_headless):
-  """Creates a either a Headless Service or a LoadBalancer Service depending
+  """Creates either a Headless Service or a LoadBalancer Service depending
   on the is_headless parameter.
   """
   post_url = 'http://%s:%d/api/v1/namespaces/%s/services' % (
       kube_host, kube_port, namespace)
-  request_body = make_service_config(service_name, pod_name, service_port_list,
+  request_body = _make_service_config(service_name, pod_name, service_port_list,
                                      container_port_list, is_headless)
-  return do_post(post_url, 'Create Service', request_body)
+  return _do_post(post_url, 'Create Service', request_body)
 
 
 def create_pod(kube_host, kube_port, namespace, pod_name, image_name,
@@ -193,13 +193,13 @@ def create_pod(kube_host, kube_port, namespace, pod_name, image_name,
   Controller' which creates a configurable number of 'identical Replicas' of
   Pods and automatically restarts any Pods in case of failures (for eg: Machine
   failures in Kubernetes). This makes it less flexible for our test use cases
-  where we might want to slightly different set of args to each Pod and hence we
+  where we might want slightly different set of args to each Pod. Hence we
-  directly create Pods (and not care much about Kubernetes failures since those
+  directly create Pods and not care much about Kubernetes failures since those
-  are very rare).
+  are very rare.
   """
   post_url = 'http://%s:%d/api/v1/namespaces/%s/pods' % (kube_host, kube_port,
                                                          namespace)
-  request_body = make_pod_config(pod_name, image_name, container_port_list,
+  request_body = _make_pod_config(pod_name, image_name, container_port_list,
                                  cmd_list, arg_list)
   return do_post(post_url, 'Create Pod', request_body)
 
@@ -207,10 +207,10 @@ def create_pod(kube_host, kube_port, namespace, pod_name, image_name,
 def delete_service(kube_host, kube_port, namespace, service_name):
   del_url = 'http://%s:%d/api/v1/namespaces/%s/services/%s' % (
       kube_host, kube_port, namespace, service_name)
-  return do_delete(del_url, 'Delete Service')
+  return _do_delete(del_url, 'Delete Service')
 
 
 def delete_pod(kube_host, kube_port, namespace, pod_name):
   del_url = 'http://%s:%d/api/v1/namespaces/%s/pods/%s' % (kube_host, kube_port,
                                                            namespace, pod_name)
-  return do_delete(del_url, 'Delete Pod')
+  return _do_delete(del_url, 'Delete Pod')