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

Add Portainer integration #129438

Open
wants to merge 27 commits into
base: dev
Choose a base branch
from
Open

Conversation

Thomas55555
Copy link
Contributor

@Thomas55555 Thomas55555 commented Oct 29, 2024

Breaking change

Proposed change

Add Portainer integration.
In the first step it is possible to watch the state of the containers in your portainer instance.
In a second step we can add buttons to start, stop or restart the containers.
This integration uses the aiotainer library: https://github.com/Thomas55555/aiotainer

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • 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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@kbickar
Copy link
Contributor

kbickar commented Nov 4, 2024

Tested this out a bit and have some notes:

  • The Host field asks for a hostname or ip during setup but it actually requires a URL. Suggest adding "https://" if it doesn't start with that
  • Given the above requires a URL, it would be better to use that with the port rather than separate the port field. I have mine setup with a proxy so I access it with https://home/portainer but that's not possible as the port is put on the end making it https://home/portainer:80 instead of https://home/portainer:80
  • The parsing of the entities return value requires all fields to exist when they might not. Mine did not have EnableGPUManagement in the json response (maybe because there are no GPUs?) and so the parsing of the NodeData failed. Looking at the code, this and a few others have the omitempty attribute meaning they may not be present in the response: https://github.com/portainer/portainer/blob/develop/api/portainer.go

@Thomas55555
Copy link
Contributor Author

Tested this out a bit and have some notes:

  • The Host field asks for a hostname or ip during setup but it actually requires a URL. Suggest adding "https://" if it doesn't start with that

We now add http:// if port 9000 is given and we add https:// if port is 9443. If no port is given we raise an Exception.

  • Given the above requires a URL, it would be better to use that with the port rather than separate the port field. I have mine setup with a proxy so I access it with https://home/portainer but that's not possible as the port is put on the end making it https://home/portainer:80 instead of https://home/portainer:80

I also thought about it, but maybe for some people it's to difficult.
Another question. You mentioned port 80. The default port for http-connections is 9000. Have you tried this.
For me it's not possible to combine https and port 9000

https://home/portainer:80 instead of https://home/portainer:80

What do you mean exactly? https://home:80/portainer
Than you're right and it would be a problem.

  • The parsing of the entities return value requires all fields to exist when they might not. Mine did not have EnableGPUManagement in the json response (maybe because there are no GPUs?) and so the parsing of the NodeData failed. Looking at the code, this and a few others have the omitempty attribute meaning they may not be present in the response: https://github.com/portainer/portainer/blob/develop/api/portainer.go

Solved

@kbickar
Copy link
Contributor

kbickar commented Nov 13, 2024

I have it setup so navigating to https://home/portainer loads portainer (which is actually being hosted on 9443, but the redirect obscures that). Given the port is configurable, the integration shouldn't make assumptions about the port (I would add https:// if there's no protocol unless it's specifically 9000).

If all that's needed is a URL then just have the configuration be a URL. People that use portainer already know how to access it via the URL so that should be easily available and less complicated than splitting it into hostname (with a protocol) and port

@kbickar
Copy link
Contributor

kbickar commented Nov 24, 2024

During startup it gives a new error:

    await api.get_status()
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/aiotainer/client.py", line 87, in get_status
    self.data = portainer_list_to_dictionary(portainer_list)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/aiotainer/utils.py", line 13, in portainer_list_to_dictionary
    NodeData.from_dict(portainer).id: NodeData.from_dict(portainer)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 74, in __mashumaro_from_dict__
mashumaro.exceptions.MissingField: Field "is_edge_device" of type bool is missing in NodeData instance```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants