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 build #894

Closed
wants to merge 8 commits into from
Closed

Fix failing Docker build #894

wants to merge 8 commits into from

Conversation

kaihendry
Copy link
Contributor

#750 which I think solves
#892 (comment)
because it locks down to .meteor/release

#750 which I think solves
#892 (comment)
because it locks down to .meteor/release
@kaihendry
Copy link
Contributor Author

CI is failing here since it's not building in the more controlled Dockerfile environment

I need to update .travis.yml

https://github.com/unee-t/frontend/blob/master/.travis.yml#L23

@nbiton nbiton temporarily deployed to unee-t-pr-894 February 7, 2020 06:48 Inactive
@nbiton nbiton temporarily deployed to unee-t-pr-894 February 7, 2020 06:49 Inactive
@kaihendry
Copy link
Contributor Author

WIP, don't have time to work on it.

@kaihendry
Copy link
Contributor Author

It appears running tests in the docker environment is pretty tricky.

@nbiton nbiton temporarily deployed to unee-t-pr-894 February 8, 2020 10:31 Inactive
@franck-boullier
Copy link
Member

CI is failing here since it's not building in the more controlled Dockerfile environment

This is actually not accurate AFAICT: when the tests are run on the node js 12 image built by Travis CI then the tests are passing:

image

See https://travis-ci.org/unee-t/frontend/builds/647650880 for an example of this.

There are indeed a few warnings flagged in the tests but apparently nothing fatal.

One solution might be to:
1- get Docker to build an image similar to the image that Travis CI node js 12 builds <-- get a working Docker image.
2- Optimize our code to make sure we are addressing the warning flagged by the build

@kaihendry what do you think of this methodology?

@franck-boullier
Copy link
Member

More information:

  • Docker installs meteor 1.9.
    image

  • Travis CI seems to be using an older (unspecified AFAICT) meteor version
    image

See the latest build: https://travis-ci.org/unee-t/frontend/builds/647920287

@nbiton nbiton temporarily deployed to unee-t-pr-894 February 9, 2020 05:59 Inactive
@nbiton nbiton temporarily deployed to unee-t-pr-894 February 9, 2020 06:07 Inactive
@nbiton nbiton temporarily deployed to unee-t-pr-894 February 9, 2020 06:11 Inactive
@kaihendry
Copy link
Contributor Author

https://travis-ci.org/unee-t/frontend/jobs/647935938?utm_medium=notification&utm_source=github_status has the same node_js as https://travis-ci.org/unee-t/frontend/builds/647650880 and it fails. meteor npm install --save bcrypt is probably something to do it.

@franck-boullier
Copy link
Member

More information:

There was a similar looking issue here: laverdet/node-fibers#409.
Need to check if the fix implemented will also work for us.

@nbiton nbiton temporarily deployed to unee-t-pr-894 February 9, 2020 07:49 Inactive
@kaihendry
Copy link
Contributor Author

kaihendry commented Feb 9, 2020

Spent too much time getting the proprietary travis env working here. I rather defer to #897 and use CodeBuild & focus on solely using the Dockerfile as the build env as before

@kaihendry kaihendry closed this Feb 9, 2020
@kaihendry
Copy link
Contributor Author

The should deploy via https://ap-southeast-1.console.aws.amazon.com/codesuite/codepipeline/pipelines/frontend/view?region=ap-southeast-1

But now the problem here is that the env in the code deploy was not updated since https://github.com/unee-t/frontend/blob/master/buildspec.yml#L36 only updates the image name, not the environment.

The workaround I think is to do a manual ./deploy.sh to refresh/resync the env and create a new task. There might be a automated way of doing it by somehow mapping SSM secrets into the ECS docker-compose, but I am not sure how to do that.

@franck-boullier
Copy link
Member

But now the problem here is that the env in the code deploy was not updated since https://github.com/unee-t/frontend/blob/master/buildspec.yml#L36 only updates the image name, not the environment.

Moving to a logic where the Docker image in seen as a separate entity from the rest of the code as described in Issue #895 might be worth considering to have a clean way of maintaining our codebase...
See this article for a description of the “Docker repo” pattern

@kaihendry
Copy link
Contributor Author

Just ran a ./deploy.sh that created the new task https://ap-southeast-1.console.aws.amazon.com/ecs/home?region=ap-southeast-1#/taskDefinitions/meteor/394 with the updated API_ACCESS_TOKEN.

Changing a secret across the env amongst the frontend, backend and the all the lambdas presents a lot of challenges and it would take quite some work to automate.

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.

3 participants