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

Add more structure to our directories, and document it #4851

Closed
bgrant0607 opened this issue Feb 26, 2015 · 30 comments
Closed

Add more structure to our directories, and document it #4851

bgrant0607 opened this issue Feb 26, 2015 · 30 comments

Comments

@bgrant0607
Copy link
Member

@bgrant0607 bgrant0607 commented Feb 26, 2015

Our code is hard to navigate, and github search isn't the greatest. It would help if the directory structure weren't so flat. I'm also not sure it makes sense for plugin to not be under pkg. #2306 is related, but separate.

Strawman:

  • master/
    • service/
    • nodecontroller/
    • replicationcontroller/
    • resourcequota/
    • scheduler/
    • storage/
      • etcd/
    • ui/
  • node/
    • kubelet/
    • proxy/
  • cloudprovider/
  • client/
    • auth/
    • config/
    • lib/
    • kubectl/
  • common/
    • apischema/
      • internal/
      • public/
        • v1beta1/
        • v1beta2/
        • v1beta3/
    • apiserver/
    • apiframework/
      • api/: generic files from pkg/api
      • meta/
      • conversion/
      • registry/ (generic CRUD)
      • runtime/
      • version/
      • watch/
    • auth/
    • util/
      • labels/
      • probe/
      • resource/
      • types/

@smarterclayton @lavalamp @thockin

@lavalamp
Copy link
Member

@lavalamp lavalamp commented Feb 26, 2015

Hm, I'd actually instead consider putting the controllers under plugin along with scheduler. client/ is used by multiple packages, so I think it should go in common. conversion/ is meant to be generic, so that should go under util.

  • master/
    • service/
    • resourcequota/
    • storage/
      • etcd/
    • ui/
  • node/
    • kubelet/
    • proxy/
  • plugin/
    • scheduler/
    • nodecontroller/
    • replicationcontroller/
  • cloudprovider/
  • kubectl/
  • common/
    • apischema/
      • internal/
      • public/
        • v1beta1/
        • v1beta2/
        • v1beta3/
    • apiserver/
    • apiframework/
      • api/: generic files from pkg/api
      • meta/
      • registry/ (generic CRUD)
      • runtime/
      • version/
      • watch/ (Maybe util/ is a better home? I need to see if this grew specific stuff.)
    • auth/ (what is this auth? how does it differ from the client/auth/?)
    • client/
      • auth/
      • config/
      • lib/
    • util/
      • labels/
      • probe/
      • resource/
      • types/
      • conversion/
@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Feb 26, 2015

Possible counter strawman - send people to the godoc page, which should display nice paragraph summaries of each package (and flog folks to add nice descriptions where there aren't)?

On Feb 26, 2015, at 1:12 PM, Brian Grant notifications@github.com wrote:

Our code is hard to navigate, and github search isn't the greatest. It would help if the directory structure weren't so flat. I'm also not sure it makes sense for plugin to not be under pkg. #2306 is related, but separate.

Strawman:

master/
service/
nodecontroller/
replicationcontroller/
resourcequota/
scheduler/
storage/
etcd/
ui/
node/
kubelet/
proxy/
cloudprovider/
client/
auth/
config/
lib/
kubectl/
common/
apischema/
internal/
public/
v1beta1/
v1beta2/
v1beta3/
apiserver/
apiframework/
api/: generic files from pkg/api
meta/
conversion/
registry/ (generic CRUD)
runtime/
version/
watch/
auth/
util/
labels/
probe/
resource/
types/
@smarterclayton @lavalamp @thockin


Reply to this email directly or view it on GitHub.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Feb 26, 2015

godoc is also necessary, but I don't see how it solves the problem of needing to look at the godoc for a zillion directories to find something.

@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Feb 27, 2015

I guess I had not viewed it as a problem so far. I don't object to it being "fixed".

On Feb 26, 2015, at 5:20 PM, Brian Grant notifications@github.com wrote:

godoc is also necessary, but I don't see how it solves the problem of needing to look at the godoc for a zillion directories to find something.


Reply to this email directly or view it on GitHub.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Feb 27, 2015

Of course, the person who wrote all the code doesn't need help finding anything. :-)

@satnam6502
Copy link
Contributor

@satnam6502 satnam6502 commented Feb 27, 2015

Where can we put source (Dockerfiles etc.) for the images we create (e.g. for logging)? It would be good to have one place where all the sources for our images are located. Currently for logging they are under contrib and cluster.

@satnam6502
Copy link
Contributor

@satnam6502 satnam6502 commented Feb 27, 2015

As someone who did not write all that code, I find it very hard to find anything. I too often end up running grep -Ri xxx or find . -name xxxx from the root directory.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Feb 27, 2015

#2992 would also facilitate grepping.

@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Feb 27, 2015

On Feb 26, 2015, at 2:23 PM, Daniel Smith notifications@github.com wrote:

Hm, I'd actually instead consider putting the controllers under plugin along with scheduler. client/ is used by multiple packages, so I think it should go in common. conversion/ is meant to be generic, so that should go under util.

master/
service/
resourcequota/
storage/
etcd/
ui/
node/
kubelet/
proxy/
plugin/
scheduler/
nodecontroller/
replicationcontroller/
cloudprovider/
kubectl/
common/
apischema/
internal/
public/
v1beta1/
v1beta2/
v1beta3/
apiserver/
apiframework/
api/: generic files from pkg/api
Hrm - I do think there are api consumption characteristics that don't cross over cleanly. Agree this whole package could do with some reorg.
meta/
registry/ (generic CRUD)
runtime/
version/
watch/ (Maybe util/ is a better home? I need to see if this grew specific stuff.)
auth/ (what is this auth? how does it differ from the client/auth/?)
It's the server side as opposed to client. I think client auth is much more consumer focused, whereas this is almost all server, so very little common code.
client/
auth/
config/
lib/
util/
labels/
probe/
resource/
types/
conversion/

Reply to this email directly or view it on GitHub.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Feb 27, 2015

@lavalamp While we have many components that are also clients, and we want users to be able to write similar components, I don't want to nest client code with server code in common. I want users who want to use the client to be able to easily find the relevant code without wading through our internals. Imagine we had build visibility.

Also, I've said this before, but I'd like to ensure that the general-purpose client functionality of kubectl to be usable as a client library, not just as a command-line tool.

I guess the node controller could be a plugin, though I'm not sure I find that case compelling.

@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Feb 27, 2015

After taking a look at the code structure again, I withdraw my hasty assertion. Definitely could benefit from a good clean up. Registry as a term needs to be yanked and replaced by storage or rest (or reststorage) consistently, and Dans master node structures would be good. We should make a decision on structure by type (things to do with pods, nodes, scheduling) or by function (master, node, controllers, scheduling, storage).

On Feb 26, 2015, at 8:57 PM, Satnam Singh notifications@github.com wrote:

As someone who did not wrote all that code, I find it very hard to find anything. I too often end up running grep -Ri xxxm or find . -name xxxx from the root directory.


Reply to this email directly or view it on GitHub.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Feb 27, 2015

Good point: #1744.

@thockin
Copy link
Member

@thockin thockin commented Feb 27, 2015

We should define exactly what we are optimizing for. Is it maximum
separation between logically distinct modules? Is it to minimize the
number of directories at each level? Is it to minimize depth of paths?

Proposed: optimize for obviousness, grouping by publicness and grouping by
genericness, in that order. For example, I think there's value in putting
distance between the internal API and the external APIs. We don't, in
general, want public consumers importing out internal objects. It will
alsmo make us think hard about things like conversion. Think how go users
will import things and try to make the layout sane for that.

To Daniel: as much as I agree that scheduler and controllers operate as
plugins, I'm not sure that warrants a directory - I think it does make them
harder to find. I could maybe argue for distinguishing "core" and "plugin"
or "primary" and "secondary" interfaces, but I would keep them close.

Random questions about choices herein.

Will cmd/* continue as-is?

I would maybe offer:

  • put kubectl in a top-level dir? Not sure
  • put api/ in a top-level dir, but only the versioned ones.
  • I'd like a place, maybe common/util or pkg/util or /util that holds
    packages which have NO kubernetes deps - so that we can sort of think of it
    as our extensions to standard libraries. Things like iptables, mount, etc
    are clear. I think labels are a grey area, and I think conversions are a
    clear tie to kubernetes.

On Thu, Feb 26, 2015 at 10:12 AM, Brian Grant notifications@github.com
wrote:

Our code is hard to navigate, and github search isn't the greatest. It
would help if the directory structure weren't so flat. I'm also not sure it
makes sense for plugin to not be under pkg. #2306
#2306 is
related, but separate.

Strawman:

  • master/
    • service/
    • nodecontroller/
    • replicationcontroller/
    • resourcequota/
    • scheduler/
    • storage/
      • etcd/
        • ui/
        • node/
    • kubelet/
    • proxy/
      • cloudprovider/
  • client/
    • auth/
    • config/
    • lib/
    • kubectl/
      • common/
    • apischema/
      • internal/
      • public/
        • v1beta1/
        • v1beta2/
        • v1beta3/
        • apiserver/
    • apiframework/
      • api/: generic files from pkg/api
      • meta/
      • conversion/
      • registry/ (generic CRUD)
      • runtime/
      • version/
      • watch/
        • auth/
    • util/
      • labels/
      • probe/
      • resource/
      • types/

@smarterclayton https://github.com/smarterclayton @lavalamp
https://github.com/lavalamp @thockin https://github.com/thockin

Reply to this email directly or view it on GitHub
#4851.

@erictune
Copy link
Member

@erictune erictune commented Mar 4, 2015

grep works fine for me regardless of the directory structure.

On Thu, Feb 26, 2015 at 9:01 PM, Tim Hockin notifications@github.com
wrote:

We should define exactly what we are optimizing for. Is it maximum
separation between logically distinct modules? Is it to minimize the
number of directories at each level? Is it to minimize depth of paths?

Proposed: optimize for obviousness, grouping by publicness and grouping by
genericness, in that order. For example, I think there's value in putting
distance between the internal API and the external APIs. We don't, in
general, want public consumers importing out internal objects. It will
alsmo make us think hard about things like conversion. Think how go users
will import things and try to make the layout sane for that.

To Daniel: as much as I agree that scheduler and controllers operate as
plugins, I'm not sure that warrants a directory - I think it does make them
harder to find. I could maybe argue for distinguishing "core" and "plugin"
or "primary" and "secondary" interfaces, but I would keep them close.

Random questions about choices herein.

Will cmd/* continue as-is?

I would maybe offer:

  • put kubectl in a top-level dir? Not sure
  • put api/ in a top-level dir, but only the versioned ones.
  • I'd like a place, maybe common/util or pkg/util or /util that holds
    packages which have NO kubernetes deps - so that we can sort of think of it
    as our extensions to standard libraries. Things like iptables, mount, etc
    are clear. I think labels are a grey area, and I think conversions are a
    clear tie to kubernetes.

On Thu, Feb 26, 2015 at 10:12 AM, Brian Grant notifications@github.com
wrote:

Our code is hard to navigate, and github search isn't the greatest. It
would help if the directory structure weren't so flat. I'm also not sure
it
makes sense for plugin to not be under pkg. #2306
#2306 is
related, but separate.

Strawman:

  • master/
  • service/
  • nodecontroller/
  • replicationcontroller/
  • resourcequota/
  • scheduler/
  • storage/
  • etcd/
  • ui/
  • node/
  • kubelet/
  • proxy/
  • cloudprovider/
  • client/
  • auth/
  • config/
  • lib/
  • kubectl/
  • common/
  • apischema/
  • internal/
  • public/
  • v1beta1/
  • v1beta2/
  • v1beta3/
  • apiserver/
  • apiframework/
  • api/: generic files from pkg/api
  • meta/
  • conversion/
  • registry/ (generic CRUD)
  • runtime/
  • version/
  • watch/
  • auth/
  • util/
  • labels/
  • probe/
  • resource/
  • types/

@smarterclayton https://github.com/smarterclayton @lavalamp
https://github.com/lavalamp @thockin https://github.com/thockin

Reply to this email directly or view it on GitHub
#4851.


Reply to this email directly or view it on GitHub
#4851 (comment)
.

@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Mar 4, 2015

The great organizers vs searchers debate - let it begin. I have a folder with every file I've ever used in it and do a search/find whenever I need something :)

----- Original Message -----

grep works fine for me regardless of the directory structure.

On Thu, Feb 26, 2015 at 9:01 PM, Tim Hockin notifications@github.com
wrote:

We should define exactly what we are optimizing for. Is it maximum
separation between logically distinct modules? Is it to minimize the
number of directories at each level? Is it to minimize depth of paths?

Proposed: optimize for obviousness, grouping by publicness and grouping by
genericness, in that order. For example, I think there's value in putting
distance between the internal API and the external APIs. We don't, in
general, want public consumers importing out internal objects. It will
alsmo make us think hard about things like conversion. Think how go users
will import things and try to make the layout sane for that.

To Daniel: as much as I agree that scheduler and controllers operate as
plugins, I'm not sure that warrants a directory - I think it does make them
harder to find. I could maybe argue for distinguishing "core" and "plugin"
or "primary" and "secondary" interfaces, but I would keep them close.

Random questions about choices herein.

Will cmd/* continue as-is?

I would maybe offer:

  • put kubectl in a top-level dir? Not sure
  • put api/ in a top-level dir, but only the versioned ones.
  • I'd like a place, maybe common/util or pkg/util or /util that holds
    packages which have NO kubernetes deps - so that we can sort of think of it
    as our extensions to standard libraries. Things like iptables, mount, etc
    are clear. I think labels are a grey area, and I think conversions are a
    clear tie to kubernetes.

On Thu, Feb 26, 2015 at 10:12 AM, Brian Grant notifications@github.com
wrote:

Our code is hard to navigate, and github search isn't the greatest. It
would help if the directory structure weren't so flat. I'm also not sure
it
makes sense for plugin to not be under pkg. #2306
#2306 is
related, but separate.

Strawman:

  • master/
  • service/
  • nodecontroller/
  • replicationcontroller/
  • resourcequota/
  • scheduler/
  • storage/
  • etcd/
  • ui/
  • node/
  • kubelet/
  • proxy/
  • cloudprovider/
  • client/
  • auth/
  • config/
  • lib/
  • kubectl/
  • common/
  • apischema/
  • internal/
  • public/
  • v1beta1/
  • v1beta2/
  • v1beta3/
  • apiserver/
  • apiframework/
  • api/: generic files from pkg/api
  • meta/
  • conversion/
  • registry/ (generic CRUD)
  • runtime/
  • version/
  • watch/
  • auth/
  • util/
  • labels/
  • probe/
  • resource/
  • types/

@smarterclayton https://github.com/smarterclayton @lavalamp
https://github.com/lavalamp @thockin https://github.com/thockin

Reply to this email directly or view it on GitHub
#4851.


Reply to this email directly or view it on GitHub
#4851 (comment)
.


Reply to this email directly or view it on GitHub:
#4851 (comment)

@satnam6502
Copy link
Contributor

@satnam6502 satnam6502 commented Mar 4, 2015

One of the things I miss from my tenure at Microsoft (and many years beforehand) is Visual Studio which let me easily navigate vast swathes of C++ and have some clue about what was going on. I switched to vim yesterday with go-vim and YCM and tagbar but I still feel like a caveman.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Mar 6, 2015

Re. searching vs. organizing: People new to the project don't know what to search for, and some search terms get hits everywhere.

As the project grows, we'll need to pay attention to both -- neither will be sufficient on its own.

@davidopp
Copy link
Member

@davidopp davidopp commented Mar 17, 2015

I agree with Brian, this is a real problem, and grep doesn't help unless you know what to search for.

As a random aside, I think it's unfortunate that people don't comment at the file level (in addition to at the function and and package level). Code is not randomly distributed into files; it's helpful to know what is the organizing principle behind each file. Having Github directory listings show these file descriptions instead of the much less useful last commit description in the directory listings would be the icing on the cake...

Anyway, I think the directory structure Brian proposed at the beginning seems basically reasonable. As for plugin/ I think we should get rid of it. Just have directories for controllers, schedulers, auth, etc. directly under master.

@brendandburns
Copy link
Contributor

@brendandburns brendandburns commented Mar 23, 2015

I don't think that this makes the v1.0 cut.

@brendandburns brendandburns modified the milestones: v1.0-bubble, v1.0 Mar 23, 2015
@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Aug 18, 2015

The lack of any obvious structure is causing new code to be put into random places. With the ongoing API group work, we also desperately need a home for common API infrastructure that's independent of any particular API group.

Another stab at API-related directories (not exhaustive, probably some incorrect categorizations):

  • apiframework (everything that's resource-type-independent)
    • server
      • apiserver
      • auth
      • errors (from api/errors)
      • rest (from api/rest)
      • storage
      • httplog
    • client
      • auth
      • metadata (from api/meta)
      • other generic CRUD/REST/client packages
    • common
      • runtime
      • conversion
      • conversion and deepcopy generation
      • types
        • api/unversioned.go, unversioned Status, unversioned constants, etc.
        • version
      • watch
      • util
        • fieldpath
        • labels
        • types (from pkg/types)
        • jsonpath
        • strategicmerge
        • yaml
        • probably a number of other packages from pkg/util that are specific to our client-server infrastructure

We don't have to move all the directories at once.

cc @wojtek-t @krousey @caesarxuchao

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Aug 18, 2015

I guess since I/we want to use the type machinery for component configuration (similar to scheduler config, kubeconfig) as well as for the API, it would be worth making that another subdirectory under apiframework, distinct from the other random stuff in common.

@thockin
Copy link
Member

@thockin thockin commented Aug 18, 2015

I'd love to see pkgs that are used by one component live under that
component. pkg/proxy -> cmd/kube-proxy. pkg/master -> apiserver,
pkg/kubelet -> cmd/kubelet.

pkg/* should be things that are plausibly used across more than one cmd/*

On Mon, Aug 17, 2015 at 7:09 PM, Mike Danese notifications@github.com
wrote:

Second try:
https://docs.google.com/spreadsheets/d/1B4PZVrwcUMFd9e93zETI-NClVTtDdC8sLcnxHms1lUs/edit?usp=sharing


Reply to this email directly or view it on GitHub
#4851 (comment)
.

@mikedanese
Copy link
Member

@mikedanese mikedanese commented Aug 19, 2015

go1.5 adds internal/ package visibility constraints so we can enforce private packages (at least as long as we, and our downstreams, are using go build)

@thockin
Copy link
Member

@thockin thockin commented Aug 20, 2015

I'm far less concerned about hiding packages than I am about organizing
them. :) internal/ is cute, but I don't think we've done a good enough job
of breaking up the packages and grouping them into subtrees to do much with
it yet.

On Wed, Aug 19, 2015 at 3:50 PM, Mike Danese notifications@github.com
wrote:

go1.5 adds internal/ package visibility constraints so we can enforce
private packages (at least as long as we, and our downstreams, are using go
build)


Reply to this email directly or view it on GitHub
#4851 (comment)
.

@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Aug 20, 2015

Also, anything you move to internal/ probably needs to be verified
Openshift isnt using. Internal is cute but not our problem right now.

@bgrant0607 bgrant0607 added this to the v1.2-candidate milestone Sep 12, 2015
@bgrant0607 bgrant0607 removed this from the v1.2-candidate milestone Feb 19, 2016
@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Jun 1, 2017

1.6 mostly completed the separation of these into better defined packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.