Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added converter service to prod #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+67 −40
Open

Added converter service to prod #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5ef447
First attempt at getting api into prod
kiat-huang Jul 4, 2020
9c8ae58
Create deploy on prod
kiat-huang Jul 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
FROM python:3.7-stretch
FROM python:3.7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would explicitly specify the os using 3.7-buster. This makes sure we have a stable base os which changes only when we specify it in the image build.


RUN apt-get update && mkdir -p /app
RUN apt-get update && apt-get install -y --no-install-recommends \
python3.7 \
python3-pip \
python3-flask
# python3.7-venv
Comment on lines +3 to +7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I python is available in the image. It is built from the sources. If we want to use the packaged version then we should use a different base image.


WORKDIR /app
RUN mkdir -p /app

COPY src /app/
COPY requirements.txt /app/
RUN pip install -r requirements.txt

COPY /src/ /app/
WORKDIR /app

ENV FLASK_APP "api.py"
RUN pip3 install -r requirements.txt --upgrade
RUN pip3 install setuptools --upgrade
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is setuptools installed separately?


ENV FLASK_APP=src/api.py
ENV FLASK_DEBUG=1
ENV LC_ALL=C.UTF-8
ENV LANG=C.UTF-8

45 changes: 30 additions & 15 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,52 @@
DOCKER_REPO ?= gcr.io/online-bridge-hackathon-2020
# Project / Org
GCP_PROJECT ?= globalbridge-app
GKE_ZONE ?= europe-west4-b

# Service / App
GKE_CLUSTER_NAME ?= prod-cluster
RELEASE_NAME ?= converter
K8S_NS ?= prod-${RELEASE_NAME}
EXTERNAL_ADDRESS ?= ${RELEASE_NAME}.prod.globalbridge.app

# Docker Config
DOCKER_REPO ?= gcr.io/${GCP_PROJECT}
VERSION ?= $(shell cat VERSION)
DOCKER_TAG=${DOCKER_REPO}/data-converter-api:${VERSION}
DOCKER_TAG = ${DOCKER_REPO}/${RELEASE_NAME}:${VERSION}

EXTERNAL_ADDRES ?= data-converter.hackathon.globalbridge.app

DDS_K8S_NS ?= data-converter-api
GCP_PROJECT ?= online-bridge-hackathon-2020
GKE_CLUSTER_NAME ?= hackathon-cluster
GKE_ZONE ?= europe-west3-b
ifeq (${SILENT},1)
VERBOSE_TEST :=
else
VERBOSE_TEST := -v
endif


release: build push

build:
libdds/.git:
git submodule update --init
Comment on lines +26 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no submodules which means the submodule target should be dropped.


build:
docker build -t ${DOCKER_TAG} .

push:
docker push ${DOCKER_TAG}

deploy: set_gcp_context ensure_ns
helm upgrade --install data-converter-api ./chart \
helm upgrade --install ${RELEASE_NAME} ./chart \
--set image="${DOCKER_TAG}" \
--set externalHostname="${EXTERNAL_ADDRES}" \
--namespace ${DDS_K8S_NS} \
--history-max=10
--set externalHostname="${EXTERNAL_ADDRESS}" \
--namespace ${K8S_NS} \
# --history-max=10

uninstall: set_gcp_context
helm del data-converter-api --namespace ${DDS_K8S_NS}
helm del ${RELEASE_NAME} --namespace ${K8S_NS}

set_gcp_context:
gcloud container clusters get-credentials ${GKE_CLUSTER_NAME} --zone ${GKE_ZONE} --project ${GCP_PROJECT}

ensure_ns:
kubectl create ns ${DDS_K8S_NS} || :
kubectl create ns ${K8S_NS} || :

run_local_tests:
python3 -m unittest discover
python3 -m unittest discover ${VERBOSE_TEST}
2 changes: 1 addition & 1 deletion VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.0.1
0.0.2
2 changes: 1 addition & 1 deletion chart/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: v1
description: A helm chart for Data-Converter API
name: data-converter-api
name: converter
version: 0.1.0
16 changes: 8 additions & 8 deletions chart/templates/data-converter-api.yaml → chart/templates/converter.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ kind: Deployment
metadata:
name: {{ .Release.Name }}-deployment
labels:
app: data-converter-api
app: {{ .Release.Name }}
spec:
replicas: {{ int .Values.replicas }}
selector:
matchLabels:
app: data-converter-api
app: {{ .Release.Name }}
template:
metadata:
labels:
app: data-converter-api
app: {{ .Release.Name }}
spec:
containers:
- name: api
Expand All @@ -25,20 +25,20 @@ spec:
- flask
- run
- --host=0.0.0.0
- --port=5000
- --port={{ .Values.targetPort }}
ports:
- containerPort: 5000
- containerPort: {{ .Values.targetPort }}
---
apiVersion: v1
kind: Service
metadata:
name: {{ .Release.Name }}-service
spec:
selector:
app: data-converter-api
app: {{ .Release.Name }}
ports:
- port: 80
targetPort: 5000
targetPort: {{ .Values.targetPort }}
protocol: TCP
name: http
---
Expand All @@ -50,7 +50,7 @@ metadata:
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
cert-manager.io/cluster-issuer: letsencrypt-production
labels:
app: data-converter-api
app: {{ .Release.Name }}
name: {{ .Release.Name }}-ingress
spec:
tls:
Expand Down
7 changes: 4 additions & 3 deletions chart/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
image: gcr.io/online-bridge-hackathon-2020/data-converter-api:0.0.0-dev
image: gcr.io/globalbridge-app/converter
replicas: 1
externalHostname: data-converter.hackathon.globalbridge.app
tlsSecret: data-converter-api-tls-secret
externalHostname: converter.prod.globalbridge.app
tlsSecret: converter-tls-secret
targetPort: 5000
7 changes: 1 addition & 6 deletions requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
aniso8601==8.0.0
click==7.1.2
Flask==1.1.2
Flask-Cors==3.0.8
Flask-RESTful==0.3.8
itsdangerous==1.1.0
Jinja2==2.11.2
MarkupSafe==1.1.1
aniso8601==8.0.0
Comment on lines -6 to +4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requirements.txt is supposed to hold versions for implicit dependencies too. I can see how that helps avoid random bugs from upgrading a library without noticing.

I would be happy to use dependencies without versions too. That would be a useful way for development if we have over 95% test coverage. The high test coverage would then allow us catch any surprise bugs from library upgrades.

pytz==2020.1
six==1.15.0
Werkzeug==1.0.1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a belief that Wekzeug is a required dependency for flask. Is that correct?

7 changes: 7 additions & 0 deletions src/api.py
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
#!/usr/bin/env python3.7

import sys
import os
sys.path.append('os.getcwd()/python-libs')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is a relative path required?

sys.path.append('/usr/lib/python3.7')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding default python library path doesn't seem correct.


from flask import Flask, request
from flask_cors import CORS
from flask_restful import Resource, Api
Expand Down