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

Attaching shell is only possible after invoking Docker sidebar #2029

Closed
dschuessler opened this issue May 31, 2020 · 13 comments · Fixed by #2036
Closed

Attaching shell is only possible after invoking Docker sidebar #2029

dschuessler opened this issue May 31, 2020 · 13 comments · Fixed by #2036

Comments

@dschuessler
Copy link

Steps to reproduce:

  1. Start a multi-container docker-compose project with docker-compose up.
  2. Try to open a shell via the command picker (Cmd+Shift+P) on Mac

Expected behavior: I can select a container to which I want to attach a shell.

Actual behavior: I get a toast telling me "No running containers are available to attach" though there clearly are some. Only when I invoke the Docker sidebar it works.

ghjg-minhh

@bwateratmsft
Copy link
Contributor

@dschuessler I was not able to reproduce this. I notice in your GIF that you have quite a few containers running, and it's taking a good deal of time to load them--is your Docker daemon over SSH or TCP?

@dschuessler
Copy link
Author

I'm not sure. I use Docker for Mac on my machine and do not remember configuring any connection options, so I guess I'm using the defaults. Please let me know how to check if this does not answer your question.

@bwateratmsft
Copy link
Contributor

bwateratmsft commented Jun 1, 2020

I'm guessing it's Docker Desktop for Mac then. I'll try creating a bunch of containers and see if I can reproduce with more.

Any changes to the refresh interval setting? docker.explorerRefreshInterval

@dschuessler
Copy link
Author

Thank you for digging into this.

I have no Docker specific settings in VSCode.

@bwateratmsft
Copy link
Contributor

Gotcha. It's hard to tell from the gif but it looks like the docker-compose up action is still in progress, up until just before you switch views over to the Explorer page (I noticed that after the last attempt of "attach shell" and before switching, it scrolled quite a few more lines).

Just to eliminate that as a possible cause, can you try running the docker-compose up command and waiting until it is complete + about 2 seconds (the explorer refresh interval is 2s by default, so it can be about that far behind realtime), then doing the attach command?

@dschuessler
Copy link
Author

docker-compose up does not terminate as the programs in the containers don't terminate. I'm not sure what you mean by "waiting until it is complete".

@bwateratmsft
Copy link
Contributor

Ah, I'm blind! I thought you were doing the built-in command (which uses the -d flag and would terminate when everything is done starting), I see now it is without that flag.

@bwateratmsft
Copy link
Contributor

@dschuessler I was able to reproduce the bug, I'm looking at how best to fix it.

@bwateratmsft bwateratmsft self-assigned this Jun 2, 2020
@bwateratmsft
Copy link
Contributor

bwateratmsft commented Jun 2, 2020

@EricJizbaMSFT, I have two ideas for a solution but I want to get your thoughts since one of them would involve a change in vscode-azureextensionui. The sequence of events is this:

  1. Something refreshes the Explorer UI. No containers are running. For example, you can just open the explorer tab. Objects are loaded and AzExtParentTreeItem._clearCache becomes false.
  2. Leave the explorer tab. Refresh events stop, since we only refresh if the view is visible.
  3. Start a container
  4. Try to run "Attach Shell". This calls AzExtTreeDataProvider.showTreeItemPicker(), which uses the cached data (no containers), since _clearCache is false.

So, my two ideas for solution:

  1. Change in vscode-azureextensionui: Calling showTreeItemPicker() will clear the cache, i.e., call treeItem.clearCache() (where treeItem is startingTreeItem || treeDataProvider._rootTreeItem, as is today)
  2. Change in Docker extension: Before any commands calling showTreeItemPicker(), the Docker extension will call clearCache(). ~34 instances of this.

@ejizba
Copy link
Member

ejizba commented Jun 2, 2020

The auto-refresh behavior is very unique to Docker, so I think your fix here is going to have to be unique to Docker. The Docker example is accessing local resources which is very fast, but most of our trees display Azure resources, which can take a while to load (hence why the cache is beneficial). Also, clearing the cache means the user would lose any progress they've made with the "Load more..." option.

I'm okay if you want to change the ui package (aka something like number 1) - but only if it's a configurable thing that's off by default. For example, perhaps it's a setting on AzExtTreeDataProvider

@bwateratmsft
Copy link
Contributor

bwateratmsft commented Jun 2, 2020

I'll go ahead and do it in Docker then. Perhaps we can limit it to containers stuff only, since images / registries / etc. aren't nearly as volatile as containers are.

@bwateratmsft
Copy link
Contributor

bwateratmsft commented Jun 4, 2020

@dschuessler Thanks for finding this and helping us narrow down the problem! It should be fixed in our upcoming 1.3.0 release. I tested it out and everything seemed to work nicely.

@bwateratmsft
Copy link
Contributor

The fix for this is now released in 1.3.0.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants