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

dockerfile update #1103

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

dockerfile update #1103

wants to merge 2 commits into from

Conversation

OlegPhenomenon
Copy link
Contributor

@OlegPhenomenon OlegPhenomenon commented Jul 20, 2023

Tasks:

  • Update Dockerfile.

Problem

  • When deploying an auction, docker indicates that it cannot find the downloaded chromedriver. The basis of docker's work is in layers, and each layer tries to isolate itself from each other. The instructions were to first download the chromedriver and then by another command install it. That counts as two different instructions in two different layers.
  • There is also another problem. internetee/ruby:3.2.2 was added, but the code that is specified in the Dockerfile is identical to what is in the base image. This is also why duplication errors occur.

Solution

  • I got rid of the unnecessary instructions, and added the user "rails" and gave him permissions to use the docker environment. This is considered good security practice.
  • "Removed the unnecessary installation of postgres-client, now it's installed in the base image.
  • Added a new suffix to the base image tag, the base image looks like this: internetee/ruby:3.2.2-refactor. The 'refactor' suffix will be dropped when it's time to update the base image for a new Ruby version. I added 'refactor' so there's an option to switch to the old image internetee/ruby:3.2.2 and to make it clear which image is being used."
  • "I also updated Dockerfile images with the suffixes: staging, deploy, and generic. They also use the base image internetee/ruby:3.2.2 and have the rails roles

What problems might be

  • There's an issue with the Dockerfile with the 'deploy' suffix. The thing is, when adding the rails role, it tries to use the same name and uses this name to connect to the database. The solution I see is adding a user with this name to the database."

How to test?

  • Deploy the auction using this dockerfile.
  • Manually verify that the system is working properly.
  • Also check that unit tests and system tests pass without problems.

Why Not for merge?
I also noticed that the base images are subject to refactoring. Again, as a good practice, a separate user should be added so that operations in the docker are not performed as root. The version of postgresql and a number of other libraries should also be updated. Once the base images have been updated and we are satisfied that there are no problems, then we can merge the updated dockerfile into the branch.

@OlegPhenomenon OlegPhenomenon added the Not for merging for testing and development only label Jul 20, 2023
@viezly
Copy link

viezly bot commented Jul 20, 2023

Changes preview:

Legend:

👀 Review pull request on Viezly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not for merging for testing and development only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant