On 07/17/2015 08:16 AM, Travis Rhoden wrote: > Hi Owen, > > I'm still giving this one some thought. I've gone back and reviewed > https://github.com/ceph/ceph-deploy/pull/320 a few more times. I do > understand how it works (it took a couple times through it), and > cosmetic things notwithstanding I can appreciate what it is doing. I > also fully get that the choice of sqlalchemy vs <choice of data store> > makes no difference to the merit of the idea. I'm still formulating > my opinion, however, but wanted to let you know I was thinking about > it. Thanks for this reply, but please don't get too focused on the patch. At the time of writing this patch I thought MVC would be completely uncontentious. It was never intended to illustrate the benefits of MVC. That is more work than this patch was intended to achieve. It was written to show a model can be practically done with a sqlalchemy implementation of an MVC's model with no important deployment overhead, and illustrate how SQL queries can be mapped easily as an advantage of using a RDBMS model, rather than illustrating MVC best practice. The rest of the email tries to show horn the patch into the discussion of the validity of MVC with respect to the patch: https://github.com/ceph/ceph-deploy/pull/320 Its a nights work and only a discussion point, its half existing code I already have (for rgw setup), and half MVC in its self. Please think of it more as an aid to a conversation (like slides) rather than as final or a good example of MVC best practice. It would need more work to be this, and its probably a days work to be close to a good example. The model is clear, but the views could be clearer in the way they are abstracted, and can be extended in consistent ways, so I made a few comment on the patch showing where I think the code is not very MVC. A good example in how its not MVC enough to be an MVC example is the "set" operations that should be in a controller method, and comparing data in the model, the data in the model should be loaded via views. In addition if the model is based on a RDBMS, the set operations would be performed by the RDBMS and not in python. The first post in this thread is more stand alone in where I would see us going if we went MVC. The development of the patch helped me see many places where we could add consistency by being more MVC, rather than actually following the design pattern enough to show best practice. I would be happy to chat about the code face to face but reviewing this code directly without comments does not show the benefits of MVC. In summary about this patch: (1) It is intended to be a point of interactive discussion, rather than left stand alone, and could do with more functions illustrating how similar these can be. (2) It is far from fully MVC, both as a discussion point it includes not very MVC code, partially to show how the code could be made more MVC, and as I time boxed the effort and run out of time. (3) It must be understood as its more correct in model, than it is in view separation, and controller clarity. (4) I rather enjoyed making a Relational model of all complete ceph clusters, and this code is more expanded than needed at this point (but it was fun). (5) In the relational model I have not followed complete "3rd normal form" as I feel that for cluster models in an MVC context its nice to have entities map closely with reality, (and help make touples map closely to tasks) rather than use more complexity, for the sake of less data in the database which I don't think is an important objective. (6) the model was also expanded to be more "complete" than needed for this discussion, in part for some new members of the SUSE ceph team to understand the constraints and structure of a ceph cluster deployment. Best regards Owen > > - Travis > > On Tue, Jul 14, 2015 at 4:24 AM, Owen Synge <osynge@xxxxxxxx> wrote: >> Dear all, >> >> ceph-deploy is to quote >> >> <quote> >> It is not a generic deployment system, it is only for Ceph, and is >> designed for users who want to quickly get Ceph running with sensible >> initial settings without the overhead of installing Chef, Puppet or Juju. >> >> It does not handle client configuration beyond pushing the Ceph config >> file and users who want fine-control over security settings, partitions >> or directory locations should use a tool such as Chef or Puppet. >> </quote> >> >> What we have currently in ceph-deploy is mixing cluster state discovery, >> cluster state modification, in many functions with no attempt to reuse >> state discovery code. I see no consistent design pattern, but can see >> code that is production grade. Being production grade code I oppose >> major code changes that might break functionality happening all at once. >> >> I propose with some small changes for finding out what is to be done, >> storing it in one place, finding out what state the cluster is and >> storing it in one place. Then comparing these two stored states, then >> making changes as needed to the clusters state as just general good >> practice. This is can be called MVC in design pattern terminology if all >> the state gathering code is separated from all the state changing code, >> these would then be View parts to MVC. The controller would be the >> component that directs the interaction between the model and the views >> which also should be separate. >> >> From experience I have found that MVC has 5 major benefits compared to >> the way ALL of the code base is in ceph-deploy for a project like >> ceph-deploy when I have made similar incremental changes toward this >> design pattern in the past on the YAIM project for deploying dcache. >> (which was more complex than ceph deployment and I had to write it in BASH) >> >> (1) Cluster state discovery code only has to be developed tested and >> used once as it is placed in model. >> >> (2) the MVC model can be nested, so potentially removing the need to >> iterate across nodes, in more than one place in the code base. >> >> (3) Making the code MVC will allow the model and the view that is >> involved in state gathering code to be reused the functionality and >> validation of ceph-deploy in general will improve siginifcantly. >> >> (4) The command line to desired model state, "view" code can be shared >> by all ceph-deploy components. >> >> (5) By discovering the state of the cluster before changing any cluster >> state, things such as needing a change of ceph.conf, which requires a >> flag argument to allow this, can be detected at the start of an >> operation so the user can get a clear and early error, rather than >> failing at a later stage for configuration. Making cluster configuration >> closer to atomic, and following the good practice of failing fast. >> >> For me this clearly sates why MVC is a good standard approach to cluster >> management that will reduce code duplication in many areas of the code >> base and simple way to keep the code base extensible. >> >> Considering we do not have this model, some may consider moving to this >> model a major re-factor. I think this is invalid because I think its >> reasonable to see it from this perspective: >> >> (A) we can start on an area of the code base that is under new >> development. eg rgw support in ceph-deploy. >> >> (B) I can see removing code duplication as a incremental process that >> will each have its own change. >> >> (C) I do not see the need to move all code to an MVC model at the same time. >> >> (D) I do not see splitting your code into discovery , storing state and >> applying changes to significantly increase the work for any one change. >> >> Now my points are made I have a couple of final misunderstandings to >> clear up in any discussion of MVC: >> >> (I) I would be happy for the model to be based upon python objects. >> >> (II) While I like the idea of using sqlalcamy and sqllite as a nice way >> to get a type safe model with firm constraints, I do not see this as the >> only way we can make a model for MVC. I demonstrated in a code sample >> with a draft of a complete relational model to ceph cluster deployment >> using sqllite in memory, that made use of the flexibility of setuptools >> the deployment issues people raised with this approach where not >> present. This said it is essential to understand I do not see this as >> more than a potential implementation, or in any way other than as a >> potential implementation, of a model for the use by the MVC design patten. >> >> So to summarise: >> >> I think we should go toward an MVC pattern in ceph-deploy for new >> development (such as rgw), in preference to code consistency across >> ceph-deploy. >> >> I think we should re-factor existing code to use a shared model in an >> MVC pattern only after the model has matured with new development work >> based upon it. >> >> I think even if ceph-deploy never gets any new functionality, beyond >> what is currently planned, moving to MVC will reduce the support burden >> in the long/medium term and potentially in the short term. >> >> Should further features ever be commissioned from ceph-deploy I think >> any work already done towards building an MVC pattern will be reused and >> result in less work. >> >> Best regards >> >> Owen >> -- >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 21284 (AG Nürnberg) Maxfeldstraße 5 90409 Nürnberg Germany -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
