Fix Kubernetes config provider for recent changes in Kub API

Kubernetes secret volumes are now mounted as read-only, so we have to write the files *only* via the Kub API

Fixes https://jira.coreos.com/browse/QUAY-911

conf/nginx/server-base.conf.jnj

@@ -99,6 +99,14 @@ location ~ ^/cnr {
     limit_req zone=repositories burst=10;
 }
 
+location ~ ^/api/suconfig {
+    proxy_pass   http://web_app_server;
+
+    # For suconfig, set our read timeout as super large for both DB migrations
+    # and awaiting for secrets to be updated.
+    proxy_read_timeout 2000;
+}
+
 location ~ ^/v2 {
     # If we're being accessed via v1.quay.io, pretend we don't support v2.
     if ($host = "v1.quay.io") {

util/config/provider/__init__.py

@@ -2,8 +2,6 @@ from util.config.provider.fileprovider import FileConfigProvider
 from util.config.provider.testprovider import TestConfigProvider
 from util.config.provider.k8sprovider import KubernetesConfigProvider
 
-import os
-
 def get_config_provider(config_volume, yaml_filename, py_filename, testing=False, kubernetes=False):
   """ Loads and returns the config provider for the current environment. """
   if testing:
@@ -13,4 +11,3 @@ def get_config_provider(config_volume, yaml_filename, py_filename, testing=False
     return KubernetesConfigProvider(config_volume, yaml_filename, py_filename)
 
   return FileConfigProvider(config_volume, yaml_filename, py_filename)
-

util/config/provider/basefileprovider.py

@@ -0,0 +1,70 @@
+import os
+import logging
+
+from util.config.provider.baseprovider import (BaseProvider, import_yaml, export_yaml,
+                                               CannotWriteConfigException)
+
+logger = logging.getLogger(__name__)
+
+class BaseFileProvider(BaseProvider):
+  """ Base implementation of the config provider that reads the data from the file system. """
+  def __init__(self, config_volume, yaml_filename, py_filename):
+    self.config_volume = config_volume
+    self.yaml_filename = yaml_filename
+    self.py_filename = py_filename
+
+    self.yaml_path = os.path.join(config_volume, yaml_filename)
+    self.py_path = os.path.join(config_volume, py_filename)
+
+  def update_app_config(self, app_config):
+    if os.path.exists(self.py_path):
+      logger.debug('Applying config file: %s', self.py_path)
+      app_config.from_pyfile(self.py_path)
+
+    if os.path.exists(self.yaml_path):
+      logger.debug('Applying config file: %s', self.yaml_path)
+      import_yaml(app_config, self.yaml_path)
+
+  def get_config(self):
+    if not self.config_exists():
+      return None
+
+    config_obj = {}
+    import_yaml(config_obj, self.yaml_path)
+    return config_obj
+
+  def config_exists(self):
+    return self.volume_file_exists(self.yaml_filename)
+
+  def volume_exists(self):
+    return os.path.exists(self.config_volume)
+
+  def volume_file_exists(self, filename):
+    return os.path.exists(os.path.join(self.config_volume, filename))
+
+  def get_volume_file(self, filename, mode='r'):
+    return open(os.path.join(self.config_volume, filename), mode=mode)
+
+  def get_volume_path(self, directory, filename):
+    return os.path.join(directory, filename)
+
+  def list_volume_directory(self, path):
+    dirpath = os.path.join(self.config_volume, path)
+    if not os.path.exists(dirpath):
+      return None
+
+    if not os.path.isdir(dirpath):
+      return None
+
+    return os.listdir(dirpath)
+
+  def requires_restart(self, app_config):
+    file_config = self.get_config()
+    if not file_config:
+      return False
+
+    for key in file_config:
+      if app_config.get(key) != file_config[key]:
+        return True
+
+    return False

util/config/provider/baseprovider.py

@@ -1,6 +1,9 @@
 import logging
 import yaml
 
+from abc import ABCMeta, abstractmethod
+from six import add_metaclass
+
 from jsonschema import validate, ValidationError
 
 from util.config.schema import CONFIG_SCHEMA
@@ -56,6 +59,7 @@ def export_yaml(config_obj, config_file):
     raise CannotWriteConfigException(str(ioe))
 
 
+@add_metaclass(ABCMeta)
 class BaseProvider(object):
   """ A configuration provider helps to load, save, and handle config override in the application.
   """
@@ -64,61 +68,61 @@ class BaseProvider(object):
   def provider_id(self):
     raise NotImplementedError
 
+  @abstractmethod
   def update_app_config(self, app_config):
     """ Updates the given application config object with the loaded override config. """
-    raise NotImplementedError
 
+  @abstractmethod
   def get_config(self):
     """ Returns the contents of the config override file, or None if none. """
-    raise NotImplementedError
 
+  @abstractmethod
   def save_config(self, config_object):
     """ Updates the contents of the config override file to those given. """
-    raise NotImplementedError
 
+  @abstractmethod
   def config_exists(self):
     """ Returns true if a config override file exists in the config volume. """
-    raise NotImplementedError
 
+  @abstractmethod
   def volume_exists(self):
     """ Returns whether the config override volume exists. """
-    raise NotImplementedError
 
+  @abstractmethod
   def volume_file_exists(self, filename):
     """ Returns whether the file with the given name exists under the config override volume. """
-    raise NotImplementedError
 
+  @abstractmethod
   def get_volume_file(self, filename, mode='r'):
     """ Returns a Python file referring to the given name under the config override volume. """
-    raise NotImplementedError
 
+  @abstractmethod
   def write_volume_file(self, filename, contents):
     """ Writes the given contents to the config override volumne, with the given filename. """
-    raise NotImplementedError
 
+  @abstractmethod
   def remove_volume_file(self, filename):
     """ Removes the config override volume file with the given filename. """
-    raise NotImplementedError
 
+  @abstractmethod
   def list_volume_directory(self, path):
     """ Returns a list of strings representing the names of the files found in the config override
         directory under the given path. If the path doesn't exist, returns None.
     """
-    raise NotImplementedError
 
+  @abstractmethod
   def save_volume_file(self, filename, flask_file):
     """ Saves the given flask file to the config override volume, with the given
         filename.
     """
-    raise NotImplementedError
 
+  @abstractmethod
   def requires_restart(self, app_config):
     """ If true, the configuration loaded into memory for the app does not match that on disk,
         indicating that this container requires a restart.
     """
-    raise NotImplementedError
 
+  @abstractmethod
   def get_volume_path(self, directory, filename):
     """ Helper for constructing file paths, which may differ between providers. For example,
     kubernetes can't have subfolders in configmaps """
-    raise NotImplementedError

util/config/provider/fileprovider.py

@@ -1,8 +1,9 @@
 import os
 import logging
 
-from util.config.provider.baseprovider import (BaseProvider, import_yaml, export_yaml,
+from util.config.provider.baseprovider import export_yaml, CannotWriteConfigException
-                                               CannotWriteConfigException)
+from util.config.provider.basefileprovider import BaseFileProvider
+
 
 logger = logging.getLogger(__name__)
 
@@ -16,52 +17,19 @@ def _ensure_parent_dir(filepath):
     raise CannotWriteConfigException(str(ioe))
 
 
-class FileConfigProvider(BaseProvider):
+class FileConfigProvider(BaseFileProvider):
-  """ Implementation of the config provider that reads the data from the file system. """
+  """ Implementation of the config provider that reads and writes the data
+      from/to the file system. """
   def __init__(self, config_volume, yaml_filename, py_filename):
-    self.config_volume = config_volume
+    super(FileConfigProvider, self).__init__(config_volume, yaml_filename, py_filename)
-    self.yaml_filename = yaml_filename
-    self.py_filename = py_filename
-
-    self.yaml_path = os.path.join(config_volume, yaml_filename)
-    self.py_path = os.path.join(config_volume, py_filename)
 
   @property
   def provider_id(self):
     return 'file'
 
-  def update_app_config(self, app_config):
-    if os.path.exists(self.py_path):
-      logger.debug('Applying config file: %s', self.py_path)
-      app_config.from_pyfile(self.py_path)
-
-    if os.path.exists(self.yaml_path):
-      logger.debug('Applying config file: %s', self.yaml_path)
-      import_yaml(app_config, self.yaml_path)
-
-  def get_config(self):
-    if not os.path.exists(self.yaml_path):
-      return None
-
-    config_obj = {}
-    import_yaml(config_obj, self.yaml_path)
-    return config_obj
-
   def save_config(self, config_obj):
     export_yaml(config_obj, self.yaml_path)
 
-  def config_exists(self):
-    return self.volume_file_exists(self.yaml_filename)
-
-  def volume_exists(self):
-    return os.path.exists(self.config_volume)
-
-  def volume_file_exists(self, filename):
-    return os.path.exists(os.path.join(self.config_volume, filename))
-
-  def get_volume_file(self, filename, mode='r'):
-    return open(os.path.join(self.config_volume, filename), mode=mode)
-
   def write_volume_file(self, filename, contents):
     filepath = os.path.join(self.config_volume, filename)
     _ensure_parent_dir(filepath)
@@ -78,16 +46,6 @@ class FileConfigProvider(BaseProvider):
     filepath = os.path.join(self.config_volume, filename)
     os.remove(filepath)
 
-  def list_volume_directory(self, path):
-    dirpath = os.path.join(self.config_volume, path)
-    if not os.path.exists(dirpath):
-      return None
-
-    if not os.path.isdir(dirpath):
-      return None
-
-    return os.listdir(dirpath)
-
   def save_volume_file(self, filename, flask_file):
     filepath = os.path.join(self.config_volume, filename)
     _ensure_parent_dir(filepath)
@@ -99,17 +57,3 @@ class FileConfigProvider(BaseProvider):
       raise CannotWriteConfigException(str(ioe))
 
     return filepath
-
-  def requires_restart(self, app_config):
-    file_config = self.get_config()
-    if not file_config:
-      return False
-
-    for key in file_config:
-      if app_config.get(key) != file_config[key]:
-        return True
-
-    return False
-  
-  def get_volume_path(self, directory, filename):
-    return os.path.join(directory, filename)

util/config/provider/k8sprovider.py

@@ -2,11 +2,13 @@ import os
 import logging
 import json
 import base64
+import time
 
 from requests import Request, Session
 
-from util.config.provider.baseprovider import get_yaml, CannotWriteConfigException
+from util.config.provider.baseprovider import CannotWriteConfigException, get_yaml
-from util.config.provider.fileprovider import FileConfigProvider
+from util.config.provider.basefileprovider import BaseFileProvider
+
 
 logger = logging.getLogger(__name__)
 
@@ -20,14 +22,12 @@ SERVICE_ACCOUNT_TOKEN_PATH = '/var/run/secrets/kubernetes.io/serviceaccount/toke
 QE_NAMESPACE = os.environ.get('QE_K8S_NAMESPACE', 'quay-enterprise')
 QE_CONFIG_SECRET = os.environ.get('QE_K8S_CONFIG_SECRET', 'quay-enterprise-config-secret')
 
-class KubernetesConfigProvider(FileConfigProvider):
+class KubernetesConfigProvider(BaseFileProvider):
   """ Implementation of the config provider that reads and writes configuration
       data from a Kubernetes Secret. """
   def __init__(self, config_volume, yaml_filename, py_filename):
     super(KubernetesConfigProvider, self).__init__(config_volume, yaml_filename, py_filename)
 
-    self.yaml_filename = yaml_filename
-
     # Load the service account token from the local store.
     if not os.path.exists(SERVICE_ACCOUNT_TOKEN_PATH):
       raise Exception('Cannot load Kubernetes service account token')
@@ -35,34 +35,26 @@ class KubernetesConfigProvider(FileConfigProvider):
     with open(SERVICE_ACCOUNT_TOKEN_PATH, 'r') as f:
       self._service_token = f.read()
 
-    # Make sure the configuration volume exists.
-    if not self.volume_exists():
-      os.makedirs(config_volume)
-
   @property
   def provider_id(self):
     return 'k8s'
 
-  def save_config(self, config_obj):
+  def get_volume_path(self, directory, filename):
-    self._update_secret_file(self.yaml_filename, get_yaml(config_obj))
+    # NOTE: Overridden to ensure we don't have subdirectories, which aren't supported
-    super(KubernetesConfigProvider, self).save_config(config_obj)
+    # in Kubernetes secrets.
-
+    return "_".join([directory.rstrip('/'), filename])
-  def write_volume_file(self, filename, contents):
-    super(KubernetesConfigProvider, self).write_volume_file(filename, contents)
-
-    try:
-      self._update_secret_file(filename, contents)
-    except IOError as ioe:
-      raise CannotWriteConfigException(str(ioe))
 
   def volume_file_exists(self, filename):
+    # NOTE: Overridden because we don't have subdirectories, which aren't supported
+    # in Kubernetes secrets.
     secret = self._lookup_secret()
     if not secret or not secret.get('data'):
       return False
     return filename in secret['data']
 
-
   def list_volume_directory(self, path):
+    # NOTE: Overridden because we don't have subdirectories, which aren't supported
+    # in Kubernetes secrets.
     secret = self._lookup_secret()
 
     if not secret:
@@ -74,9 +66,16 @@ class KubernetesConfigProvider(FileConfigProvider):
         paths.append(filename[len(path) + 1:])
     return paths
 
-  def remove_volume_file(self, filename):
+  def save_config(self, config_obj):
-    super(KubernetesConfigProvider, self).remove_volume_file(filename)
+    self._update_secret_file(self.yaml_filename, get_yaml(config_obj))
 
+  def write_volume_file(self, filename, contents):
+    try:
+      self._update_secret_file(filename, contents)
+    except IOError as ioe:
+      raise CannotWriteConfigException(str(ioe))
+
+  def remove_volume_file(self, filename):
     try:
       self._update_secret_file(filename, None)
     except IOError as ioe:
@@ -126,6 +125,27 @@ class KubernetesConfigProvider(FileConfigProvider):
 
     self._assert_success(self._execute_k8s_api('PUT', secret_url, secret))
 
+    # Wait until the local mounted copy of the secret has been updated, as
+    # this is an eventual consistency operation, but the caller expects immediate
+    # consistency.
+    while True:
+      matching_files = set()
+      for secret_filename, encoded_value in secret['data'].iteritems():
+        expected_value = base64.b64decode(encoded_value)
+        try:
+          with self.get_volume_file(secret_filename) as f:
+            contents = f.read()
+
+          if contents == expected_value:
+            matching_files.add(secret_filename)
+        except IOError:
+          continue
+
+      if matching_files == set(secret['data'].keys()):
+        break
+
+      # Sleep for a second and then try again.
+      time.sleep(1)
 
   def _lookup_secret(self):
     secret_url = 'namespaces/%s/secrets/%s' % (QE_NAMESPACE, QE_CONFIG_SECRET)
@@ -148,6 +168,3 @@ class KubernetesConfigProvider(FileConfigProvider):
 
     request = Request(method, url, data=data, headers=headers)
     return session.send(request.prepare(), verify=False, timeout=2)
-
-  def get_volume_path(self, directory, filename):
-    return "_".join([directory.rstrip('/'), filename])

util/config/provider/test/test_k8sprovider.py

@@ -8,7 +8,12 @@ from test.fixtures import *
 
 class TestKubernetesConfigProvider(KubernetesConfigProvider):
   def __init__(self):
+    self.config_volume = ''
     self.yaml_filename = 'yaml_filename'
+    self.py_filename = None
+
+    self.yaml_path = os.path.join(self.config_volume, self.yaml_filename)
+
     self._service_token = 'service_token'
     self._execute_k8s_api = Mock()
 
@@ -21,7 +26,6 @@ class TestKubernetesConfigProvider(KubernetesConfigProvider):
 ])
 def test_get_volume_path(directory, filename, expected):
   provider = TestKubernetesConfigProvider()
-  
   assert expected == provider.get_volume_path(directory, filename)