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

fix: failing docker builds #24

Merged
merged 3 commits into from
Oct 27, 2021
Merged

fix: failing docker builds #24

merged 3 commits into from
Oct 27, 2021

Conversation

notsatan
Copy link
Contributor

The Problem

As mentioned in #23, docker builds were failing with the error message

Error response from daemon: failed to parse Dockerfile: ENV must have two arguments

The cause of the issue was that the ENV command in dockerfile expects two values - the name of the environment variable, and its value. The GITHUB_TOKEN environment variable had no value set against it, causing docker builds to fail

ENV GITHUB_TOKEN

The Fix

This PR resolves #23

Added a new build argument named AUTH_TOKEN that would then be set as the value for the GITHUB_TOKEN environment variable in dockerfile

Updated the docker build command in README!

The docker build command was failing midway since `ENV` needs two
parameters - the name of the variable, and its value. In the
dockerfile, the variable had no value set against it - causing
the error

Fix this by introducing a new build arg, and setting this as the
value for the environment variable
Update the docker build command in the readme to reflect changes
introduced with the `AUTH_TOKEN` arg introduced in the dockerfile
in the previous commit
@notsatan
Copy link
Contributor Author

Given that the GITHUB_TOKEN environment variable is needed by the docker image during runtime (and is not optional), I'm thinking of making it be mandatory in the dockerfile - i.e. docker builds would fail if the variable is not set with the build command.

Since the `GITHUB_TOKEN` being used as the environment variable in
the docker image was needed (and not optional) - allowing docker
builds to succeed, only to result in a crash when the container was
run would be a waste of time.

With this commit, the `AUTH_TOKEN` argument is mandatory for the
`docker build` command to succeed. This greatly cuts down the
response time to detect an error with the build command
@notsatan
Copy link
Contributor Author

notsatan commented Oct 22, 2021

@Abhinav-26 this should fix the issue with failing docker builds.

I've also modified the dockerfile to ensure that the docker build command fails if AUTH_TOKEN (which will be used to set the GITHUB_TOKEN env variable) is not provided

RUN test -n "$AUTH_TOKEN"

If this is not needed, or should be a separate issue/PR by itself - let me know, I'll selectively roll this commit back!

@arushi09-hub
Copy link

Hi There,
We thank you for your valuable contributions. Please follow the steps below for the Giveaways Entry -

  1. Once the PR is raised you can join the discord community for the upcoming updates.
  2. After joining the community, send your PR link on the #contrib channel with the tag @hacktoberfest.
  3. We will evaluate the PR and once it is accepted , we will send you the swags.

@Abhinav-26 Abhinav-26 merged commit 4eb6766 into devtron-labs:main Oct 27, 2021
@notsatan notsatan deleted the fix/patch-issue-23 branch October 27, 2021 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue building docker image with dockerfile
3 participants