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.

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

refactor: deprecate devstack from registrar IDA #39

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a416375
refactor: deprecate devstack from registrar IDA
huniafatima-arbi Sep 4, 2024
24528ee
fix: add branch factor in tag name
huniafatima-arbi Sep 6, 2024
418fa58
perf: removed pull request trigger for image build workflow
huniafatima-arbi Sep 6, 2024
3599883
chore: resolved feedback against the PR
huniafatima-arbi Sep 10, 2024
22891bd
chore: temp code for running workflow on pull request
huniafatima-arbi Sep 10, 2024
980281b
fix: update the build and push action
huniafatima-arbi Sep 10, 2024
7ba9c87
fix: updating the keys according to the newer version
huniafatima-arbi Sep 10, 2024
e2117d6
fix: adding context
huniafatima-arbi Sep 10, 2024
c282f45
fix: adding context
huniafatima-arbi Sep 10, 2024
bb7741d
fix: added debugging step
huniafatima-arbi Sep 11, 2024
9ce84cf
fix: ubuntu issue resolved. added git checkout
huniafatima-arbi Sep 11, 2024
5a179b9
fix: changed key
huniafatima-arbi Sep 11, 2024
c3ee07b
fix: changed key
huniafatima-arbi Sep 11, 2024
6966b5b
fix: removing the context
huniafatima-arbi Sep 11, 2024
a8be567
fix: removed test step
huniafatima-arbi Sep 11, 2024
c5f3bbe
fix: fix fix
huniafatima-arbi Sep 11, 2024
9b8978b
fix: check if the git checkout is required
huniafatima-arbi Sep 11, 2024
81e5bdf
fix: check if the git checkout is required
huniafatima-arbi Sep 11, 2024
95317c4
fix: check if the git checkout is required
huniafatima-arbi Sep 11, 2024
0458ff8
fix: output
huniafatima-arbi Sep 11, 2024
34388f7
fix: output
huniafatima-arbi Sep 11, 2024
3c94441
fix: output
huniafatima-arbi Sep 11, 2024
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
50 changes: 50 additions & 0 deletions .github/workflows/push-registrar-image.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
name: Build and Push Docker Images

on:
workflow_dispatch:

schedule:
- cron: "0 4 * * *"
Copy link
Member

Choose a reason for hiding this comment

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

Let's schedule it to run only on the weekdays and mention the default time zone which will be used i.e. UTC

Suggested change
- cron: "0 4 * * *"
- cron: "0 4 * * 1-5" # UTC Time


jobs:
push:
runs-on: ubuntu-latest

steps:
- name: Checkout current repo
uses: actions/checkout@v4

- name: Get tag name
id: get-tag-name
uses: actions/github-script@v5
with:
script: |
const branchName = context.ref.split('/').slice(-1)[0];
const tagName = branchName === 'master' ? 'latest' : branchName;
console.log('Will use tag: ' + tagName);
return tagName;
result-encoding: string

- name: Build and push Dev Docker image
uses: docker/build-push-action@v1
Copy link
Member

Choose a reason for hiding this comment

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

Let's try using the latest available v6 version.

Suggested change
uses: docker/build-push-action@v1
uses: docker/build-push-action@v6

with:
dockerfile: dockerfiles/registrar.Dockerfile
push: true
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_PASSWORD }}
target: dev
repository: edxops/registrar-devstack-dev
Copy link
Member

Choose a reason for hiding this comment

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

As the dev environment is solely used within the devstack setup, there is no need to add a redundant identifier. Simply specifying registrar-dev will suffice. Since we will be updating the image, there's no need to distinguish it from previous versions.

Suggested change
repository: edxops/registrar-devstack-dev
repository: edxops/registrar-dev

tags: ${{ steps.get-tag-name.outputs.result }},${{ github.sha }}

- name: Send failure notification
if: failure()
uses: dawidd6/action-send-mail@v3
with:
server_address: email-smtp.us-east-1.amazonaws.com
server_port: 465
username: ${{secrets.edx_smtp_username}}
password: ${{secrets.edx_smtp_password}}
subject: Push Image to docker.io/edxops failed in registrar
to: [email protected]
from: github-actions <[email protected]>
body: Push Image to docker.io/edxops for registrar failed! For details see "github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ services:
CELERY_BROKER_PASSWORD: password
DJANGO_WATCHMAN_TIMEOUT: 30
ANALYTICS_DASHBOARD_CFG: /edx/etc/registrar.yml
image: edxops/registrar-dev:${OPENEDX_RELEASE:-latest}
image: edxops/registrar-devstack-dev:${OPENEDX_RELEASE:-latest}
Copy link
Member

Choose a reason for hiding this comment

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

This change can be skipped if we decide to use registrar-dev.

Suggested change
image: edxops/registrar-devstack-dev:${OPENEDX_RELEASE:-latest}
image: edxops/registrar-dev:${OPENEDX_RELEASE:-latest}

working_dir: /edx/app/registrar
networks:
default:
Expand Down
95 changes: 95 additions & 0 deletions dockerfiles/registrar.Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
FROM ubuntu:focal as app

# ENV variables for Python 3.12 support
ARG PYTHON_VERSION=3.12
ENV TZ=UTC
ENV TERM=xterm-256color
ENV DEBIAN_FRONTEND=noninteractive

# software-properties-common is needed to setup Python 3.12 env
RUN apt-get update && \
apt-get install -y software-properties-common && \
apt-add-repository -y ppa:deadsnakes/ppa

# System requirements.
RUN apt-get update
RUN DEBIAN_FRONTEND=noninteractive apt-get install -qy \
Copy link
Member

Choose a reason for hiding this comment

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

we've already defined the flag above so it is not needed here.

Suggested change
RUN DEBIAN_FRONTEND=noninteractive apt-get install -qy \
RUN apt-get install -qy \

git-core \
language-pack-en \
build-essential \
# libmysqlclient-dev header files needed to use native C implementation for MySQL-python for performance gains.
libmysqlclient-dev \
# mysqlclient wont install without libssl-dev
libssl-dev \
# mysqlclient>=2.2.0 requires pkg-config (https://github.com/PyMySQL/mysqlclient/issues/620)
pkg-config \
curl \
python3-pip \
python${PYTHON_VERSION} \
python${PYTHON_VERSION}-dev \
python${PYTHON_VERSION}-distutils

# need to use virtualenv pypi package with Python 3.12
RUN curl -sS https://bootstrap.pypa.io/get-pip.py | python${PYTHON_VERSION}
RUN pip install virtualenv

# delete apt package lists because we do not need them inflating our image
RUN rm -rf /var/lib/apt/lists/*

# Python is Python3.
RUN ln -s /usr/bin/python3 /usr/bin/python

# Setup zoneinfo for Python 3.12
RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

# Use UTF-8.
RUN locale-gen en_US.UTF-8
ENV LANG en_US.UTF-8
ENV LANGUAGE en_US:en
ENV LC_ALL en_US.UTF-8

ARG COMMON_CFG_DIR="/edx/etc"
ARG COMMON_APP_DIR="/edx/app"
ARG REGISTRAR_APP_DIR="${COMMON_APP_DIR}/registrar"
ARG REGISTRAR_VENV_DIR="${COMMON_APP_DIR}/venvs/registrar"
ARG REGISTRAR_CODE_DIR="${REGISTRAR_APP_DIR}"

ENV PATH="$REGISTRAR_VENV_DIR/bin:$PATH"
ENV REGISTRAR_APP_DIR ${REGISTRAR_APP_DIR}
ENV REGISTRAR_CODE_DIR ${REGISTRAR_CODE_DIR}

# Working directory will be root of repo.
WORKDIR ${REGISTRAR_CODE_DIR}

# cloning git repo
RUN curl -L https://github.com/openedx/registrar/archive/refs/heads/master.tar.gz | tar -xz --strip-components=1


RUN virtualenv -p python${PYTHON_VERSION} --always-copy ${REGISTRAR_VENV_DIR}

RUN pip install --upgrade pip setuptools

ENV REGISTRAR_CFG="${COMMON_CFG_DIR}/registrar.yml"

# Expose ports.
EXPOSE 18734
EXPOSE 18735

FROM app as dev

RUN pip install --no-cache-dir -r requirements/devstack.txt

ENV DJANGO_SETTINGS_MODULE registrar.settings.devstack

CMD while true; do python ./manage.py runserver 0.0.0.0:18734; sleep 2; done

FROM app as prod

RUN pip install --no-cache-dir -r ${REGISTRAR_CODE_DIR}/requirements/production.txt

ENV DJANGO_SETTINGS_MODULE registrar.settings.production

# Gunicorn 19 does not log to stdout or stderr by default. Once we are past gunicorn 19, the logging to STDOUT need not be specified.
CMD ["gunicorn", "--workers=2", "--name", "registrar", "-c", "/edx/app/registrar/registrar/docker_gunicorn_configuration.py", "--log-file", "-", "--max-requests=1000", "registrar.wsgi:application"]
Copy link
Member

Choose a reason for hiding this comment

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

We're currently using gunicorn==23.0.0 in production.txt so the comment can be removed, and the command can be simplified as following:

Suggested change
CMD ["gunicorn", "--workers=2", "--name", "registrar", "-c", "/edx/app/registrar/registrar/docker_gunicorn_configuration.py", "--log-file", "-", "--max-requests=1000", "registrar.wsgi:application"]
CMD ["gunicorn", "--workers=2", "--name", "registrar", "-c", "/edx/app/registrar/registrar/docker_gunicorn_configuration.py", "--max-requests=1000", "registrar.wsgi:application"]


Copy link
Member

Choose a reason for hiding this comment

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

nit: remove extra lines from end of file.


Loading