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

Feedback on the project #53

Open
denstorti opened this issue Aug 19, 2020 · 1 comment
Open

Feedback on the project #53

denstorti opened this issue Aug 19, 2020 · 1 comment

Comments

@denstorti
Copy link
Member

denstorti commented Aug 19, 2020

2020-jun-project1-group1

Documentation:

  • README Great stuff 👍👍👍:
    • Nice presentation with slides!
    • Badges
    • Roadmap
    • Contributing --> CONTRIBUTING file?
    • License file
    • Nice emojis in the kick and run flow 👍
    • terraform-docs :)
  • Solution diagram:
    • Good ASG scaling policy
    • Good: containerInsights enabled
    • Easy to understand and visualize
      • (TBD) "Wordpress instance" should be "Wordpress task"
      • (TBD) Aurora DB in a single AZ? No Multi-AZ?
      • (TBD) EFS is regional, move it in
      • (TBD) ECR not in the diagram
      • (TBD) Add cloudwatch to the diagram
      • (TBD) Container logs in CW
  • (TBD) - EFS not encrypted
  • To be fixed: make kick-n-run did not work. sed failed for AWS_DEFAULT_REGION.
  • (TBD - minor) Colors failing for make wait-lb .
  • (TBD) - A record with LB alias not added to ROUTE 53
  • (TBD) Make targets are tech specific tf-ci-plan --> consider using agnostics (better for semantics)
 === 🔐 Login to ECR docker registry: \e[32m835575113948.dkr.ecr.region=ap-southeast-2.amazonaws.com\e[0m

Error response from daemon: Get https://835575113948.dkr.ecr.region=ap-southeast-2.amazonaws.com/v2/: Service Unavailable
make[1]: *** [ecr-login] Error 1
make: *** [kick-n-run] Error 2
  • (TBD) two Makefile (pone in the root and other in ./docker) duplicating same variables. Error above needs to be fixed in both places.

  • Error while deploying ECS service (I needed to comment out ECS service tags)

InvalidParameterException: The new ARN and resource ID format must be enabled to add tags to the service. Opt in to the new format and try again. "devops-wordpress-ecs_service"

Project board & collab

  • Tidy, tasks updated, PRs linked to issues 👍
  • Good call outs in the docs about WIP and to be improved.
  • Diagram versioned as draw.io for collaboration. Really good!

Great work, you did it! 🚀 :)

@alanlima
Copy link
Contributor

Thanks guys for the feedback. I really appreciate it, more changes to improve the code base from that 😁

We will address those fixes.

Just a quick question regards the failing in the sed, was it because your config file doesn't have space between =?

About the error in ECS Service, we had that with new AWS accounts which has this setting disabled by default. I will add the script to enable it in the kick-n-run.

Again, thanks @kikobr82 @caiocezart @denstorti for this opportunity and your constructive feedback 😄😄😄

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

No branches or pull requests

2 participants