Redesign Build Deployment Process (External) #113
Conversation
Need to make it available outside the docker folder as the build context was specified in the docker compose as the repo root directory and corresponding paths in the Dockerfile were relative to the root directory. Additionally, added Environment variables defined in docker compose file. The image wasn't running directly after building because these things were missing: - needed to add db container as well - ports needed to be exposed for both db and dashboard container - common network needed to be added under which both the containers had to be linked - all environment variables had to be added, especially important ones like DB_HOST
Added Dockerfile in repo root.
Encountered error while building and pushing docker image as a part of the CI. Due to config.py mentioned in COPY command in Dockerfile not being there in the repo online. Added config-fake.py in its place for now, so image is pushed to Dockerhub. But eventually, will need to handle as per decision to consolidate differences in external and internal repos.
Replaced config.py with config-fake.py in external Dockerfile.
In the external public Docker image, make the config-fake.py available as config.py by copying it over into the container as config.py Internally, if custom config.py is needed, can copy it over in the internal Dockerfile. But might not even need the config.py
Copy config-fake.py as config.py
Found usage of two variables referring to same port. Had exposed using both variable names. But since same port exposed, should be fine to expose just once.
Removed expose port duplication
1. Changes in related files including Dockerfile for loading OpenPATH logo from local image rather than URL link. 2. Changed sed to jq in start.sh. 3. Changed dash version to latest v 2.16.1 4. Added new Dockerfile for image_build_push yml. Removed assets/ from COPY command as only contents of assets/ were being copied to root app directory. In COPY assets command, copy entire directory instead of just css file.
Multiple changes for external repo differences
Currently the branch specified - "image-push-merge" is available locally on my system. I use it to test the automated docker image push mechanism whenever any changes are merged to this branch. Once, everything looks good, need to change this to master or main as per the repo.
Added TODO to change image push branch
Had added it initially for testing purposes. Can remove now so it doesn't expose any sensitive info.
Removed printing Docker username
|
Checking in the first initial set of changes for consolidating the differences in the external and internal versions. Currently included these changes:
Next are various changes to consolidate differences; made in this commit.
|
- Option 1: Can we read in these values directly from environment variables instead of reading from a class? - Option 2: Create a class that makes all environment variables available as python variables For now, I’m going ahead with reading in these variables directly. So, will need to replace all uses of CognitoConfig class with variable name directly in files that use it. Shankari mentioned in a commit whether we even need config.py? e-mission@927817a Also, removed references to both config-fake.py and config.py in the Dockerfiles.
Replacing CognitoConfig class with ENV variables
Added actual template value directly where the variable was being used through the config.py file.
Replaced dictionary in config.py
The other ENV vars that were added in Docker-compose files are currently present in the Dockerfile in the external repo root. Removing them from there and can add them as per requirement. For local testing, add them when “docker run” is used using the -e flag. For usage in stage / production, can be set by cloud team in AWS Codebuild as they currently do.
Removing ENV variables from Dockerfile
Changed pillow version to 10.3.0 Editing online manually to test if Github action still works after re-adding image_build_push.yml file.
Can directly set DB_HOST since we can now use environment variables. No need to use jq or sed to replace file contents and copy over files.
Removed sed / jq usage from start scripts
Docker image created locally is not taking the name that I passed in for it. Hopefully, every new image created in an individual runner is named the same thing, so that this workaround is successful.
Adding artifact upload for internal repo to be able to pull image tag.
Trying to do this the way I originally planned.
Reverting the changes I made to test docker compose
Adding other env vars as a template
Adding other env vars to .env. Otherwise, the file will get overwritten with the server tag only every time.
Remove extraneous fi
Because that makes sense.
There was a problem hiding this comment.
In addition to the main change below, you also need to copy over the changes from
e-mission/em-public-dashboard#125
in here
| echo "COGNITO_CLIENT_ID=''" > .env | ||
| echo "COGNITO_CLIENT_SECRET=''" > .env | ||
| echo "COGNITO_REDIRECT_URL=''" > .env | ||
| echo "COGNITO_TOKEN_ENDPOINT=''" > .env | ||
| echo "COGNITO_USER_POOL_ID=''" > .env | ||
| echo "COGNITO_REGION=''" > .env | ||
| echo "COGNITO_AUTH_URL=''" > .env |
There was a problem hiding this comment.
I am confused. If the goal is to have all the environment variables stored in .env, then why are you overriding them here? And if the goal is not have them in here, then why do we need to fill them in?
Basically, if you are going to override the values with empty strings anyway, then it is not clear that you need the variables in the first place.
There was a problem hiding this comment.
@nataliejschultz Had added these to the .env file in this commit.
The reason for doing this was that GitHub Actions was overwriting the .env file as a part of the workflow. During the workflow only the latest server tag is written into the .env file.
There was a problem hiding this comment.
Added a separate template file in this commit.
Keeping the .env file only for latest server image tag.
There was a problem hiding this comment.
@MukuFlash03 Are you sure that multiple .env files are supported in docker? How did you test this to make sure that if the values are set, they are visible in the codebase?
There was a problem hiding this comment.
I don't think they would be set directly.
I thought of this approach, I had mentioned this in the commit:
I am expecting the users would either set them directly in the system or use the docker compose prod yml file.
So, with this option, the cognito env file would literally just be a template.
The second option is that we will need to load the env variables from the specified file using the python library python-dotenv. Will need to add this in requirements.txt.
So, the code in utils/cognito_utils.py (for instance) would look like:
from dotenv import load_dotenv
load_dotenv(.env.cognito)
def get_tokens(code):
client_id = os.getenv("COGNITO_CLIENT_ID", '')
client_secret = os.getenv("COGNITO_CLIENT_SECRET", '')
redirect_uri = os.getenv("COGNITO_REDIRECT_URL", '')
token_endpoint = os.getenv("COGNITO_TOKEN_ENDPOINT", '')
There was a problem hiding this comment.
python-dotenv is overkill
I guess it is fair to assume that the users will set the values in the environment, we do assume that for the server code. Once we figure out how to maintain the list of environment variables from the server, we should use the same approach here. Ideally this would be through a search so that we don't have to maintain a separate template file and ensure that it doesn't bitrot.
There was a problem hiding this comment.
Note also that we don't have to support a .env with a single tag. There are ways of editing files programmatically that don't involve >>
| - name: Add, Commit, Push changes to .env file | ||
| run: | | ||
| git config --local user.email "[email protected]" | ||
| git config --local user.name "GitHub Action" |
There was a problem hiding this comment.
nit: make name more meaningful.
These store tags differently depending on the trigger event - Push OR Workflow dispatch
Similarly done with public-dashboard. See comment: e-mission/em-public-dashboard#125 (comment)
… workflow file The .env file is meant to be used only for the docker image tag to store the latest server image tag. If other variables are stored there, the workflow will overwrite the .env file to store only the image tag whenever the CI/CD runs. Hence, separating out the Cognito variables in a separate template file. I am expecting the users would either set them directly in the system or use the docker compose prod yml file.
This was mainly needed for Push event but since Workflow dispatch event would be setting the latest server image tag in .env file, the push event can read from this file directly.
This is probably the first and only time we'll need to manually change the image tag. Hence forth, the CI/CD should automatically update the .env file with the latest tag whenever the workflow dispatch is triggered on new merges to server.
There was a problem hiding this comment.
@MukuFlash03 Also, please make sure to check that the file names are valid, consistent with e-mission/em-public-dashboard#125 (comment)
and
e-mission/em-public-dashboard#125 (comment)
.env.cognito.template
Outdated
There was a problem hiding this comment.
@MukuFlash03 Are you sure that multiple .env files are supported in docker? How did you test this to make sure that if the values are set, they are visible in the codebase?
This will ensure that if the CI/CD pipeline fails in any prior steps such as the docker related ones, the .env file isn't updated. This is because the the docker failures can include errors image not found which can occur due to incorrect tags.
|
Squash merging again, with 90 commits for 13 files. |
Updating docs to reference changes from the Redesign PR e-mission#113
This PR serves as the implementation for this issue for redesigning our build and deployment processes across e-mission-server, join, admin-dash, public-dash primarily.
Collaborating on this along with @nataliejschultz .
All four redesign PRs for the external repos:
E-mission-server: e-mission/e-mission-server#961
Join: e-mission/nrel-openpath-join-page#29
Admin-dash: #113
Public-dash: e-mission/em-public-dashboard#125
A rough plan can be:
1st phase
2nd phase: