Skip to content

Adding a read API endpoint to get container states similar to GetDeploymentsSummary but for all containers that Komodo can see.#386

Merged
mbecker20 merged 7 commits intomoghtech:1.17.1from
alexshore:main
Mar 29, 2025
Merged

Conversation

@alexshore
Copy link
Contributor

@alexshore alexshore commented Mar 28, 2025

I wanted to create a custom homepage widget for Komodo so I could just see what was running/stopped and a total count but an endpoint like GetDeploymentsSummary and GetStacksSummary didn't exist for all containers.

At first, I just made a separate python API (https://github.com/alexshore/komodo-containers-api) but thought this could be a good way to up my contributions to open source software so heres my attempt at contributing a GetDockerContainersSummary endpoint :)

I'm not too fussed as what counts as what category. For my use-case it's helpful for there really to just be 2 final "states" that the docker containers are categorised into (i.e. just Stopped and Running) but thats a bit more specific to my use-case than to Komodo so right now they're split into Stopped, Running, Unknown and Unhealthy but that can be changed.

I also wasn't sure where in the file to put the new code so I just put it all next to ListAllDockerContainers as it performs a similar function.

Hope this is all good and doesn't need too many more changes! Thanks for your help in discord too :)

@alexshore alexshore changed the base branch from main to 1.17.1 March 28, 2025 20:24
Copy link
Member

@mbecker20 mbecker20 left a comment

Choose a reason for hiding this comment

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

Just 2 nitpicks. Also, this api is not being used by frontend, do you have any ideas for that part?

#[derive(Serialize, Deserialize, Debug, Clone, Default)]
pub struct GetDockerContainersSummaryResponse {
/// The total number of Containers
pub total: I64,
Copy link
Member

@mbecker20 mbecker20 Mar 29, 2025

Choose a reason for hiding this comment

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

Use u32 as the number type, in line with the other summary responses eg https://docs.rs/komodo_client/latest/komodo_client/api/read/struct.GetStacksSummaryResponse.html

Copy link
Member

Choose a reason for hiding this comment

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

I actually realized some are using u32, some I64, I think I will change them all to u32

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I just used I64 as thats what GetDeploymentsSummary was using

fixed in 73f7e89

&[],
)
.await
.context("failed to get containers from db")?;
Copy link
Member

Choose a reason for hiding this comment

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

.context("failed to get servers from db")?;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 9db8273

@alexshore
Copy link
Contributor Author

alexshore commented Mar 29, 2025

for the frontend I don't really have any idea, currently on the dashboard it shows servers, deployments, and stacks, so it doesn't really make much sense to show containers too.

i only really wanted it so an external service could access it, but if we added a server filter param like in ListDockerContainers or ListAllDockerContainers then you could just have a summary at the top of each server page in the frontend i guess? often you'll have quite a lot of containers running I think so could be good to see statuses directly rather than having to scroll a long list

i know almost nothing about frontend though so I wouldnt even know where to start in adding something like that

@mbecker20 mbecker20 merged commit 6125331 into moghtech:1.17.1 Mar 29, 2025
@mbecker20 mbecker20 mentioned this pull request Mar 29, 2025
mbecker20 pushed a commit that referenced this pull request Apr 2, 2025
* added GetDockerContainersSummary endpoint in rust api

* typescript stuff

* more autogenned typescript stuff

* fixed comments to be in line with actual behaviour

* fixed ReadResponse for GetDockerContainersSummary

* I64 -> u32 for response types

* more accurate error context
mbecker20 added a commit that referenced this pull request Apr 14, 2025
* interpolate into slack / discord url

* fix js client docs

* js client should be type: module

* click table tags to toggle tag filter

* git token helper early return when empty provider

* reorder Stack fields

* action support interpolation doc

* Fix for the {account} login fails when the account name contains '$' (#385)

* GetDeploymentsSummary (#386)

* added GetDockerContainersSummary endpoint in rust api

* typescript stuff

* more autogenned typescript stuff

* fixed comments to be in line with actual behaviour

* fixed ReadResponse for GetDockerContainersSummary

* I64 -> u32 for response types

* more accurate error context

* backend for build files on host / ui defined

* core api supports non repo based build

* Ntfy as Alerter (#404)

* add ntfy alerter

* add ntfy alerter

* add ntfy alerter

---------

Co-authored-by: GFXSpeed <github@schafhaupt.com>

* Improve docs around running periphery in a container (#402)

* Add ports that need to be exposed if periphery is remote from core

* Spelling: overide -> override

* Add info about using a custom config file when running periphery in docker

* clean up ntfy alerter

* clean up ResourceSyncConfig

* update build cache after create / update

* refresh stack cache log

* Build UI Defined / file on host frontend

* update clap + rustls

* don't cleanup build files

* clean up dockerfile full path

* update BuildListItemInfo + UI table

* add Other Resources page

* add 5 second ws reconnection timeout

* Make listener address configurable & Add support for IPv6 (#405)

* Make listener address configurable

* Make listener address configurable for periphery

* rename listener_address -> bind_ip

---------

Co-authored-by: Jacky Fong <hello@huzky.dev>
Co-authored-by: Alex Shore <kntrllr@gmail.com>
Co-authored-by: Niklas <108990063+GFXSpeed@users.noreply.github.com>
Co-authored-by: GFXSpeed <github@schafhaupt.com>
Co-authored-by: theRAAPster <theraapster@gmail.com>
Co-authored-by: Daniel Vos <45820840+vosdev@users.noreply.github.com>
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