Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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 option to instantly go to new container? #3036

Closed
YouveGotMeowxy opened this issue Jun 15, 2024 · 26 comments
Closed

Add option to instantly go to new container? #3036

YouveGotMeowxy opened this issue Jun 15, 2024 · 26 comments
Assignees
Labels
enhancement New feature or request

Comments

@YouveGotMeowxy
Copy link

I have the 'auto-redirect to new container w/matching name' option checked (works about half the time), and I'd also love to have it do so immediately w/out the delay it currently has. (the dialog showing there's a matching container, with the progress bar on the button).

When working on building images, and doing tweak after tweak with a million restarts, the delay slows the flow. :)

@YouveGotMeowxy YouveGotMeowxy added the enhancement New feature or request label Jun 15, 2024
@amir20
Copy link
Owner

amir20 commented Jun 16, 2024

Maybe the options should be instant, delayed, never.

works about half the time

I have seen this happen too. But I am not sure why. The redirect happens in javascript so the tab needs to be active. Can you help out and figure out when it doesn't work? The code is pretty simple.

const nextContainer = computed(
() =>
[
...allContainers.value.filter(
(c) =>
c.host === containers.value[0].host &&
c.created > logEntry.date &&
c.name === containers.value[0].name &&
c.state === "running",
),
].sort((a, b) => +a.created - +b.created)[0],
);

It also only works with containers not swarm. I don't use this often so if you use it a lot then help make it better. ❤️

@YouveGotMeowxy
Copy link
Author

Maybe the options should be instant, delayed, never.

Perfect :)

works about half the time

Maybe that's what it is. I'll have to pay closer attention, since I do a ton of tab switching. Maybe the js can keep checking if the tab is active and only do the switch when it is. That way even if tab isn't active during the container shutdown/restart it'll still switch once the js detects the tab is active no matter how much time elapses? I don't really know much js so I'm just thinking outloud.

@FrancYescO
Copy link

@amir20 why are you checking for c.state === "running" ? maybe this is the cause of faling redirect in some cases as, if i'm supposing right, the container can be in other states ("restarting" or also "exited"...) status and you will be not redirected

@amir20
Copy link
Owner

amir20 commented Dec 10, 2024

Hey @FrancYescO. I missed this comment. You are right that I look for running. If the container is restarting, starting, or whatever, I assume it will eventually get to "running" right? If it is exited then I assume it started and something broke.

Is that what you think is happening? If you can think of an example of where "running" is not what the user wants then a quick PR is easy.

@FrancYescO
Copy link

FrancYescO commented Dec 10, 2024

If it is exited then I assume it started and something broke

yes, or maybe is sloooowly starting, as the check for redirect is done at a specific time (at +5s from the exit of the old container?) and not "waiting for the new container running" maybe redirect to a container that have a newer create date (if any), ignoring the state is the best thing to do..

for the scope of the issue maybe having the actually hardcoded 5000 value configurable from settings is enough... putting it to 0 and the redirect will be instant. ;)

@amir20
Copy link
Owner

amir20 commented Dec 10, 2024

yes, or maybe is sloooowly starting, as the check for redirect is done at a specific time (at +5s from the exit of the old container?) and not "waiting for the new container running" maybe redirect to a container that have a newer create date (if any), ignoring the state is the best thing to do..

That's not what's happening. That piece of code is reactive, and it gets triggered every time a container's state changes. This could occur 15 seconds later, causing the computed part in Vue to be retriggered. If it finds a new container that meets all the criteria, it will display the UI that redirects. The UI will then redirect after 5 seconds.

The hardcoded value is simply a UI wait time.

@FrancYescO
Copy link

So assume there are 2 edge case that does not trigger the redirect if i got it right: the actual container gets destroyed before the new one go in the running status (this is pretty standard during portainer stack redeploy) and simply if the new container crashes/go in a restart loop on start

@amir20
Copy link
Owner

amir20 commented Dec 10, 2024

@FrancYescO while doing some testing I noticed some times it was in a bad state. When your container stops, do you see a "container stopped" component?

Screenshot 2024-12-10 at 12 23 17 PM

The log that does the forwarding is inside that component. I noticed some times it wasn't showing up correctly.

I think I fixed it in #3445

Can you try pr-3445? You should see the container stopped correctly. Otherwise the condition would never trigger.

@FrancYescO
Copy link

FrancYescO commented Dec 10, 2024

When your container stops, do you see a "container stopped" component?

thinking about it, maybe sometime this not popupped, but just done a container update via portainer, the component correctly popupped, but i was not redirected to the new container (i was on another tab, but i dont think this matters i never get redirected while portainer updates a container, maybe becouse while stopped the old container, the new one is still not renamed)

btw i'll see if something changes with #3445

@amir20
Copy link
Owner

amir20 commented Dec 10, 2024

Hmm. I have been doing

docker run --rm --name test amir20/echo -r
# then kill it with ctrl+c
docker run --rm --name test amir20/echo -r

Dozzle should redirect to the new one.

@FrancYescO
Copy link

FrancYescO commented Dec 11, 2024

This is an example test case where you will be not redirected (also with #3445):

docker run --name test hello-world
#now click on dozzle on the test container, it is already stopped but doesn't matter i assume
docker create  hello-world
# 975df249989b2b06ff24dd3bc2d970ac77d5a31c6b04ae8c011cbc36a9f4346d
docker rm test
#here you will remain on visualization of an orphaned container not present in the sidebar
docker rename 975df249989b2b06ff24dd3bc2d970ac77d5a31c6b04ae8c011cbc36a9f4346d test
docker start test
#container correctly popup in the sidebar but you will be not redirected

PS: not used echo container cause: The requested image's platform (linux/arm64) does not match the detected host platform (linux/amd64/v3)

@amir20
Copy link
Owner

amir20 commented Dec 11, 2024

Interesting 🤔 This should work because I added support for rename. But I haven't tested the redirect. Your directions help so let me try it out.

@amir20
Copy link
Owner

amir20 commented Dec 11, 2024

It didn't for me either. I think the problem is c.created > logEntry.date. That only looks for containers that have been created after the current one has been stopped. Which in this case with create then rename and start is not true.

@FrancYescO
Copy link

FrancYescO commented Dec 11, 2024

Maybe adding a "container created/started/renamed" stub logs (like the stopped one) and attach the logic to these?

Btw try to have the browser dozzle tab not as the active one (but for me seems not working also when the tab is active)

@amir20
Copy link
Owner

amir20 commented Dec 11, 2024

I think I fixed it by updating the logic for created. #3446

Can you try pr-3446?

@amir20
Copy link
Owner

amir20 commented Dec 11, 2024

Btw try to have the browser dozzle tab not as the active one (but for me seems not working also when the tab is active)

Unfortunately, the redirect does occur in JavaScript. This behavior depends on the browser. I'll try to reproduce it in Chrome, but it's challenging because the browser may terminate the JavaScript at any time.

@FrancYescO
Copy link

FrancYescO commented Dec 11, 2024

Looks i'm still not redirected... are you using the hello-world container for the tests (as it is a container that fastly stop itself)?
Also noticed that the new container in the sidebar shows a wrong running/uptime (probably just mislabeled, as looks it is the create time)
image

@amir20
Copy link
Owner

amir20 commented Dec 11, 2024

are you using the hello-world container for the tests

I was not. I didn't think that makes a difference. The problem is I don't have all containers enabled. But I can enable it. Question to you, is a container dying right away a valid use case? Do you start a container, it dies and then start a new one again?

Also noticed that the new container in the sidebar shows a wrong running/uptime (probably just mislabeled, as looks it is the create time)

That's right. Easy fix when I get to it.

@amir20
Copy link
Owner

amir20 commented Dec 11, 2024

are you using the hello-world container for the tests

Ah, this makes sense because of c.state === "running". I believe this is a question about user intent. As someone using Dozzle, a user would likely want to follow the next container only if it is running and has not exited. It could be confusing to continue to the next container if they keep exiting.

I think it would be helpful to understand your use case. Are you using hello-world as an example, or do you actually have a use case where the container is short lived?

@FrancYescO
Copy link

Are you using hello-world as an example, or do you actually have a use case where the container is short lived?

I have 2 cases when this happens:

  • the new version of the container being deployed have a misconfiguration/something broken an just crash on start (or go in a restart loop if configured as restart: always)
  • the container in general is a container that need to do only some check/configurations stuffs for the other containers in the stack, so the exit after few seconds is "genuine"

In both cases i think is preferable being redirected to the newest logs

PS. The "container stopped" label, if you click on an already stopped container shows the actual timestamp instead of timestamp when the container was really stopped, this is a little misleading I don't know if can have his part of culprit in this issue

@amir20
Copy link
Owner

amir20 commented Dec 12, 2024

I see. Those use cases make sense. But I think now I remembering why it was implemented this way. 😟 Imagine restart: always is true. Then Dozzle finds a new container with the same name. The UI shows do you want to redirect? In that timeframe, a few more instance of that container have been created. Which one Dozzle does redirect to?

With the PR at #3446 I have removed running as a requirement. Give it a try. I am not sure how it will work out, I think it will need a little more testing.

The "container stopped" label, if you click on an already stopped container shows the actual timestamp

It's a little wacky right now. I could find when the container exited. Instead, I just use when the stream received EOF. Probably a bug.

Can you test #3446 again? You'll have to play around with it to make sure it works.

@FrancYescO
Copy link

The UI shows do you want to redirect? In that timeframe, a few more instance of that container have been created. Which one Dozzle does redirect to?

The instant redirect indirectly solve this issue :D but maybe, the best thing to do when delayed is to reset the timer of the ongoing redirect and follow the new one if any newer found (but hey, here we are just restarting the cointainer, not recreating that changes the ID, so the redirect should work and this case should of new instances found should be very limited)

@FrancYescO
Copy link

basically looks it's working, noticed a bug: if stopped (restarted) multiple time, the redirect will happen multiple times :D

image

@amir20
Copy link
Owner

amir20 commented Dec 12, 2024

D'OH. Not sure how to fix it though. The two rows are isolated so they have no knowledge of each other.

@FrancYescO
Copy link

just ignore it, nothing really important :P

maybe this redirect can be moved in the future to some other component not related to the stopped row..

@amir20
Copy link
Owner

amir20 commented Dec 13, 2024

I have merged for now. Thinking about this a little more, if the IDs are the same then the popup shouldn't show up.

But for now I'll leave as is. I won't be releasing a new version until next week. Meanwhile, master should be at a good spot if you can test it.

Repository owner locked and limited conversation to collaborators Jan 10, 2025
@amir20 amir20 converted this issue into discussion #3538 Jan 10, 2025

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants