-
Notifications
You must be signed in to change notification settings - Fork 18
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
Redundant CephCSI
prefix in all the CRDs
#10
Comments
Regarding the kubectl ops, typing I also would like to direct your attention to this answer on reddit on the same subject: https://www.reddit.com/r/kubernetes/comments/vszgrw/crd_naming_convention_how_to_avoid_collisions/ |
@travisn @BlaineEXE any thoughts on this one as we are talking about naming conventions in a couple of places already? |
Personally, I understand the idea, but I think this will end up being a usability hassle for users. Consider that most users don't use fully qualified resource kinds when using kubectl. They just use Also consider that "driver" and "operators" are broad topics that might conflict with resource names that are core to the k8s project today or might be added in the future. "cephclusters" will conflict with the plural form of Rook's "cephcluster", which is going to contribute to user confusion, especially when these resources are deployed in the same ns as the Rook. We have already seen users get confused when there are multiple different API providers of the StorageCluster type. I think that these issues are guaranteed to happen if this decision is made. Travis also shared an anecdote with us in Rook huddle today about users becoming confused in the early days of Rook when CephCluster was called "Cluster." There are 2 cases of prior proof to show that users will be confused by this choice. From the outside, it seems to me like this is an attempt to save some characters on screen or on the commandline without consideration for how users will be negatively impacted. This might have made sense in the 80s when TTYs were limited to 80 characters wide, but in the modern era, unnecessarily vague, short naming is unnecessary and is considered an anti-pattern. I am adamantly, 100% against this idea on the grounds of its experientially-proven poor user experience. [Update] I understand that name collisions in k8s cannot be guaranteed to not happen. That is why the api version can be given. But this just seems like a decision to be deliberately obtuse and cause users headache when we could easily give a name that has a low possibility of collision without users having to change their normal workflows. Like, if StorageCluster has a collision, that is a bummer, but at least StorageCluster is a reasonably clear name even without the API component. Just "Driver" is unnecessarily vague and will confuse users, I guarantee it. |
This needs more discussion, I'd suggest it be reopened... I do see an argument for not putting a prefix on all the types with the assumption that we don't normally expect a naming collision with the short name. While we can encourage in the docs to use a more qualified name, neither do we want a common scenario either for a collision. It is extremely confusing if you run
If the prefix is removed, and updating CRD names based on suggestions in #8, the CRD names might be:
While Ultimately the naming is up to us. There is nothing on K8s naming conventions that addresses the naming conflicts. @nb-ohad Do you see any other discussions on this thread besides the one reddit link? I didn't find others, so it doesn't seem like a standard one way or the other. |
My preference would be to not provide any optional short-names for these CRDs. Users are not expected to interact with them much, so it isn't nice to clutter their kubectl search/auto-completion with these names either. That also makes it much simpler to have The main drawback would be the extra typing for a few administrators and developers while testing. I think that the priority should be on the users that deploy applications, not the other two groups. |
This is done in #62 closing it |
makefile: update makefile to add additional SA to csv
Naming of APIs in design doc so far is
The group we chose is
csi.ceph.io
, first point, do we need to haveCephCSI
prefix in all the CRDs? It'll be repeated in Kind/Resource and also in Group. Second, most of these APIs are mentioning a config and we are deriving desired state out of it, as such should we again haveConfig
in the name as an abstraction?After a rename the APIs will read as (in GR form)
The only con (technically) that I can think of is during kubectl ops, we might be forced to type upto non-overlapping GR (unless we could use shortName) if there is a conflicting Kind/Resource. Since everything is a config, could we make group as
config.csi.ceph.io
?The text was updated successfully, but these errors were encountered: