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

client: app info doesn't show binded volumes #2168

Open
ggarnier opened this Issue Sep 25, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@ggarnier
Member

ggarnier commented Sep 25, 2018

app info command could show information about the volumes that are binded to that app. Currently the only way to check this is running volume list command and searching for the app name.

@theag3nt

This comment has been minimized.

Show comment
Hide comment
@theag3nt

theag3nt Oct 6, 2018

I'll have a look at this if you don't mind.

theag3nt commented Oct 6, 2018

I'll have a look at this if you don't mind.

@ayr-ton

This comment has been minimized.

Show comment
Hide comment
@ayr-ton

ayr-ton Oct 9, 2018

@theag3nt Any findings on this one?

ayr-ton commented Oct 9, 2018

@theag3nt Any findings on this one?

@theag3nt

This comment has been minimized.

Show comment
Hide comment
@theag3nt

theag3nt Oct 9, 2018

@ayr-ton Not much yet. I had some trouble setting up a full build and test environment with tsuru-bootstrap. I've read it's deprecated, but as it's still included in the documentation I expected it to mostly work. Did some small fixes, but still stuck at the dashboard install. But I think I'll let it go, and I either try the docker-compose method, or set up a VM with the installer.

Nonetheless, I still want to implement a fix, I was just a bit delayed. 🙂

theag3nt commented Oct 9, 2018

@ayr-ton Not much yet. I had some trouble setting up a full build and test environment with tsuru-bootstrap. I've read it's deprecated, but as it's still included in the documentation I expected it to mostly work. Did some small fixes, but still stuck at the dashboard install. But I think I'll let it go, and I either try the docker-compose method, or set up a VM with the installer.

Nonetheless, I still want to implement a fix, I was just a bit delayed. 🙂

@ayr-ton

This comment has been minimized.

Show comment
Hide comment
@ayr-ton

ayr-ton Oct 9, 2018

@theag3nt Hmmm... maybe there's an opportunity to updated the README.md providing new getting started instructions. \o/

ayr-ton commented Oct 9, 2018

@theag3nt Hmmm... maybe there's an opportunity to updated the README.md providing new getting started instructions. \o/

@theag3nt

This comment has been minimized.

Show comment
Hide comment
@theag3nt

theag3nt Oct 10, 2018

@ayr-ton The README is probably fine, I've just jumped straight to the Contribution chapter of the documentation where Vagrant with tsuru-bootstrap is still listed as an option. Maybe that part of the documentation could be improved.

theag3nt commented Oct 10, 2018

@ayr-ton The README is probably fine, I've just jumped straight to the Contribution chapter of the documentation where Vagrant with tsuru-bootstrap is still listed as an option. Maybe that part of the documentation could be improved.

@ggarnier

This comment has been minimized.

Show comment
Hide comment
@ggarnier

ggarnier Oct 11, 2018

Member

@theag3nt probably the easiest way to run a development environment is using docker compose: https://docs.tsuru.io/stable/contributing/compose.html

You could also try tsuru installer: https://docs.tsuru.io/stable/installing/using-tsuru-installer.html

If you find something is missing, confusing or outdated in our docs, feel free to contribute to it also, we would really appreciate!

Member

ggarnier commented Oct 11, 2018

@theag3nt probably the easiest way to run a development environment is using docker compose: https://docs.tsuru.io/stable/contributing/compose.html

You could also try tsuru installer: https://docs.tsuru.io/stable/installing/using-tsuru-installer.html

If you find something is missing, confusing or outdated in our docs, feel free to contribute to it also, we would really appreciate!

@theag3nt

This comment has been minimized.

Show comment
Hide comment
@theag3nt

theag3nt Oct 12, 2018

@ggarnier Thanks, I've managed to setup an environment with docker-compose and minikube. It wasn't easy but now I have an application with a bound volume, so I can at least reproduce the current situation.

I had a look in the source code for the client's app-info and volume-list commands and the server's /apps/<app> and /volumes endpoints. As I see there is currently no direct way of filtering the volume list before it's returned by the API. At a first glance, I think there are two possibilities:

  1. Extend the /volume endpoint with an app parameter (like some other endpoints) so it can return only the volumes currently bound to the specified app. I will look into this and propose a more detailed solution if this is preferred.
  2. Filter the list of all volumes from /volume endpoint on the client side. This is much easier I think.

Either way, the app-info command will have to be extended by adding one more API call to the server (/volumes endpoint), and adding the results to the printed output. Mostly affecting AppInfo.Run(), AppInfo.Show(), app and app.String().

What do you think?

theag3nt commented Oct 12, 2018

@ggarnier Thanks, I've managed to setup an environment with docker-compose and minikube. It wasn't easy but now I have an application with a bound volume, so I can at least reproduce the current situation.

I had a look in the source code for the client's app-info and volume-list commands and the server's /apps/<app> and /volumes endpoints. As I see there is currently no direct way of filtering the volume list before it's returned by the API. At a first glance, I think there are two possibilities:

  1. Extend the /volume endpoint with an app parameter (like some other endpoints) so it can return only the volumes currently bound to the specified app. I will look into this and propose a more detailed solution if this is preferred.
  2. Filter the list of all volumes from /volume endpoint on the client side. This is much easier I think.

Either way, the app-info command will have to be extended by adding one more API call to the server (/volumes endpoint), and adding the results to the printed output. Mostly affecting AppInfo.Run(), AppInfo.Show(), app and app.String().

What do you think?

@ggarnier

This comment has been minimized.

Show comment
Hide comment
@ggarnier

ggarnier Oct 18, 2018

Member

I think listing all volumes and filtering on the client may not be a good option, as the number of volumes could be really large.

I vote for changing the volumes api to support filtering by binded app, and adding this call to the client.

Member

ggarnier commented Oct 18, 2018

I think listing all volumes and filtering on the client may not be a good option, as the number of volumes could be really large.

I vote for changing the volumes api to support filtering by binded app, and adding this call to the client.

@andrestc

This comment has been minimized.

Show comment
Hide comment
@andrestc

andrestc Oct 18, 2018

Member

I agree on filtering on the API, but we might need to also filter on the client just to make sure that a newer client doesn't show all existent volumes if the user is running older version of the API (one that does not support the filter).

Member

andrestc commented Oct 18, 2018

I agree on filtering on the API, but we might need to also filter on the client just to make sure that a newer client doesn't show all existent volumes if the user is running older version of the API (one that does not support the filter).

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