anuket-project / anuket-specifications

Anuket specifications
https://docs.anuket.io
123 stars 117 forks source link

[RA-1 All] Consistency review of current chapters #1574

Closed pgoyal01 closed 4 years ago

pgoyal01 commented 4 years ago

E2E review.

walterkozlowski commented 4 years ago

@pgoyal01, I started reviewing and I have found quite a few issues with Chapter 1 Overview, that I think it is better if I document them here, before passing to the next chapters. I tried to pretend to be a newcomer without any prior knowledge about this document or other CNTT documents.
@rabi-abdel and @kedmison you may be interested in these observations as well, e.g. because of some unclear statements about versions and some issues with RM Principles which I discovered on my way. Here we go: Chapter 1 Overview

Comment #1: Acronyms and terminology: There is no information or consistency regarding the first use of acronyms: in some cases they are explained (e.g. QoS in the 1.2 “Performance tuning” use case) but most of them are not (e.g. GRE, MLAG in 1.3 “Overlay networks”). Most of them are very well known in the industry but we need to have a more consistent approach. I suggest either define the acronym when it is first used in this document, or (better) make a comment at the beginning of the document that acronyms are defined in a glossary and provide a link to it. In some special cases (I would not include QoS into this category) it may be done both ways; for example, when this is a new acronym introduced into the industry by this document.
Looks like there are places where such definitions may be defined, Sec 1.3 Terminology refer to them but both links CNTT Reference Model and RA-1 Terminology DO NOT WORK (error 404).

Comment #2: Principles Sec 1.4 uses link CNTT Reference Model Principles to the one sentence long Sec 1.3 in https://github.com/cntt-n/CNTT/blob/master/doc/ref_model/chapters/chapter01.md#1.3 with a link https://github.com/cntt-n/CNTT/blob/master/doc/ref_model/chapters/chapter01.md#1.3 when clicking on the latter on we land in Chapter 2 Principles in RM. It seems quite a convoluted way and the reader ends up two links from the original text, probably at lost. A side comment: Sec 2.1 Overall Principles in RM need badly an update and refresh; e.g. Principle 9 explicitly forbids the use of SR-IOV (agreed when it uses PCI-PT but a nuance people may and will miss) while we have an exception process, which is not mentioned there. And few other issues (several of the principles are formulated like requirements not like principles, the text must be quite old by now). Sec 1.4 uses also another link CNTT Reference Architecture Principles which takes us back to the GitHub landing page with links to two reference architectures (Openstack Based and Kubernetes Based); I think that most readers will be completely lost; also it is circular reference because selecting Openstack we are back in the document we started but in a different place.. Also, Sec 1.4: a minor typo: in bullet points describing Four Opens “design” is the only one which uses a small not a capital as the first character. Btw: It seems we do not have any consistency writing OpenStack in some (most?) places and Openstack in others (see above).

Comment #3: OpenStack release Sec 1.5 CNTT OpenStack Release: Shouldn’t it be “This Reference Architecture document in its current version conforms to the OpenStack Pike release.” Rather than the current “This Reference Architecture document conforms to the OpenStack Pike release.” Later on we read ‘For ease, this CNTT Reference Architecture version can be referred to as "RA-1 OSTK Pike."’ We need to be clear whether we are talking about the document, about the architecture or about a version (of what? Of a document? Or of Architecture?).

Comment #4: Document Organisation The first sentence of Sec 1.6 is “The Reference Architecture requirements and the traceability where in this document the requirement is addressed is documented in Chapter 2.” contains a typo “is addressed is documented”. The traceability part of this sentence is a bit unclear, particularly as in Chapter2 Sec 2.2 “Reference Model Requirements” we have only one sentence “Traceability to Reference Model.” and that’s it! Back to the first sentence of Sec 1.6: I would propose something like “ Chapter 2 defines the Reference Architecture requirements and, when appropriate, provides references to where these requirements are addressed in this document.” Typo: “items, Please note that”, should be “items. Please note that”. The sentence “Chapter 8 identifies certain Gaps that currently exist and plans on how to address them.” is a bit misleading as Chapter 8 only make a promise to do such plans saying “Once gaps are identified, the next step will be to propose a plan to address these gaps.” and indeed Chapter 8 does not provide such plans yet (except of some hints what might be possible or not, in some cases; no such hints in other cases). Btw: why “Gaps” in Sec 1.6 (capital “G”) and “gaps” (small “g”) in Chapter 8?

rabi-abdel commented 4 years ago

Really Great review @walterkozlowski Added @bfcohen to the conversation to see if we can get some of those fixed for baldy as well.

pgoyal01 commented 4 years ago

@walterkozlowski Great review and feedback. I have fixed the links issue although the Terminology file is currently empty.

Have fixed all of the identified issues in Ch01.

W.r.t. "Traceability to Reference Model" -- the requirements in RM need to be made consistent in terms of their numbering scheme, duplicates deleted, etc. There are issues open in both RM and RA-1 (for when RM fixes their requirements) to fix the same. I had provided examples of many issues in the RM chapters months ago but they are yet to be addressed.

Thanks for great feedback.

pgoyal01 commented 4 years ago

@rabi-abdel can the corrections - PR #1595 (and linked issue $1594) - during proof reading be included in the Baldy release? Proof reading updates can be made until midnight UTC

This Issue #1574 can continue to be used for the E2E review.

walterkozlowski commented 4 years ago

@pgoyal01 @bfcohen @rabi-abdel E2E review - next installment (it will take a while..)

Chapter 2 Architecture Requirements ( sec 2.1 - 2.3.1) Comment #1 Section 2.1 Introduction What does note “RFC2119” mean? It is unclear what is the difference between Requirements using “may” (or “may not”) and Recommendations. Section 2.1 says: may: Requirements that are marked as may are considered optional. The same applies to may not. This chapter includes both "Requirements" that must be satisfied in an RA-1 conformant implementation and "Recommendations" that are optional for implementation. Was the intent here to distinguish between Requirements that are traceable within this document and Recommendations that are not (are of more general nature)? Some Requirements though are stated like general Recommendations and are not traceable to anything tangible within RA-1 document, e.g. req.gen.rsl.01 Resiliency.

Comment #2 Section 2.2. Reference Model Requirements This section contains only 4 words (Traceability to Reference Model.). What is the real purpose of this section? It needs to be clarified/fixed.

Comment #3 Sec 2.3.1 General Requirements Requirement “req.gen.rsl.01 Resiliency” without traceability or clarification is not testable. In the current form it is rather a general Recommendation (see Comment #1).

Comment #4 Sec 2.3.1 Infrastructure Requirements Requirement “req.inf.com.05” (NUMA), the traceability link does not work (Error 404) Requirement “req.inf.com.07” (different hardware configurations), the traceability link does not take the reader to Sec 3.3.3 as promised but to Chapter 3; Looks like Sec 3.3.3 has now a different title than in the traceability column for this requirement. Shouldn’t be this link rather traceable to 4.4.1? Requirement “req.stg.com.04” (SDS) Are we saying (by linking to 4.2.4.1) that if someone wants to uses SDS (this is “may” requirement) they must use Ceph? I am not questioning if this is the case (as OpenStack definitely recommends Ceph) but this is actually unclear. Are we allowing other SDS like Gluster? I would not think so. We need to make it very clear. Requirement “req.stg.com.05” is missing. Need to add it with a note “Deprecated” (or equivalent). Unless of course it is a genuine requirement which has been lost somewhere. Requirement “req.stg.com.07” similar comment to “req.stg.com.04” Requirement “req.inf.com.15” The link to Sec 4.2.3.4 does not work, it looks like this section change its name Requirement “req.inf.acc.01” “The Architecture should support Application Specific Acceleration (exposed to VNFs).” This must be confusing for the readers, and the text in 3.2.6 does not help. What are we really trying to say they “should” do here?

iangardner22 commented 4 years ago

Here is my review of the intro and chapter 1

iangardner22 commented 4 years ago

Here is my review of the intro and chapter 2

I move on to Ch3 and later when I find time.

iangardner22 commented 4 years ago

@pgoyal01 - is this progressing? Just wondering whether to have a look at CH3+4...