#10 WIP: OpenShift Support
Merged 3 years ago by frantisekz. Opened 3 years ago by frantisekz.

file modified
+3 -2
@@ -25,6 +25,7 @@ 

                        python3-dateutil \

                        python3-bodhi-client \

                        python3-pytz \

+                       python3-psycopg2 \

                        python3-requests \

                        python3-setuptools

  
@@ -58,8 +59,8 @@ 

  # EXPOSE 5005/tcp

  EXPOSE 5005

  

- RUN echo "SECRET_KEY = '`cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 64 | head -n 1`'" >> /etc/oraculum/settings.py

- RUN echo "SQLALCHEMY_DATABASE_URI = 'sqlite:////var/tmp/oraculum.sqlite'" >> /etc/oraculum/settings.py

+ #RUN echo "SECRET_KEY = '`cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 64 | head -n 1`'" >> /etc/oraculum/settings.py

+ #RUN echo "SQLALCHEMY_DATABASE_URI = 'sqlite:////var/tmp/oraculum.sqlite'" >> /etc/oraculum/settings.py

  RUN echo "OIDC_CLIENT_SECRETS = '/etc/oraculum/client_secrets.json'" >> /etc/oraculum/settings.py

  

  CMD [ "runserver" ]

file modified
+3 -1
@@ -9,5 +9,7 @@ 

  OIDC_REQUIRE_VERIFIED_EMAIL = False

  OIDC_SCOPES = ['openid', 'email', 'profile']

  

- SKIP_CACHE_AGE_CHECK = False # Skip checking cache age in runtime, make sure to set up cron with "runcli.py sync" if set to True

+ # MAX_DB_AGE will be ignored if FORCE_CACHED_DATA, DB cache will be used no matter how old it is

+ # make sure to set up cron with "runcli.py sync" if set to True

+ FORCE_CACHED_DATA = False

  MAX_DB_AGE = 1800 # Max cache age allowed in seconds (30 minutes)

file modified
+4
@@ -2,6 +2,10 @@ 

  if [[ $1 == runserver ]]; then

      echo $CLIENT_SECRETS > /etc/oraculum/client_secrets.json

  

+     # Prepare database

+     oraculum init_db

+     oraculum upgrade_db

+ 

      mod_wsgi-express-3 start-server /usr/share/oraculum/oraculum.wsgi --user apache --group apache \

      --port 5005 --threads 5 --include-file /etc/httpd/conf.d/oraculum.conf --log-level info \

      --log-to-terminal --access-log --startup-log

file modified
+8 -1
@@ -19,12 +19,13 @@ 

  

  from flask import Flask

  from flask_sqlalchemy import SQLAlchemy

- 

  from flask_caching import Cache

  from flask_cors import CORS

  from flask_oidc import OpenIDConnect

  from flask_login import LoginManager

  

+ from . import config

+ 

  import logging

  import os

  
@@ -37,6 +38,9 @@ 

  app = Flask(__name__)

  app.secret_key = 'not-really-a-secret'

  

+ # Is this an OpenShift deployment?

+ openshift_production = os.getenv('OPENSHIFT_PROD')

+ 

  cache = Cache(app, config={'CACHE_TYPE': 'simple'})

  

  # Load default config, then override that with a config file
@@ -52,6 +56,9 @@ 

  

  app.config.from_object(default_config_obj)

  

+ if openshift_production:

+     config.openshift_config(app.config, openshift_production)

+ 

  config_file = os.environ.get('ORACULUM_CONFIG', default_config_file)

  

  if os.path.exists(config_file):

file modified
+3 -1
@@ -49,6 +49,7 @@ 

  

      if current_rev:

          print("Database already initialized and at rev %s - not re-initializing" % current_rev)

+         return True

      else:

          print("Initializing Database")

          db.drop_all()
@@ -57,6 +58,7 @@ 

          print("Initializing alembic version")

  

          al_command.stamp(alembic_cfg, "head")

+         return True

  

  

  def upgrade_db():
@@ -67,7 +69,7 @@ 

  def sync():

      print("Refreshing DB Cache")

      app.config['MAX_DB_AGE'] = 0

-     app.config['SKIP_CACHE_AGE_CHECK'] = False

+     app.config['FORCE_CACHED_DATA'] = False

      db_utils.refresh_data("get_actions", controllers.main.get_actions())

      db_utils.refresh_data("api_v1_landing_page", controllers.main.get_landing_page_data())

      db_utils.refresh_data("api_v1_libkarma",

file modified
+30 -1
@@ -17,6 +17,8 @@ 

  # Authors:

  #    Josef Skladanka <jskladan@redhat.com>

  

+ import os

+ import sys

  

  class Config(object):

      DEBUG = True
@@ -40,13 +42,14 @@ 

      OIDC_REQUIRE_VERIFIED_EMAIL = False

      OIDC_SCOPES = ['openid', 'email', 'profile']

  

-     SKIP_CACHE_AGE_CHECK = False

+     FORCE_CACHED_DATA = False

      MAX_DB_AGE = 1800  # keep data cached for 30 minutes

  

  

  class ProductionConfig(Config):

      PRODUCTION = True

      DEBUG = False

+     FORCE_CACHED_DATA = True

  

  

  class DevelopmentConfig(Config):
@@ -60,3 +63,29 @@ 

  class TestingConfig(Config):

      TRAP_BAD_REQUEST_ERRORS = True

      TESTING = True

+ 

+ def openshift_config(config_object, openshift_production):

+     # First, get db details from env

+     try:

+         config_object["SQLALCHEMY_DATABASE_URI"] = "postgresql+psycopg2://%s:%s@%s:%s/%s" % (

+             os.environ["POSTGRESQL_USER"],

+             os.environ["POSTGRESQL_PASSWORD"],

+             os.environ["POSTGRESQL_SERVICE_HOST"],

+             os.environ["POSTGRESQL_SERVICE_PORT"],

+             os.environ["POSTGRESQL_DATABASE"]

+         )

+         config_object["SECRET_KEY"] = os.environ["SECRET_KEY"]

+         config_object["CLIENT_SECRETS"] = os.environ["CLIENT_SECRETS"]

+     except(KeyError):

+         print("OpenShift mode enabled but required values couldn't be fetched. "

+               "Check, if you have these variables defined in you env: "

+               "(POSTGRESQL_[USER, PASSWORD, DATABASE, SERVICE_HOST, SERVICE_PORT], SECRET_KEY, CLIENT_SECRETS)", file=sys.stderr)

+         sys.exit(1)

+ 

+     # Get some more data from OpenShift, if set

+     if os.getenv("KANBAN_CONFIG"):

+         config_object["KANBAN_CONFIG"] = os.getenv("KANBAN_CONFIG")

+ 

+     # Some final touches for oidc

+     if os.getenv("OVERWRITE_REDIRECT_URI"):

+         config_object["OVERWRITE_REDIRECT_URI"] = os.getenv("OVERWRITE_REDIRECT_URI")

file modified
+2 -5
@@ -28,9 +28,9 @@ 

  def is_new_enough(db_time):

      """

      Checks if given db_time is new enough according to MAX_DB_AGE

-     Skips check if SKIP_CACHE_AGE_CHECK is set to True

+     Skips check if FORCE_CACHED_DATA is set to True

      """

-     if app.config['SKIP_CACHE_AGE_CHECK']:

+     if app.config['FORCE_CACHED_DATA']:

          return True

      if not db_time:

          return False
@@ -41,10 +41,7 @@ 

  def refresh_data(provider, data):

      """

      Refreshes given data for given provider in the db

-     Returns immediately when SKIP_CACHE_AGE_CHECK is set

      """

-     if app.config['SKIP_CACHE_AGE_CHECK']:

-         return True

      row = CachedData.query.filter_by(provider=provider).first()

      if row:

          row.time_created = datetime.utcnow()

Support getting db details from OpenShift env

Contains: https://pagure.io/fedora-qa/oraculum/pull-request/9

(Code taken from blockerbugs and testdays port)

@jskladan if we want to have development Dockerfile, this will break it (I've commented out all writes to settings.py as OpenShift handles settings through env).

The idea I have to get both OpenShift support and dev Dockerfile is to load configfile only when not in OpenShift env. WDYT?

rebased onto 307709f

3 years ago

1 new commit added

  • Dockerfile: Specify OIDC_CLIENT_SECRETS via settings.py
3 years ago

I'd rather see (and I've already discussed this with @lbrabec yesterday) a multistaged Dockerfile where one can specifically target either Development or Deployment setup.

Do what you must to make the dockerfile working in openshift correctly, and we'll take care of the devel setup after that.

Just a question - did you comment those lines in Dockerfile because they break the openshift workflow, or because these lines are redundant, since Openshift handles these values differently?

Nipicky, but please rename this to openshift_prod or openshift_production to give the variable more semantic meaning.

1 new commit added

  • Dockerfile: Require python3-psycopg2
3 years ago

I don't really understand the intent behind or openshift == "0" here. What is the usecase for this particular bit? Do we even use the TEST env at all?

Just a question - did you comment those lines in Dockerfile because they break the openshift workflow, or because these lines are redundant, since Openshift handles these values differently?

Yes, I've commented writing of SECRET_KEY and SQLALCHEMY_DATABASE_URI in Dockerfile because settings.py would override values that are fetched from OpenShift env otherwise.

I know I'm being overly pushy on this topic, but how about FORCE_CACHED_DATA instead? Just a proposal, though, I'm absolutely fine with ALWAYS_USE_CACHE too..

I don't really understand the intent behind or openshift == "0" here. What is the usecase for this particular bit? Do we even use the TEST env at all?

Hmm, the only purpose was to be able to specify non-production OpenShift env at some later point should we need to do so. It can be removed if you want :)

I know I'm being overly pushy on this topic, but how about FORCE_CACHED_DATA instead? Just a proposal, though, I'm absolutely fine with ALWAYS_USE_CACHE too..

Yeah, I can change that, don't have strong feelings for ALWAYS_USE_CACHE, it was just something that came to my mind.

1 new commit added

  • Rename variable openshift > openshift_production
3 years ago

just an honest question - is it necessary to use postgres here? Since the data we store is non-persistent anyway, we could easily do with in-memory (or disk-stored) sqlite inside the container, and if the container dies, we don't care much about losing the cached data (and I guess it communishift dies, the postgresql pod dies with it anyway).
It might slow down upgrades, though (could be worked around by running the sync during build time, so the rebuilt container starts with fresh data already), so I need to repeat I'm not pushing/forcing this, just honestly asking.

rebased onto 5d1501f

3 years ago

Might be worth changing the message here ;)

just an honest question - is it necessary to use postgres here? Since the data we store is non-persistent anyway, we could easily do with in-memory (or disk-stored) sqlite inside the container, and if the container dies, we don't care much about losing the cached data (and I guess it communishift dies, the postgresql pod dies with it anyway).
It might slow down upgrades, though (could be worked around by running the sync during build time, so the rebuilt container starts with fresh data already), so I need to repeat I'm not pushing/forcing this, just honestly asking.

Might be worth changing the message here ;)

just an honest question - is it necessary to use postgres here? Since the data we store is non-persistent anyway, we could easily do with in-memory (or disk-stored) sqlite inside the container, and if the container dies, we don't care much about losing the cached data (and I guess it communishift dies, the postgresql pod dies with it anyway).
It might slow down upgrades, though (could be worked around by running the sync during build time, so the rebuilt container starts with fresh data already), so I need to repeat I'm not pushing/forcing this, just honestly asking.

Hmm, it probably isn't necessary, but few things made me think that it might be better to use postgres.

  • Sync will be done from a different pod than the one that is serving oraculum- it probably could be made to use file/sqlite storage, but it seemed easier/better to use postgres
  • We might want to have more pods running in paralel in the future - if load on oraculum increases (easy karma or some other projects might start using it...), i thought it will be cleaner instead of accessing the file from persistent storage from different pods

I can try to make cron working with sqlite if you want though :)

1 new commit added

  • Update warning for missing of some env vars
3 years ago

The error message has me thinking you had a different usecase in mind - the initialize_db does not check whether "db is behind alembic". It just checks whether the db has the Alembic revision stamp (any at all).

Looks like the intent was to also make sure the db is up-to-date (based on the error message, that is) so please either change the message or code to be in sync with the intent. THX

I can try to make cron working with sqlite if you want though :)

No need, the explanation is good enough for me. THX

1 new commit added

  • OpenShift: Simplify logic
3 years ago

1 new commit added

  • OVERWRITE_REDIRECT_URI
3 years ago

1 new commit added

  • Containers: Move db init/upgrade to entrypoint
3 years ago

rebased onto b6d5c8a

3 years ago

Pull-Request has been merged by frantisekz

3 years ago