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

Stop binding ports in the reservable range locally #200

Closed
1 task
sarayourfriend opened this issue Mar 25, 2022 · 11 comments
Closed
1 task

Stop binding ports in the reservable range locally #200

sarayourfriend opened this issue Mar 25, 2022 · 11 comments
Assignees
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: api Related to the Django API 🧱 stack: catalog Related to the catalog and Airflow DAGs 🧱 stack: frontend Related to the Nuxt frontend 🧱 stack: ingestion server Related to the ingestion/data refresh server

Comments

@sarayourfriend
Copy link
Contributor

sarayourfriend commented Mar 25, 2022

Problem

We currently bind ports in the reservable range, except for the API proxy in the frontend repository.

This means that our local development services can clash with system services (not to mention default ports of other projects that are also binding in the reserveable range; how many things do you know that run on 3000 or 8080 or 8000?)

For example SyncThing runs on port 8384 by default which is fine because it's a non-ephemeral system service.

Also there's always the possibility that this will happen again: https://developer.apple.com/forums/thread/682332

And there's nothing wrong with an OEM adding a system service in that range, the range is specifically reserved for such things. We're just lucky that nothing has conflicted with our choices (or the default values) thus far.

Description

There is actually a range of ports available for dynamic and ephemeral services: https://en.wikipedia.org/wiki/List_of_TCP_and_UDP_port_numbers#Dynamic,_private_or_ephemeral_ports

Let's switch our services to use those so that we avoid conflicting with other projects or, indeed, other running applications that could be critical to a developers flow or conflict with a new system service from an OEM or from the dev themselves.

It'd be nice if we just used a solid block of ports so that it's easier to remember where each service is running locally. For example we could do the following, going lowest in the stack to highest, with the main services grouped together and then auxiliary services (Databases, Redis, etc) grouped after those.

  • Airflow -> 65000
  • API -> 65001
  • Nuxt -> 65002
  • Everything else (multiple PG instances, redis, etc) -> 6500{3..}

Alternatively, the other direction as Madison suggested:

  • Nuxt -> 65000
  • API -> 65001
  • Airflow -> 65002
  • Everything else (multiple PG instances, redis, etc) -> 6500{3..}

Implementation

  • 🙋 I would be interested in implementing this feature.
@sarayourfriend sarayourfriend added 🟩 priority: low Low priority and doesn't need to be rushed 🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users labels Mar 25, 2022
@sarayourfriend sarayourfriend changed the title Stop binding low port numbers locally Stop binding ports in the reservable range locally Mar 25, 2022
@AetherUnbound
Copy link
Contributor

I actually think that the reverse might be best, start with frontend -> API -> Airflow. The higher the port, the "further back in the stack". As you mention, databases and other supported services could just be added on incrementally as needed. I like this idea, because I can never remember which port something is on and using an explicitly different port would allow me to bookmark that on localhost as well 🙂

I think that having a monorepo (#192) would make this organization a lot easier, rather than having to cross reference which next port in 3 different repos is available for some new service.

@sarayourfriend
Copy link
Contributor Author

I actually think that the reverse might be best, start with frontend -> API -> Airflow.

👍 I'm not too worried about the specific order so whatever folks on the team most like is what I'd suggest 🙂 It makes no difference to me.

@zackkrida
Copy link
Member

Also there's always the possibility that this will happen again: developer.apple.com/forums/thread/682332

And it has! @dhruvkb identified a new one:

macOS 12.3 adds a service /usr/libexec/remotepairingdeviced that listens on port 49152. Can’t run the Talkback proxy anymore now :sad-panda:

@dhruvkb
Copy link
Member

dhruvkb commented Mar 31, 2022

@AetherUnbound we could reserve about 100 ports for each repo using the first two digits constant, third as a repo number and the last two as arbitrary ports.

501xx - catalog
502xx - API
503xx - frontend

@sarayourfriend sarayourfriend added good first issue New-contributor friendly help wanted Open to participation from the community labels Jun 29, 2022
@sarayourfriend
Copy link
Contributor Author

sarayourfriend commented Jun 29, 2022

I'm adding help wanted and good first issue to this issue because I keep running into this locally and it's quite frustrating! We bind redis to the host in the API on the actual redis port which clashes with any local running redis instances. It's a theoretically straightforward issue so I might pick it up myself in the next couple weeks unless someone else gets to it first. It would be a nice QOL improvement for me personally. I'm sure other folks will run into this as well.

Update: blocked on my other work for the time being so going to try this in the API.

@sarayourfriend sarayourfriend self-assigned this Jun 29, 2022
@sarayourfriend
Copy link
Contributor Author

It seems like there are some service interactions between the ingestion server and the workers (and maybe the API?) that I am not fully aware of. I've got a diff below of my start in the API but going to unassign myself from this.

The diff for API to use 602 port prefix
    diff --git a/.github/workflows/ci_cd.yml b/.github/workflows/ci_cd.yml
    index 5d5ded35..95218f67 100644
    --- a/.github/workflows/ci_cd.yml
    +++ b/.github/workflows/ci_cd.yml
    @@ -144,7 +144,7 @@ jobs:
      - name: Smoke test ReDoc site
        run: |
-          curl --fail 'http://localhost:8000/v1/?format=openapi'
+          curl --fail 'http://localhost:60200/v1/?format=openapi'

      - name: Run API tests
        run: just api-test
diff --git a/api/README.md b/api/README.md
index 74b6f721..79f8970a 100644
--- a/api/README.md
+++ b/api/README.md
@@ -9,7 +9,7 @@ The API has two sets of documentation.
      ```bash
      just sphinx-live
      ```
-    - visiting the `https://localhost:3000/` endpoint
+    - visiting the `https://localhost:60207/` endpoint
  - contain more details on how to contribute to the API project

- [Consumer docs](https://api.openverse.engineering/)
@@ -19,5 +19,5 @@ The API has two sets of documentation.
      ```bash
      just up
      ```
-    - visiting the `https://localhost:8000/` endpoint
+    - visiting the `https://localhost:60200/` endpoint
  - contain more details about the API endpoints with usage examples
diff --git a/api/catalog/settings.py b/api/catalog/settings.py
index 4c915d5f..cffb599c 100644
--- a/api/catalog/settings.py
+++ b/api/catalog/settings.py
@@ -63,7 +63,7 @@ if DEBUG:
SHORT_URL_WHITELIST = {
    "api-dev.openverse.engineering",
    "api.openverse.engineering",
-    "localhost:8000",
+    "localhost:60200",
}
SHORT_URL_PATH_WHITELIST = ["/v1/list", "/v1/images/"]

@@ -186,7 +186,7 @@ CACHES = {
}

# Produce CC-hosted thumbnails dynamically through a proxy.
-THUMBNAIL_PROXY_URL = config("THUMBNAIL_PROXY_URL", default="http://localhost:8222")
+THUMBNAIL_PROXY_URL = config("THUMBNAIL_PROXY_URL", default="http://localhost:60202")

THUMBNAIL_WIDTH_PX = config("THUMBNAIL_WIDTH_PX", cast=int, default=600)
THUMBNAIL_JPG_QUALITY = config("THUMBNAIL_JPG_QUALITY", cast=int, default=80)
diff --git a/api/docs/guides/quickstart.md b/api/docs/guides/quickstart.md
index d277d4cb..45449d2d 100644
--- a/api/docs/guides/quickstart.md
+++ b/api/docs/guides/quickstart.md
@@ -30,7 +30,7 @@ Ensure that you have installed `mkcert` (and the corresponding NSS tools). You c
    just up
    ```

-5. Point your browser to `http://localhost:8000`. You should be able to see the API documentation.
+5. Point your browser to `http://localhost:60200`. You should be able to see the API documentation.
    ![API ReDoc](/_static/api_redoc.png)

6. Load the sample data. This could take a couple of minutes.
diff --git a/api/test/api_live_integration.py b/api/test/api_live_integration.py
index c5b88235..6e78acdd 100644
--- a/api/test/api_live_integration.py
+++ b/api/test/api_live_integration.py
@@ -19,9 +19,9 @@ designed. Run with the `pytest -s` command from this directory.
"""


-API_URL = os.getenv("INTEGRATION_TEST_URL", "http://localhost:8000")
+API_URL = os.getenv("INTEGRATION_TEST_URL", "http://localhost:60200")
known_apis = {
-    "http://localhost:8000": "LOCAL",
+    "http://localhost:60200": "LOCAL",
    "https://api.openverse.engineering": "PRODUCTION",
    "https://api-dev.openverse.engineering": "TESTING",
}
diff --git a/api/test/auth_test.py b/api/test/auth_test.py
index 37aac957..c1ae7207 100644
--- a/api/test/auth_test.py
+++ b/api/test/auth_test.py
@@ -55,7 +55,7 @@ def test_auth_token_exchange(client, test_auth_tokens_registration):
def test_auth_email_verification(client, test_auth_token_exchange):
    # This test needs to cheat by looking in the database, so it will be
    # skipped in non-local environments.
-    if API_URL == "http://localhost:8000":
+    if API_URL == "http://localhost:60200":
        verify = OAuth2Verification.objects.last()
        code = verify.code
        path = reverse("verify-email", args=[code])
diff --git a/api/test/constants.py b/api/test/constants.py
index 7254fe5a..99420667 100644
--- a/api/test/constants.py
+++ b/api/test/constants.py
@@ -1,10 +1,10 @@
import os


-API_URL = os.getenv("INTEGRATION_TEST_URL", "http://localhost:8000")
+API_URL = os.getenv("INTEGRATION_TEST_URL", "http://localhost:60200")

KNOWN_ENVS = {
-    "http://localhost:8000": "LOCAL",
+    "http://localhost:60200": "LOCAL",
    "https://api.openverse.engineering": "PRODUCTION",
    "https://api-dev.openverse.engineering": "TESTING",
}
diff --git a/docker-compose.yml b/docker-compose.yml
index beed92cc..60eb13d5 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -3,7 +3,7 @@ services:
  db:
    image: postgres:13.2-alpine
    ports:
-      - "5432:5432"
+      - "60201:5432"
    env_file:
      - postgres/env.docker
    healthcheck:
@@ -14,7 +14,7 @@ services:
  thumbnails:
    image: ghcr.io/wordpress/openverse-imaginary:latest
    ports:
-      - "8222:8222"
+      - "60202:8222"
    environment:
      PORT: 8222
      MALLOC_ARENA_MAX: 2
@@ -23,7 +23,7 @@ services:
  upstream_db:
    image: postgres:13.2-alpine
    ports:
-      - "5433:5432"
+      - "60203:5432"
    volumes:
      - catalog-postgres:/var/lib/postgresql/data
      - ./sample_data:/sample_data
@@ -35,12 +35,12 @@ services:
  cache:
    image: redis:4.0.10
    ports:
-      - "6379:6379"
+      - "60204:6379"

  es:
    image: docker.elastic.co/elasticsearch/elasticsearch:7.12.0
    ports:
-      - "9200:9200"
+      - "60205:9200"
    environment:
      # disable XPack
      # https://www.elastic.co/guide/en/elasticsearch/reference/5.3/docker.html#_security_note
@@ -72,8 +72,8 @@ services:
    volumes:
      - ./api:/api
    ports:
-      - "8000:8000"
-      - "3000:3000"
+      - "60200:8000"
+      - "60207:3000"
    depends_on:
      - db
      - es
@@ -88,7 +88,7 @@ services:
    image: openverse-ingestion_server
    command: gunicorn -c ./gunicorn.conf.py
    ports:
-      - "8001:8001"
+      - "60208:8001"
    depends_on:
      - db
      - es
@@ -105,7 +105,7 @@ services:
    image: openverse-ingestion_server
    command: gunicorn -c ./gunicorn_worker.conf.py
    ports:
-      - "8002:8002"
+      - "60209:8002"
    depends_on:
      - db
      - es
@@ -119,8 +119,8 @@ services:
  proxy:
    image: nginx:alpine
    ports:
-      - "9080:9080"
-      - "9443:9443"
+      - "60210:9080"
+      - "60211:9443"
    depends_on:
      - web
    volumes:
diff --git a/ingestion_server/README.md b/ingestion_server/README.md
index d59d0bac..f1ad2cea 100644
--- a/ingestion_server/README.md
+++ b/ingestion_server/README.md
@@ -59,7 +59,7 @@ To make cURL requests to the server
```bash
pipenv run \
  curl \
-    --XPOST localhost:8001/task \
+    --XPOST localhost:60208/task \
    -H "Content-Type: application/json" \
    -d '{"model": <model>, "action": <action>}'
```
diff --git a/ingestion_server/test/test_constants.py b/ingestion_server/test/test_constants.py
index fad5615a..a466972f 100644
--- a/ingestion_server/test/test_constants.py
+++ b/ingestion_server/test/test_constants.py
@@ -3,6 +3,6 @@
service_ports = {
    "upstream_db": 65433,  # from 5433
    "db": 65432,  # from 5432
-    "es": 60200,  # from 9200
+    "es": 6020,  # from 9200
    "ingestion_server": 60001,  # from 8001
}
diff --git a/justfile b/justfile
index 24d31b26..b7afc09e 100644
--- a/justfile
+++ b/justfile
@@ -107,14 +107,14 @@ cert:
    -curl -s -o /dev/null -w '%{http_code}' 'http://{{ es_host }}/_cluster/health?pretty'

# Wait for Elasticsearch to be healthy
-@wait-for-es es_host="localhost:9200":
+@wait-for-es es_host="localhost:60205":
    just _loop \
    '"$(just es-health {{ es_host }})" != "200"' \
    "Waiting for Elasticsearch to be healthy..."

# Check if the media is indexed in Elasticsearch
@check-index index="image":
-    -curl -sb -H "Accept:application/json" "http://localhost:9200/_cat/aliases/{{ index }}" | grep -c "{{ index }}-"
+    -curl -sb -H "Accept:application/json" "http://localhost:60205/_cat/aliases/{{ index }}" | grep -c "{{ index }}-"

# Wait for the media to be indexed in Elasticsearch
@wait-for-index index="image":
@@ -132,7 +132,7 @@ _ing-install:
    cd ingestion_server && pipenv install --dev

# Perform the given action on the given model by invoking the ingestion-server API
-_ing-api model action port="8001":
+_ing-api model action port="60208":
    curl \
      -X POST \
      -H 'Content-Type: application/json' \
@@ -144,7 +144,7 @@ _ing-api model action port="8001":
    -curl -s -o /dev/null -w '%{http_code}' 'http://{{ ing_host }}/'

# Wait for the ingestion-server to be healthy
-@wait-for-ing ing_host="localhost:8001":
+@wait-for-ing ing_host="localhost:60208":
    just _loop \
    '"$(just ing-health {{ ing_host }})" != "200"' \
    "Waiting for the ingestion-server to be healthy..."
@@ -172,7 +172,7 @@ _api-install:

# Check the health of the API
@web-health:
-    -curl -s -o /dev/null -w '%{http_code}' 'http://localhost:8000/healthcheck'
+    -curl -s -o /dev/null -w '%{http_code}' 'http://localhost:60200/healthcheck'

# Wait for the API to be healthy
@wait-for-web:
@@ -201,7 +201,7 @@ dj-local +args:

# Make a test cURL request to the API
stats media="images":
-    curl "http://localhost:8000/v1/{{ media }}/stats/"
+    curl "http://localhost:60200/v1/{{ media }}/stats/"

# Get Django shell with IPython
ipython:

@dhruvkb
Copy link
Member

dhruvkb commented Sep 25, 2022

Seems like binding to port 50292 in GitHub Actions is flaky. I've perceived an increase in CI failures (that go away on re-run) since WordPress/openverse-api#844 was merged.

@sarayourfriend
Copy link
Contributor Author

@dhruvkb Do you have an example of the flaky runs? Are they failing specifically on binding to that port only, do you think if we just changed that one it could address the flakiness?

@dhruvkb
Copy link
Member

dhruvkb commented Sep 26, 2022

It takes a little effort to go through the runs and find occurrences, because when they occurred I usually re-ran them and they passed. Nonetheless here are a couple of examples:

It's confusing why the error would be

Error starting userland proxy: listen tcp4 0.0.0.0:502xx: bind: address already in use

and if that is indeed the case, how it would go away on a re-run.

@sarayourfriend
Copy link
Contributor Author

That is strange. Let's change it to another port and see if it helps... but maybe there's actually some kind of race condition we introduced at some point that is causing multiple of the same docker service to get spun up?

dhruvkb pushed a commit that referenced this issue Feb 20, 2023
…jango-3.2.7

Bump django from 3.2.6 to 3.2.7 in /openverse-api
@obulat obulat added 🧱 stack: frontend Related to the Nuxt frontend 🧱 stack: backend labels Feb 24, 2023
@AetherUnbound AetherUnbound added 🧱 stack: api Related to the Django API 🧱 stack: ingestion server Related to the ingestion/data refresh server 🧱 stack: catalog Related to the catalog and Airflow DAGs and removed 🧱 stack: backend labels May 15, 2023
@sarayourfriend
Copy link
Contributor Author

We haven't seen this flakiness anymore recently, I believe, so I'm going to close this issue along with #1035.

@sarayourfriend sarayourfriend closed this as not planned Won't fix, can't repro, duplicate, stale Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: api Related to the Django API 🧱 stack: catalog Related to the catalog and Airflow DAGs 🧱 stack: frontend Related to the Nuxt frontend 🧱 stack: ingestion server Related to the ingestion/data refresh server
Projects
Archived in project
Development

No branches or pull requests

5 participants