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.

Sign up for GitHub

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

Jump to bottom

docs(manifest): add gotchas to template for multiple lb web services #1377

Closed
wants to merge 3 commits into from
Closed

docs(manifest): add gotchas to template for multiple lb web services #1377

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f49ee1b
feat(manifest): Add notes about gotchas - template
GavinRay97 Sep 12, 2020
c3f863a
Update templates/workloads/services/lb-web/manifest.yml
GavinRay97 Sep 13, 2020
70a5b93
Remove note about valid CPU/Mem pairs per suggestion
GavinRay97 Sep 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions templates/workloads/services/lb-web/manifest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,17 @@ image:
http:
# Requests to this path will be forwarded to your service.
# To match all requests you can use the "/" path.
# NOTE: If you plan to deploy multiple services, this value must be unique per-service
# NOTE: It is unnecessary to start the path with a "/", for example "/api" versus "api"
path: '{{.Path}}'
GavinRay97 marked this conversation as resolved.
Show resolved Hide resolved
# You can specify a custom health check path. The default is "/"
# WARNING: Passing healthchecks are required and are how ECS measures task health. Failure to pass healthcheck may result cyclical restarts.
# NOTE: The healthcheck URL should be appended to the "path" value.
# For example, if "path" is set to "api", and your API serves it's healthcheck on "/healthz", this value should be "/api/healthz"
# healthcheck: '{{.HealthCheckPath}}'
Comment on lines +21 to 24
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# WARNING: Passing healthchecks are required and are how ECS measures task health. Failure to pass healthcheck may result cyclical restarts.
# NOTE: The healthcheck URL should be appended to the "path" value.
# For example, if "path" is set to "api", and your API serves it's healthcheck on "/healthz", this value should be "/api/healthz"
# healthcheck: '{{.HealthCheckPath}}'
# healthcheck: '{{.HealthCheckPath}}' {{if ne .Path "/"}} # Must be a path under '{{.Path}}'. For example: '{{.Path}}/_healthcheck' {{end}}

Copy link
Author

@GavinRay97 GavinRay97 Sep 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I double checked on this one -- for one service, it threw errors that the prefixed path wasn't found.

Think this one has a weird interplay with the Load Balancer. For example, with Hasura:
https://github.com/hasura/graphql-engine

It seems that if you put this at a subpath, path: "hasura", you cannot load any endpoint after it being deployed with Copilot because the internal application router is broken.

Because the root path already had /hasura in it, when I tried to visit the route /console at /hasura/console, it throw nonexistent path error from the Hasura service, and then in the Fargate logs I could see it wasn't stripping the /hasura prefix from the route:

image

See http_info.url here:

{
  "type": "http-log",
  "timestamp": "2020-09-13T18:55:04.819+0000",
  "level": "error",
  "detail": {
    "operation": {
      "error": {
        "path": "$",
        "error": "resource does not exist",
        "code": "not-found"
      },
      "request_id": "105a938a-82b6-4b62-83bc-594ddaa2fc73",
      "response_size": 65,
      "raw_query": ""
    },
    "http_info": {
      "status": 404,
      "http_version": "HTTP/1.1",
      "url": "/hasura/console",
      "ip": "73.179.121.23",
      "method": "GET",
      "content_encoding": null
    }
  }
}

In Hasura's case, the Healthcheck endpoint lives at /healthz, but giving /hasura/healthz as a path to Copilot results in failure, and giving /healthz cannot work because it'll never hit the /hasura path and be routed 😬

image

This kind of effectively renders the web service useless since none of the paths will route properly.

(Unrelated: Would attaching a domain fix this?)

Might be better for someone from your team who understands this more to weigh in here, I think I may have been mistaken on this one, sorry.

Copy link
Contributor

Choose a reason for hiding this comment

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

Heya!

I double-checked this and the healthcheck path should still be the path in the service code (just /healthz and not /hasura/healthz). I believe this is because it's a property of the Target Group so you don't need to prefix it with the rule path ("hasura").

Out of curiosity: Does running the container locally work? My suspicion is that the container fails to come up so it never gets to the point where it can query the "/healthz" path.

Copy link
Author

@GavinRay97 GavinRay97 Sep 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm 🤔

The hasura instance does run locally, and it was running on the ELB there (that error format with path: $, error: xxx is Hasura's web server response) but because of the LB having it at /hasura, when Hasura router handled the requests it was trying to send them to wrong place.

For example:

Load Balanced Route | Desired App Path | Routed App Path               
----------------------------------------------------------
/hasura/console     |     /console     | /hasura/console

So basically because the app webserver router is parsing /hasura/console and not stripping /hasura, it won't route properly.

healthcheck path should still be the path in the service code (just /healthz and not /hasura/healthz). I believe this is because it's a property of the Target Group so you don't need to prefix it with the rule path ("hasura").

This makes sense -- so it is not possible to check the healthcheck path from outside of the running container then right?

If you want to test this really quick, you can make a Dockerfile like below:

FROM hasura/graphql-engine:v1.3.1

And this manifest + try deploying it (it needs HASURA_GRAPHQL_DATABASE_URL env connected to Postgres):

name: hasura
type: Load Balanced Web Service
image:
  # Assuming Dockerfile in folder /hasura from project root
  build: hasura/Dockerfile
  port: 8080

http:
  path: "hasura"
  healthcheck: "/healthz"

cpu: 512
memory: 1024
count: 1

variables:
  HASURA_GRAPHQL_ENABLE_CONSOLE: "true"
  HASURA_GRAPHQL_DEV_MODE: "true"
  # Set Postgres RDS (or any other Postgres) credentials here
  HASURA_GRAPHQL_DATABASE_URL: postgres://<RDS DB Username>:<RDS DB Password>@xxxxxxxxxx.us-east-1.rds.amazonaws.com/<DB name>
  HASURA_GRAPHQL_ENABLED_LOG_TYPES: startup, http-log, webhook-log, websocket-log, query-log


# NOTE: Not all values for CPU + Memory are valid, refer to below link for valid value pairs:
# https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-cpu-memory-error.html
GavinRay97 marked this conversation as resolved.
Show resolved Hide resolved
# Number of CPU units for the task.
cpu: {{.CPU}}
# Amount of memory in MiB used by the task.
Expand Down