Skip to content

Add shared Makefile used by all components#3

Open
suokko wants to merge 1 commit into
online-bridge-hackathon:masterfrom
suokko:shared_makefile
Open

Add shared Makefile used by all components#3
suokko wants to merge 1 commit into
online-bridge-hackathon:masterfrom
suokko:shared_makefile

Conversation

@suokko
Copy link
Copy Markdown
Contributor

@suokko suokko commented Jul 19, 2020

A shared Makefile aims to remove duplicated image build and deployment
targets from each component. Avoiding duplicate Makefile targets makes
it easier to maintain a working deployment.


I created an example branch how to use the shared makefile in DDS service. I also pushed an annotated tag which makes git describe output more than just a commit hash for version.

https://github.com/suokko/DDS-1/tree/shared_makefile

@suokko suokko requested review from kiat-huang and tameware July 19, 2020 10:32
Copy link
Copy Markdown
Contributor

@tameware tameware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love it! I'm a big fan of DRY.

I have not tested this. I don't know whether our existing deploy process is working - that makes me less inclined to try a new one.

Comment thread make/Makefile
release: push

.dockerignore: .gitignore
sed 's#^[^/]#**/\0#' < $< > $@
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment as to what's going on here.

A shared Makefile aims to remove duplicated image build and deployment
targets from each component. Avoiding duplicate Makefile targets makes
it easier to maintain a working deployment.

Signed-off-by: Pauli <suokkos@gmail.com>
@suokko suokko force-pushed the shared_makefile branch from 8046cb9 to 3bbff8a Compare July 19, 2020 14:31
@suokko
Copy link
Copy Markdown
Contributor Author

suokko commented Jul 19, 2020

I love it! I'm a big fan of DRY.

I have not tested this. I don't know whether our existing deploy process is working - that makes me less inclined to try a new one.

This is based on @kiat-huang 's changes which managed to deploy to production. I don't know if my deployment code works because docker push returns Access denied.

I added some comments to the Makefile.

@tameware
Copy link
Copy Markdown
Contributor

@kiat, let's get @suokko and all the knaves the access we'll need to deploy. Let me know if I can help test this. I'd hate to push a new deploy procedure without a demonstration that it works.

@suokko
Copy link
Copy Markdown
Contributor Author

suokko commented Jul 19, 2020

I create a personal cluster. My cluster isn't exact copy of production because terraform failed with a permission error. I have no idea how terraform can fail when it should be using service account with same privileges as I have.

I used Makefile to deploy dds (make deploy GCP_PROJECT=deal-test-project GKE_ZONE=europe-west4-a GKE_CLUSTER_NAME=bridge-cluster). After I manually exposes the deployment I managed to solve the example deal using curl. (My cluster doesn't have correct load balancer set up for automatically exposing application)

$ curl --header "Content-Type: application/json" --request POST --data '{"hands":{"S":["D3", "C6", "DT", "D8", "DJ", "D6", "CA", "C3", "S2", "C2", "C4", "S9", "S7"],"W":["DA", "S4", "HT", "C5", "D4", "D7", "S6", "S3", "DK", "CT", "D2", "SK","H8"],"N":["C7", "H6", "H7", "H9", "CJ", "SA", "S8", "SQ", "D5", "S5", "HK", "C8", "HA"],"E":["H2", "H5", "CQ", "D9", "H4", "ST", "HQ", "SJ", "HJ", "DQ", "H3", "C9", "CK"]}}' http://34.90.253.176/api/dds-table/
{"S": {"N": 8, "E": 5, "S": 7, "W": 5}, "H": {"N": 6, "E": 6, "S": 6, "W": 7}, "D": {"N": 7, "E": 6, "S": 7, "W": 6}, "C": {"N": 9, "E": 4, "S": 8, "W": 4}, "N": {"N": 7, "E": 6, "S": 6, "W": 6}}

To me it seems like deployment mostly works.

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.

2 participants