-
Notifications
You must be signed in to change notification settings - Fork 705
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
1495 5 app repos for namespace only #1503
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Michael! Check my comment, I think we need to change the behavior a bit.
BTW, I see that your gifs are quite small :D I normally use https://www.onlineconverter.com/mp4-to-gif to easily generate gifs with certain quality.
<MessageAlert header="Looking for site-wide app repositories?"> | ||
<div> | ||
<p className="margin-v-normal"> | ||
You can view site-wide App Repositories by selecting "All Namespaces" above, if you | ||
have permission to view or edit App Repositories available to all namespaces. | ||
</p> | ||
<p className="margin-v-normal"> | ||
Kubeapps now enables you to create App Repositories in your own namespace. You can | ||
read more information in the{" "} | ||
<a href="https://github.com/kubeapps/kubeapps/blob/master/docs/user/private-app-repository.md#per-namespace-app-repositories"> | ||
Private App Repository docs | ||
</a> | ||
. | ||
</p> | ||
</div> | ||
</MessageAlert> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the approach should be a bit different. When selecting the namespace kubeapps
is when we should show site-wide repositories. When selecting "All Namespaces" we should show the ones in the kubeapps
namespace plus all the others in the different namespaces.
We would need to clearly identify the ones that are global and the ones that are namespaced (not in this PR, just thinking out loud). I would add two more columns to the table, one to identify the namespace and another to say if it's global or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the approach should be a bit different. When selecting the namespace
kubeapps
is when we should show site-wide repositories. When selecting "All Namespaces" we should show the ones in thekubeapps
namespace plus all the others in the different namespaces.
Yeah - I had planned to do that at first, but I think it could lead to a confusing user experience, for a couple of reasons. If, as a user, I want to create an app repository that will be available for all namespaces, I don't know that I'd think of the kubeapps (or whatever it is configured to) rather than "All Namespaces".
Second, when creating a new AppRepository, it'd be much simpler (UX-wise) to create one in the currently selected namespace (ie. with this PR you don't need to set or select a Namespace, because it is just the one you are in). Though I suppose we could just not allow creating AppRepositories when viewing All Namespaces? (Similar to deploying a chart where we error after you try to install a chart while "All Namespaces" is selected - not great).
So I was planning that the list of AppRepositories would only ever be the list of AppRepositories in that namespace - even when we later enable an AppRepository to have additional namespaces where it can be used, it would only show in the list for that namespace (but clearly identify that it has been available in the additional namespaces). You would get the same list when you chose kubeapps
(or whatever kubeapps namespace is) and the more recognisable "All Namespaces".
Either way, we can make it clear with additional text what you are looking at (ie. either "Showing all App Repositories available to Kubeapps across all namespaces" or "Showing App Repositories which are available in all namespaces", depending which way we go).
Let me know what you think. I'm happy to give it a try tomorrow - it sounds like you think it is important.
We would need to clearly identify the ones that are global and the ones that are namespaced (not in this PR, just thinking out loud). I would add two more columns to the table, one to identify the namespace and another to say if it's global or not.
Yes, planning to add an "Supported namespaces" column (as we'll need it when user AppRepositories can support additional namespaces later). Currently it'd be either a single namespace or All (if we update to show all app repositories when All Namespaces is selected).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - I had planned to do that at first, but I think it could lead to a confusing user experience, for a couple of reasons. If, as a user, I want to create an app repository that will be available for all namespaces, I don't know that I'd think of the kubeapps (or whatever it is configured to) rather than "All Namespaces".
Second, when creating a new AppRepository, it'd be much simpler (UX-wise) to create one in the currently selected namespace (ie. with this PR you don't need to set or select a Namespace, because it is just the one you are in). Though I suppose we could just not allow creating AppRepositories when viewing All Namespaces? (Similar to deploying a chart where we error after you try to install a chart while "All Namespaces" is selected - not great).
To avoid having a bad UX, we can specify the behavior when selecting those namespaces when creating the repositories. I am thinking on something like:
Let me know what you think. I'm happy to give it a try tomorrow - it sounds like you think it is important.
Yes, since our implementation decission (making apprepositories in the kubeapps
namespace available globally) may not be clear, I think it's important that we clarify that to the users as much as possible so they don't see weird behaviors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, Migue also agreed on this point. Please take another look: I've updated so that when selecting "All Namespaces", you see app repositories across all namespaces. I've updated the view so that an extra "Namespace" column is shown only when looking at "All Namespaces", and also updated the info box to match.
Short video (this time at 800px):
Later on when we've updated the assetsvc
we can also display (separately) here AppRepositories from other namespaces which are available in this Namespace (ie. global/site-wide ones, but also others depending).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Later on when we've updated the
assetsvc
we can also display (separately) here AppRepositories from other namespaces which are available in this Namespace (ie. global/site-wide ones, but also others depending).
I am not sure we are in the same page here. As I understood, in order to use an AppRepository, this should be either in the selected namespace or in the kubeapps
namespace. If an AppRepository is meant to be used in several namespaces, I thought we were going to copy those resources to the target namespaces, am I wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an AppRepository is meant to be used in several namespaces, I thought we were going to copy those resources to the target namespaces, am I wrong?
Originally that was the case because we were not considering Kubeapps itself being able to read AppRepository
resources cluster-wide, so it would be necessary when we were using the current users creds to be able to access the AppRepository in the namespace which the user is currently accessing. Once we updated that assumption, so the app repository controller can read AppRepository
resources cluster-wide, we no longer need to copy the AppRepository
resource - it can exist in the one namespace where that repository is created, which avoids other issues of redundant/conflicting data. That said, I'm not yet implementing support for a namespaced app repository to have additional namespaces.
Yeah - I'm using ffmpeg but had explicitly chosen a pretty small output window. I'll make it bigger next time :) |
This is awesome btw https://github.com/phw/peek |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My +1. The message is a bit confusing/long but I don't have a better suggestion 🙃
You can view App Repositories across all namespaces by selecting "All Namespaces" | ||
above, if you have permission to view App Repositories cluster-wide. We will also | ||
add information on the current page indicating App Repositories in other namespaces | ||
which can be used in the current namespace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second sentence is a bit confusing. I am not sure what it means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to say something which you were also trying to say on a different thread: that we will, later on, update this view to include displaying other app repositories which have been made available in this namespace, even though they are not defined here. I agree it's quite confusing and is an implementation detail which we don't need to expose to the user. For now I'm just removing this sentence and we'll later include those.
Ref: #1495 . Updates so that AppRepositories are display per namespace (when the FeatureFlag is enabled).
I've also added a helper which can remain for a release or two after we remove the feature flag, which only displays if you're viewing app repos for a specific namespace.
In the next PR I plan to:
Quick example: