adopt playground containers #200

feloy · 2024-01-30T16:21:38Z

This PR adds a new class PodmanConnection, whose instance listens for new/updated 'podman' container providers, so other managers can get informed of new containers and adopt some of them depending on their labels.

It is used as part of this PR for adopting playground containers.

It is expected to be used to detect and adopt recipe pods in a future PR.

Fixes #57

packages/backend/src/managers/playground.ts

feloy · 2024-01-30T16:23:44Z

packages/backend/src/managers/playground.ts

@@ -56,6 +59,33 @@ export class PlayGroundManager {
    this.queries = new Map<number, QueryState>();
  }

+  async adoptRunningPlaygrounds() {
+    const containers = await containerEngine.listContainers();


@benoitf this call returns an empty list when the extension is loaded at startup (and the correct list of containers when the extension is reloaded, after podman desktop has finished initialization).

What would be the right way to wait for the container engine to be up and running?

We need to materialize the connection from AI studio to podman machine at a central point I'm pretty sure there is some code that try to get this connection

I'm finally using provider.onDidRegisterContainerConnection to get informed.

lstocchi · 2024-01-31T09:56:54Z

packages/backend/src/managers/playground.ts

@@ -60,6 +64,43 @@ export class PlayGroundManager {
    this.queries = new Map<number, QueryState>();
  }

+  async adoptRunningPlaygrounds() {
+    provider.onDidRegisterContainerConnection(async (e: RegisterContainerConnectionEvent) => {


Isn't the register method only called when you create a new connection? (e.g when you create a new podman machine?) Could we use the onDidUpdateContainerConnection so it also listens to machines that were stopped and now get started?

The moment I'm trying to detect is when podman desktop starts and initializes the extensions. At this moment, the podman extension will register itself as container provider, and this is what I want to be informed.
I tried onDidUpdateContainerConnection but it is not triggered at startup.

I think what we need is a ConnectionProvider within AI Studio. As startup, it should get the first available Podman connection and then listen for connection changes. Every other piece of AI Studio should use this connection provider to get a Podman connection.

Please look at my latest commit, it should do what you describe

jeffmaury · 2024-01-31T13:28:50Z

packages/backend/src/managers/podmanConnection.ts

+  init(): void {
+    // In case the extension has not yet registered, we listen for new registrations
+    // and retain the first started podman provider
+    const disposable = provider.onDidRegisterContainerConnection((e: RegisterContainerConnectionEvent) => {


Why are we listening on connection registration andnot connection start/stop events ?

For this feature, I only want to list containers started before Podman Desktop is started, and I want to be informed when the podman extension is registered (or the listContainers would return an empty list if we try to retrieve the list before).

(note that there is a registries/ContainerRegistry which is watching at events on a specific container, which we may merge with this class at some point)

I didn't test it so maybe i'm wrong but looking at the code it looks to me that it initializes the connection when the studio is activated if there is atleast one podman machine that is started and then it starts listening to if a machine gets registered.
So if i start my desktop having my podman machine stopped, the connection is not set as there is no podman machine running. If i start my podman machine in a second moment it does not trigger its initialization bc it only listen to machine registration.
So if i start the playground or any other feature that requires to retrieve the active connection it fails, no? But i have my podman machine running fine, i just started it after my desktop was activated

Yes, you are probably right, I'll look at supporting this scenario too. Thanks

Finally, I prefer for now to remove this exposure and use of connection from the PodmcnConnection class, as it is not part of what is needed to fix the targeted issue, which is only related to getting a connection to the provider at startup.

I'll work at this refactoring as part of another PR, if necessary.

Here is a demo on Linux to see what you can test:

adopt-playground-2024-02-02_10.29.49.mp4

My other scenario is, you are using the playground, turn off the machine bc you switch to another or whatever. ai-studio should be able to remove the adopted container as it is stopped now, then you turn the machine on again, the container is there stopped, yo start it, ai-studio should be able to adopt it again.

For now, we are not checking when a machine is stopped, to mark playground as stopped. It would be an extension of the PR #194 (and not particularly part of this PR as it is a different logic).

As said in my previous message, when you stop a playground container, it is also deleted (because marked as AUTOREMOVE). So you will never have a playground container (not even stopped) when you start a podman machine.

If you stop the machine the container will be there, not deleted and you can restart it once you restart the machine.

AFAIK, it is not the expected behaviour for a container marked as auto-removable. In Inspect of this container, is it marked as this?

"HostConfig": { [...] "AutoRemove": true,

I'm doing the same test in my Windows machine, and the container is not present anymore

Not that I don't want to support these stopped containers, but if we consider having stopped playground containers, this adds a third state for the playground container in the UI (paused in addition to not-running and running), which we would also have to manage in the UI (set the playground in pause, etc). That would be a bigger change than just detecting a stopped container is started again.

Sorry for the slow response. Done it again now and i confirm that the Autoremove flag is there

Co-authored-by: axel7083 <[email protected]>

…are up

…vider

lstocchi

Tested and works fine for this specific use case

fix up renaming of IMAGE Constants.

feloy requested review from benoitf, jeffmaury, axel7083 and lstocchi January 30, 2024 16:21

axel7083 reviewed Jan 30, 2024

View reviewed changes

packages/backend/src/managers/playground.ts Outdated Show resolved Hide resolved

feloy commented Jan 30, 2024

View reviewed changes

feloy force-pushed the feat-57/adopt-playground-containers branch 3 times, most recently from 2e4db4a to 44c74bb Compare January 31, 2024 06:48

lstocchi reviewed Jan 31, 2024

View reviewed changes

feloy changed the title ~~adopt playground containers~~ [wip] adopt playground containers Jan 31, 2024

feloy force-pushed the feat-57/adopt-playground-containers branch from 5a75fda to 76beff3 Compare January 31, 2024 13:24

jeffmaury reviewed Jan 31, 2024

View reviewed changes

feloy force-pushed the feat-57/adopt-playground-containers branch from 76beff3 to 2ebd657 Compare January 31, 2024 13:32

feloy and others added 6 commits January 31, 2024 14:41

adopt playground containers

6b7db38

Update packages/backend/src/managers/playground.ts

498b4a7

Co-authored-by: axel7083 <[email protected]>

fix unit tests

458673e

use provider.onDidRegisterContainerConnection to know when providers …

ddfd4b3

…are up

start adopt when podman only provider starts

1e1834c

create a dedicated class for listening the registration of podman pro…

286cef4

…vider

feloy force-pushed the feat-57/adopt-playground-containers branch from 2ebd657 to 286cef4 Compare January 31, 2024 13:41

feloy changed the title ~~[wip] adopt playground containers~~ adopt playground containers Jan 31, 2024

remove the getConnection method from PodmanConnection

fdbb930

feloy requested a review from lstocchi February 1, 2024 08:22

lstocchi approved these changes Feb 2, 2024

View reviewed changes

feloy merged commit 10d4b0a into main Feb 2, 2024
3 checks passed

feloy deleted the feat-57/adopt-playground-containers branch February 2, 2024 12:47

mhdawson pushed a commit to mhdawson/podman-desktop-extension-ai-lab that referenced this pull request Nov 22, 2024

Merge pull request containers#200 from rhatdan/codegen

056c2db

fix up renaming of IMAGE Constants.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adopt playground containers #200

adopt playground containers #200

feloy commented Jan 30, 2024 •

edited

Loading

feloy Jan 30, 2024

jeffmaury Jan 31, 2024

feloy Jan 31, 2024

lstocchi Jan 31, 2024

feloy Jan 31, 2024

jeffmaury Jan 31, 2024

feloy Jan 31, 2024

jeffmaury Jan 31, 2024

feloy Jan 31, 2024

lstocchi Jan 31, 2024

feloy Jan 31, 2024

feloy Feb 1, 2024

feloy Feb 2, 2024

lstocchi Feb 2, 2024

feloy Feb 2, 2024 •

edited

Loading

feloy Feb 2, 2024

lstocchi Feb 2, 2024

lstocchi left a comment

adopt playground containers #200

adopt playground containers #200

Conversation

feloy commented Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feloy Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lstocchi left a comment

Choose a reason for hiding this comment

feloy commented Jan 30, 2024 •

edited

Loading

feloy Feb 2, 2024 •

edited

Loading