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

Design proposal for AppView improvements #1529

Merged
merged 4 commits into from
Mar 3, 2020
Merged

Conversation

andresmgot
Copy link
Contributor

Description of the change

Design doc for improvements introduced in #1524

Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

Great - thanks for documenting it all Andres. The application status improvement looks like a no-brainer (as in, +100 - it'd improve things a lot). Some of the others don't seem like high priority to me, and the main, major change I think we need to think very carefully about as it has the danger of becoming less-focused on the user experience, rather than more focused, imo. Details below.


### Show Application Dependencies

Application information already contains the file requirements.yaml. This is the file used by Helm to define the application dependencies. That file is encoded in base64 and it's available under the path `app.chart.files[2].value`. Once that information is properly parsed, we should be able to render that as part of the chart information.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is one of the changes with Helm3 - moving the dependencies from the separate requirements.yaml to the main Chart.yaml, though many charts will still be using the old requirements.yaml. We may need to update our Chart struct to include the dependencies and populate that from the Chart.yaml falling back to a requirements.yaml.

That said, I'm not sure that including some extra info about chart dependencies should be a priority for this work, when compared to the other points...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, I forgot about that change in the requirements specification.

Re: Priority

Agree, the goal of this design document was to list everything I could think of to improve the current app view. I didn't add priorities (maybe I should?). I can add a section to define the different items by priority and see if we agree.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'd include that towards the beginning so it's clear before you read the details (that would have helped me).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see you've already included it. I agree with the relative priorities you've given them.


### Improve Application Status Report

Right now we only show a "Ready" or "Not Ready" status message depending on the Application workloads. We take into account deployments, statefulsets and daemonsets. Only if all of those are ready, we show the "Ready" status. Since we already have that information, we can show it to the user in order to identify what can go wrong or what it's missing:
Copy link
Contributor

Choose a reason for hiding this comment

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

Big +1, this would be a huge improvement. Let's get some input on the design (if you haven't already). Excited to see this.


### Highlight Application Credentials

In the current view, application credentials can be found either reading and executing the commands that the Notes suggest, or clicking on the icon to reveal the secrets below. Those secrets usually are of the type "Opaque" (rather than `kubernetes.io/tls` or `kubernetes.io/service-account-token`). We can extract those credentials and show them in a more prominent place. That way the user can easily discover the application credentials without the need of a terminal:
Copy link
Contributor

Choose a reason for hiding this comment

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

I get that it is a little painful to do the kubectl get secret ... and pipe through base64 etc., but I think we should think through how users are using these secrets before doing too much work. I mean, wouldn't most of these secrets require a terminal to use anyway (eg. db secrets), in which case enabling easy discovery via the UI may not actually have a huge use-case. In the WP example, the one exception is the actual admin login which is required for the UI. Perhaps it could be useful to identify UI secrets, or secrets which will be needed by the user and make those more prominent.

Also, rather than a reveal, what would help me if I needed these is a copy icon (as in, I'm not interested in knowing the password, I just want to paste it somewhere, I assume).

But overall, similar to the dependencies, this seems like a lesser priority imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get that it is a little painful to do the kubectl get secret ... and pipe through base64 etc., but I think we should think through how users are using these secrets before doing too much work. I mean, wouldn't most of these secrets require a terminal to use anyway (eg. db secrets), in which case enabling easy discovery via the UI may not actually have a huge use-case. In the WP example, the one exception is the actual admin login which is required for the UI.

Indeed, this is more useful for web applications than for services but I think it's still an important improvement since a good percentage of charts are web apps. For users deploying these web apps it would be trivial to retrieve password if there is a secret named wordpress-password.

Perhaps it could be useful to identify UI secrets, or secrets which will be needed by the user and make those more prominent.

Yes, the problem with that is that it would require collaboration with the application developers (which is much more difficult to get).

Also, rather than a reveal, what would help me if I needed these is a copy icon (as in, I'm not interested in knowing the password, I just want to paste it somewhere, I assume).

That's a good point, I will add it to the proposal.


### Compress Application Actions

The current approach is to show one button per action (Upgrade, Delete, Rollback). This list is growing over time (for example, the backend endpoint for running tests is ready to be used). It can also be confusing to show a different list of buttons depending on the release state. One example is the Rollback button that is only rendered if the application has been upgraded. In order to avoid these issues, we can show a clickable menu with the different options, graying out the options not available (potentially showing a tooltip):
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not a huge priority imo, though agree it would be nicer. One point related to this that was pointed out a month ago was that people are needing to use "Upgrade" when the really want to reconfigure the existing release (ie. a new release, but with changed config options, no upgrade). I wonder if this is worth considering in relation to actions (ie. providing a way in the UI to reconfigure without upgrading and having to use the current chart). This would also enable us to revert the reversion and default to the latest chart version when actually upgrading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a low-hanging fruit, not very important but easy to implement.

One point related to this that was pointed out a month ago was that people are needing to use "Upgrade" when the really want to reconfigure the existing release (ie. a new release, but with changed config options, no upgrade). I wonder if this is worth considering in relation to actions (ie. providing a way in the UI to reconfigure without upgrading and having to use the current chart). This would also enable us to revert the reversion and default to the latest chart version when actually upgrading.

I would be wary of changing this. Helm users are used to use "upgrade" when they really mean "configure" or "modify". Helm 3 didn't change anything related to that so it will keep it that way so we should be careful diverting from the "official" way of doing things (even if they are not totally correct). We could run an experiment though having "Configure" and "Upgrade" separated but both pointing to the Upgrade form with the only difference that the version is blocked when configuring.


The current list of URLs can be improved with two small changes:

- If the browser can access the URL, we could show an icon so the user knows if the URL is working.
Copy link
Contributor

Choose a reason for hiding this comment

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

As in, we can fetch a HEAD request or similar, and as long as we get a 200 or a method not allowed, we know it is up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, that's what I was thinking. It wouldn't really mean that it's working though since I can create an ingress pointing to github.com and that won't work but it would be returning a 200.

The current list of URLs can be improved with two small changes:

- If the browser can access the URL, we could show an icon so the user knows if the URL is working.
- If the URL is not working and the cause is well known, we could show additional information for the user to debug it (e.g. Pending IPs when using LoadBalancers in Minikube [link](https://github.com/kubeapps/kubeapps/issues/953)).
Copy link
Contributor

Choose a reason for hiding this comment

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

This also sounds very useful. Worth getting input on the UI when Angel or others have time.


### Render an Extended Resources Table

Finally, we can show a single resource table with all the resources so users can inspect in detail the different resources of the application without adding too much noise to the view. Apart from the list of resources we are currently showing, we can add the list of pods related to the application (the ones that are related to a Deployment/Statefulset/Daemonset).
Copy link
Contributor

Choose a reason for hiding this comment

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

This is looking like too many knobs to me, and moving away from the idea that Kubeapps is enabling you to interact with a more general 'Application', rather than a set of Kubernetes resources. As an example, I don't see a reason for us exposing the individual pods, given that:

  1. We can show logs for the more general "service" object with kubectl logs service/wordpress (which shows logs for all pods in the deployments matching the service selector, afaict), and
  2. If a deployment for a service has a pod in an error state, we can raise that information, and only that (relevant) information, to the focus for that service (eg. scheduling error, not enough cpu or whatever), without needing to drill down so far.

I reckon that we could do something much simpler here by just bringing the services of an app to the forefront and have detailed knobs to see all k8s resources in the background if at all, rather than in the foreground. Another way of thinking about it: I think the design applied here should do to the application view what your work on the deployment form did for the deployment view: focusing on just what the user needs, rather than providing all the knobs and exposing everything. Let's think very carefully about this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is looking like too many knobs to me, and moving away from the idea that Kubeapps is enabling you to interact with a more general 'Application', rather than a set of Kubernetes resources. As an example, I don't see a reason for us exposing the individual pods

I believe we can provide both. In fact, the first part of the view is meant to be focused on that, just the application: source chart, application status, URLs, credentials... Then, when you start scrolling you get a more in-depth view: helm notes, k8s resources, specific app values...

We cannot ignore the second if we want to give a complete experience. If my application is not working because an init container from one pod is not able to start because the node run out of space and we don't allow the user to discover that we would be giving an incomplete experience.

Note that we are not forcing users to understand a pod YAML in order to use Kubeapps, we are just giving all the tools that we can to be as useful as possible.

We can show logs for the more general "service" object with kubectl logs service/wordpress (which shows logs for all pods in the deployments matching the service selector, afaict), and

That's a good point, we can enable logs for services but I think it's still useful to check a pod logs to avoid noise like multiple health-check requests or in case that just one replica is failing.

If a deployment for a service has a pod in an error state, we can raise that information, and only that (relevant) information, to the focus for that service (eg. scheduling error, not enough cpu or whatever), without needing to drill down so far.

The problem with that would be to return misleading errors. I think we don't the capacity to know the real error.

I reckon that we could do something much simpler here by just bringing the services of an app to the forefront and have detailed knobs to see all k8s resources in the background if at all, rather than in the foreground. Another way of thinking about it: I think the design applied here should do to the application view what your work on the deployment form did for the deployment view: focusing on just what the user needs, rather than providing all the knobs and exposing everything. Let's think very carefully about this one.

I agree that if possible, we should just report relevant info (or at least highlight it first) but I don't see a better way to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot ignore the second if we want to give a complete experience. If my application is not working because an init container from one pod is not able to start because the node run out of space and we don't allow the user to discover that we would be giving an incomplete experience.

I actually think we could have a tool (if there's not one already) that can be pointed at a deployment and determine the issue, returning a "report" with both human readable and machine readable info. This could be a separate tool (hmm, could be like the SelfSubjectAccessReview - with a CRD that returns a resource with the details filled in - could be very useful as a separate tool... anyway). I think we could raise your example error to something like: "The init-container 'foo-container' of the pod 'foo-abc123' has failed to start. The reported error is: 'whatever - insufficient storage'.", with further details in the report, but not attempt to provide the ability to dig into any and every k8s resource in the Kubeapps UI (at least, not yet).

IMO, I don't think we should be aiming to provide a "complete experience", (again, at least not yet). It may be down the road that once we have the smooth overview experience for the App, that we allow people to do things like drill down and view/edit individual resources, but the people who understand that UX and those resources are going to be the people who are comfortable with the CLI, so I personally think that initially the focus should be on improving things for the user who wants to install stuff and wants to know what went wrong without navigating all the resources (again, big +1 to logs per service, but let's think about going deeper).

Anyway, that's just my thoughts - I am keen to hear what you think, but not wanting to block you on this. It would be good to wait on this one to get @migmartri 's input though, I think (it may not come soon though, given his current situation, but we could start on the other P0 and P1 things?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually think we could have a tool (if there's not one already) that can be pointed at a deployment and determine the issue, returning a "report" with both human readable and machine readable info. This could be a separate tool (hmm, could be like the SelfSubjectAccessReview - with a CRD that returns a resource with the details filled in - could be very useful as a separate tool... anyway). I think we could raise your example error to something like: "The init-container 'foo-container' of the pod 'foo-abc123' has failed to start. The reported error is: 'whatever - insufficient storage'.", with further details in the report, but not attempt to provide the ability to dig into any and every k8s resource in the Kubeapps UI (at least, not yet).

I don't think such a tool exists, for the moment the easiest way to implement something close is allow people to discover errors on their own.

IMO, I don't think we should be aiming to provide a "complete experience", (again, at least not yet). It may be down the road that once we have the smooth overview experience for the App, that we allow people to do things like drill down and view/edit individual resources, but the people who understand that UX and those resources are going to be the people who are comfortable with the CLI, so I personally think that initially the focus should be on improving things for the user who wants to install stuff and wants to know what went wrong without navigating all the resources (again, big +1 to logs per service, but let's think about going deeper).

I disagree but even if this only helps people that already would be able to fix the issue with a terminal, I think is valuable. If you don't need to leave kubeapps in order to deploy, detect an error and redeploy you are saving people time.

Also, for non experienced users, they would be able to click around until they find some message that says "Error: Whatever..." giving at least the chance to fix it even if they don't know how to use kubectl or how retrieve k8s events related to a deployment.

IMO this would be a great feature for Kubeapps (and not something super complicated). It's something that as a user I would really like. For example, Andreas said: "I really like the table showing Application Resources." so I am not alone there :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think such a tool exists,

Yeah, it could be quite interesting, not just to Kubeapps, but other tools like BKPR. I'll look around out of interest.

for the moment the easiest way to implement something close is allow people to discover errors on their own.

Hmm, I think we could give it a little more thought than that, to see if we could do something which was a better experience than just exposing the CLI resources, but you could be right.

IMO, I don't think we should be aiming to provide a "complete experience", (again, at least not yet). It may be down the road that once we have the smooth overview experience for the App, that we allow people to do things like drill down and view/edit individual resources, but the people who understand that UX and those resources are going to be the people who are comfortable with the CLI, so I personally think that initially the focus should be on improving things for the user who wants to install stuff and wants to know what went wrong without navigating all the resources (again, big +1 to logs per service, but let's think about going deeper).

I disagree but even if this only helps people that already would be able to fix the issue with a terminal, I think is valuable. If you don't need to leave kubeapps in order to deploy, detect an error and redeploy you are saving people time.

I think we agree on that point though: helping people to be able to fix the issue within Kubeapps is helpful and something we want to do. We already agree that allowing people to dig to logs will be useful (and perhaps raise these to the users focus when there is an error). The only point where we're currently exploring differences is how to best expose errors or details: Should we provide direct access from the app view to all relevant k8s resources (similar to Octant?) that they can browse to find the error, or should we try to raise the error up to the app view's main flow (while still allowing people to dig down to some detail where required).

And all I'm trying to say at that point is: I'm not sure. I want to make sure we think carefully about product focus and direction, rather than just doing something which fills the gap.

Also, for non experienced users, they would be able to click around until they find some message that says "Error: Whatever..."

Yes, but is "click around until they find some message" the experience we want to enable? If it is possible for us to instead present something clearly such as "Your app 'apache' did not successfully deploy because there were not enough system resources available to create the foo resources", or "Your app 'foobar' is not successful yet because the deployment 'foobar-database' is failing to start. More details available in the [link]logs[/link]".

giving at least the chance to fix it even if they don't know how to use kubectl or how retrieve k8s events related to a deployment.

Yes - we definitely want to ensure this is possible which ever way we go.

IMO this would be a great feature for Kubeapps (and not something super complicated). It's something that as a user I would really like. For example, Andreas said: "I really like the table showing Application Resources." so I am not alone there :)

And I'm agreeing too - that being able to browse all resources would be a great feature - I'm just unsure whether we should show it prominently here on the app view regardless of errors, or whether there is a better option to help users find and fix errors on the app view while still being able to drill down to inspect the resources.

Either way, as above, I'm not wanting you to block on this - it'll be a great step forward either way - I just thought it worth getting more input. Hope that clears things up!

@andresmgot
Copy link
Contributor Author

@absoludity anything else you think we should cover here?

Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

Arg, sorry - I'd reviewed this yesterday but left it pending without submitting.


### Show Application Dependencies

Application information already contains the file requirements.yaml. This is the file used by Helm to define the application dependencies. That file is encoded in base64 and it's available under the path `app.chart.files[2].value`. Once that information is properly parsed, we should be able to render that as part of the chart information.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'd include that towards the beginning so it's clear before you read the details (that would have helped me).


### Show Application Dependencies

Application information already contains the file requirements.yaml. This is the file used by Helm to define the application dependencies. That file is encoded in base64 and it's available under the path `app.chart.files[2].value`. Once that information is properly parsed, we should be able to render that as part of the chart information.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see you've already included it. I agree with the relative priorities you've given them.


### Render an Extended Resources Table

Finally, we can show a single resource table with all the resources so users can inspect in detail the different resources of the application without adding too much noise to the view. Apart from the list of resources we are currently showing, we can add the list of pods related to the application (the ones that are related to a Deployment/Statefulset/Daemonset).
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot ignore the second if we want to give a complete experience. If my application is not working because an init container from one pod is not able to start because the node run out of space and we don't allow the user to discover that we would be giving an incomplete experience.

I actually think we could have a tool (if there's not one already) that can be pointed at a deployment and determine the issue, returning a "report" with both human readable and machine readable info. This could be a separate tool (hmm, could be like the SelfSubjectAccessReview - with a CRD that returns a resource with the details filled in - could be very useful as a separate tool... anyway). I think we could raise your example error to something like: "The init-container 'foo-container' of the pod 'foo-abc123' has failed to start. The reported error is: 'whatever - insufficient storage'.", with further details in the report, but not attempt to provide the ability to dig into any and every k8s resource in the Kubeapps UI (at least, not yet).

IMO, I don't think we should be aiming to provide a "complete experience", (again, at least not yet). It may be down the road that once we have the smooth overview experience for the App, that we allow people to do things like drill down and view/edit individual resources, but the people who understand that UX and those resources are going to be the people who are comfortable with the CLI, so I personally think that initially the focus should be on improving things for the user who wants to install stuff and wants to know what went wrong without navigating all the resources (again, big +1 to logs per service, but let's think about going deeper).

Anyway, that's just my thoughts - I am keen to hear what you think, but not wanting to block you on this. It would be good to wait on this one to get @migmartri 's input though, I think (it may not come soon though, given his current situation, but we could start on the other P0 and P1 things?).

Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

One suggestion just to capture the discussion, see what you think.

We can also show different buttons in order to show the resource YAML, description and logs if available (available for pods, services and workloads).

<img src="./img/resources-table.png">

Copy link
Contributor

Choose a reason for hiding this comment

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

"If we later develop a clearer way to capture and present errors to users, clearly identifying the cause without requiring clicking through resources, the resources table will still be useful as a way to dig deeper and learn more without leaving the UI."

@andresmgot andresmgot merged commit 60fa172 into master Mar 3, 2020
@andresmgot andresmgot deleted the appviewRevampDoc branch March 10, 2020 11:58
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