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

Check/get ingress port on add-on load #4744

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

agners
Copy link
Member

@agners agners commented Dec 6, 2023

Proposed change

Instead of setting the ingress port on install, make sure to set the port when the add-on gets loaded (on Supervisor startup and before installation). This is necessary since a dynamic ingress port is not stored as part of the add-on data storage themself but in the ingress data store. So on every Supervisor start the port needs to be transferred to the add-on model.

Note that we still need to check the port on add-on update since the add-on potentially added (dynamic) ingress on update. Same applies to add-on restore (the restored version might use a dynamic ingress port).

This addresses the following issue seen on the dev channel:

23-12-06 10:11:36 ERROR (MainThread) [aiohttp.server] Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/aiohttp/web_protocol.py", line 452, in _handle_request
    resp = await request_handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/aiohttp/web_app.py", line 543, in _handle
    resp = await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/aiohttp/web_middlewares.py", line 114, in impl
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/supervisor/supervisor/api/middleware/security.py", line 186, in block_bad_requests
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/supervisor/supervisor/api/middleware/security.py", line 202, in system_validation
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/supervisor/supervisor/api/middleware/security.py", line 269, in token_validation
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/supervisor/supervisor/api/middleware/security.py", line 280, in core_proxy
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/supervisor/supervisor/api/utils.py", line 62, in wrap_api
    answer = await method(api, *args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/supervisor/supervisor/api/__init__.py", line 456, in addons_addon_info
    return await api_addons.info(request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/supervisor/supervisor/api/addons.py", line 253, in info
    ATTR_INGRESS_PORT: addon.ingress_port,
                       ^^^^^^^^^^^^^^^^^^
  File "/usr/src/supervisor/supervisor/addons/addon.py", line 399, in ingress_port
    raise RuntimeError(f"No port set for add-on {self.slug}")
RuntimeError: No port set for add-on 8675bb79_rt_test

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to the supervisor)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast supervisor tests)
  • Tests have been added to verify that the new code works.

If API endpoints of add-on configuration are added/changed:

Instead of setting the ingress port on install, make sure to set
the port when the add-on gets loaded (on Supervisor startup and
before installation). This is necessary since the dynamic ingress
ports are not stored as part of the add-on data storage themself
but in the ingress data store. So on every Supervisor start the
port needs to be transferred to the add-on model.

Note that we still need to check the port on add-on update since
the add-on potentially added (dynamic) ingress on update. Same
applies to add-on restore (the restored version might use a dynamic
ingress port).
@agners agners added the bugfix A bug fix label Dec 6, 2023
@pvizeli pvizeli merged commit 96f4ba5 into main Dec 6, 2023
@pvizeli pvizeli deleted the fix-dynamic-ingress-port-on-su-load branch December 6, 2023 09:46
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants