[RA2 Ch4]: Reinstate Chapter 4.5 content in new format #1854

CsatariGergely · 2020-07-30T16:43:52Z

Moving the original content of Chapter 4.5 to the new structure.

closes #1637

closes anuket-project#1637 Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>

pgoyal01 · 2020-07-30T16:50:18Z

@CsatariGergely Is this only to recapture previous content or can we add/delete?

pgoyal01 · 2020-07-30T17:42:37Z

@CsatariGergely Some suggestions -- were submitted previously and resubmitting for consideration.

RA2-Ch4-Networking-Specs-2020-07-17.docx

CsatariGergely · 2020-07-31T12:24:47Z

@CsatariGergely Is this only to recapture previous content or can we add/delete?

It is only a reorganisation of the original content. I would prefer to do additions and deletions in different pr-s.

CsatariGergely · 2020-07-31T12:33:19Z

@CsatariGergely Some suggestions -- were submitted previously and resubmitting for consideration.

RA2-Ch4-Networking-Specs-2020-07-17.docx

I would prefer to add only the original content in this pr and modify in subsequent pr-s if needed.

Still some comments to the document:

Multi networking: I would not say, that the network plumbing group's spec is a de facto standard as it has only one implementation so far.
Multi networking: Support of multiple IPAM-s is in direct contradiction with req.inf.ntw.10
SR-IOV Device Pluing and SR-IOV CNI: I have them now in the same requirement. Should I separate them to two? Also we need to be more specific wihich SR-IOV CNI do we mean

pgoyal01 · 2020-07-31T16:32:27Z

@CsatariGergely The standard is based on the charter of the plumbing group and its adoption by kubernetes - and as you know implementations lag behind definition of a standard. A standard is a standard even without an implementation.

Re: Multi networking: Support of multiple IPAM-s is in direct contradiction with req.inf.ntw.10
req.inf.ntw.10 states that for a particular cluster there should be only one IPAM but it doesn't state that the mult-networking solution should only support a particular IPAM solution. We may need a requirement that the multi-networking solution must support multiple IPAM solutions and thus allowing an operator to chose the IPAM that they want to utilise.

IMO, we should split the SR-IOV Device Pluing and SR-IOV CNI requirements.

Separating SR-IOV CNI and SR-IOV device plugin requirements. Changing may-s to must-s to be in alignement with requirement wording. Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>

CsatariGergely · 2020-08-04T12:37:49Z

@CsatariGergely The standard is based on the charter of the plumbing group and its adoption by kubernetes - and as you know implementations lag behind definition of a standard. A standard is a standard even without an implementation.

There is no adoption by Kubernetes. If there would be, then we would not need metaplugins.

Re: Multi networking: Support of multiple IPAM-s is in direct contradiction with req.inf.ntw.10
req.inf.ntw.10 states that for a particular cluster there should be only one IPAM but it doesn't state that the mult-networking solution should only support a particular IPAM solution. We may need a requirement that the multi-networking solution must support multiple IPAM solutions and thus allowing an operator to chose the IPAM that they want to utilise.

Okay, so you mean that we should have a requirement which allows the solution to select from multiple IPAM solutions? With which higher level requirement can we back this requirement?

IMO, we should split the SR-IOV Device Pluing and SR-IOV CNI requirements.

Done

doc/ref_arch/kubernetes/chapters/chapter04.md

pgoyal01 · 2020-08-04T13:03:22Z

doc/ref_arch/kubernetes/chapters/chapter04.md

-|`ra2.ntw.003`|NAT less connectivity|IPVLAN CNI component of DANM or the [MACVLAN CNI](https://github.com/containernetworking/plugins/tree/master/plugins/main/macvlan) may be used||
-|`ra2.ntw.004`|User plane networks|[User Space CNI](https://github.com/intel/userspace-cni-network-plugin) may be used. The User Space CNI may use VPP or OVS-DPDK as a backend.||
-|`ra2.ntw.005`|SR-IOV|SR-IOV CNI plugin and the [SR-IOV Device Plugin](https://github.com/intel/sriov-network-device-plugin) may be used||
+|`ra2.ntw.001`|CNI multiplexer/metaplugin|As the selected CNI multiplexer/metapulgin MUST support other CNI plugins (`req.inf.ntw.06`) and SHOULD provide an API based solution to administer the networks from a central point (`req.inf.ntw.03`) the selected CNI multiplexer/metapulgin must be [DANM](https://github.com/nokia/danm).| [req.inf.ntw.06](./chapter02.md#23-kubernetes-architecture-requirements), [req.inf.ntw.03](./chapter02.md#23-kubernetes-architecture-requirements) |


How did DAMN become a "must" from a "may" -- before the restructuring exercise there was discussion and both MULTUS and DAMN were to be supported. Secondly, if we are changing the content in this PR then other changes, namely, the Plumbing group recommendations etc. can be out of scope of this PR.

Back in Prague we agreed that we should harden the requirements as this the purpose of making an RA. I've forgot about the language change, that's why applied it in this change. If this does not fit into the restructuring work I can revert those changes.

Yes there were discussions to support additional CNI multiplexers on top of DANM, but there was never a pr created to get an agreement on those changes.

Can you explain a bit more the last comment about the Plumbing group recommendations?

We should keep restructuring free of changes with regards to requirement content or compliance statements such as "may", "must", .... It's otherwise getting tricky with reviews.

Post Prague we have had discussion on this in a previously open PR including language suggested by Taylor.

The Table needs other rows -- as we need to add other requirements, for example, conformance to a standard CRD, support for any CNI plugin, etc. etc.

From, Network Plumbing Working Group
The Network Plumbing Working Group (NPWG) was formed at Kubecon NA '17 in Austin to initially work towards implementing a common standard addressing how one may attach multiple networks to pods in Kubernetes. Our work has culminated in a de-facto standard CRD.

Add DANM and Multus to the RI-2 vs the RA-2. The RA-2 should be about the requirements which implementations such as DANM and Multus can provide solutions for.

See above comments - I think we need to very explicitly document the pros/cons of each approach and document which we take. #1854 (comment)

pgoyal01 · 2020-08-05T21:00:59Z

doc/ref_arch/kubernetes/chapters/chapter04.md

-|`ra2.ntw.005`|SR-IOV|SR-IOV CNI plugin and the [SR-IOV Device Plugin](https://github.com/intel/sriov-network-device-plugin) may be used||
+|`ra2.ntw.001`|CNI multiplexer/metaplugin|As the selected CNI multiplexer/metapulgin MUST support other CNI plugins (`req.inf.ntw.06`) and SHOULD provide an API based solution to administer the networks from a central point (`req.inf.ntw.03`) the selected CNI multiplexer/metapulgin must be [DANM](https://github.com/nokia/danm).| [req.inf.ntw.06](./chapter02.md#23-kubernetes-architecture-requirements), [req.inf.ntw.03](./chapter02.md#23-kubernetes-architecture-requirements) |
+|`ra2.ntw.002`|CNI to implement a default network which implements the Kubernetes network model|[Calico](https://github.com/projectcalico/cni-plugin) must be used based on the requirement `req.inf.ntw.08` due to it's capability to handle `NetworkPolicies`|[req.inf.ntw.08](./chapter02.md#23-kubernetes-architecture-requirements)|
+|`ra2.ntw.003`|NAT less connectivity|IPVLAN CNI component of DANM or the [MACVLAN CNI](https://github.com/containernetworking/plugins/tree/master/plugins/main/macvlan) must be used||


similar to other comments why can't another IPVLAN CNI be used?

As the IPVLAN CNI is not visible for the applications I'm okay to recommend "an IPVLAN CNI".

Same issue as ra2.ntw.001 for the CNI #1854 (comment)

Add DANM and Multus to the RI-2 vs the RA-2. The RA-2 should be about the requirements which implementations such as DANM and Multus can provide solutions for.

In this case this comment is valid as the IPVLAN CNI is not visible in the application artifacts. If we all agree I can change this in this pr to a generic IPVLAN CNI.

Co-authored-by: Tom Kivlin <52716470+tomkivlin@users.noreply.github.com>

Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>

However I believe that RA shold define hard requirements for components visible for the software or the deployment artefacts of CNF-s and once we made this decision in Prague I change the musts back to mays as there are some concerns about this change in pr anuket-project#1854. The same changes will be submitted in a different pr. Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>

CsatariGergely · 2020-08-06T15:12:51Z

RA2 Meeting 2020.08.06: @CsatariGergely to change back the "may"-s to their original form.

CsatariGergely · 2020-08-07T08:16:39Z

@CsatariGergely I believe @peterwoerndle was asking not to make any changes from the original "must's" and "may's) ... "It's otherwise getting tricky with reviews"
@peterwoerndle stated,
We should keep restructuring free of changes with regards to requirement content or compliance statements such as "may", "must", .... It's otherwise getting tricky with reviews.

I've double checked the may-s and I believe they are in their original form now. Can you please @peterwoerndle and @pgoyal01 doublecheck this?

peterwoerndle · 2020-08-10T13:05:33Z

Looks to me like the requirement statements have been reverted to the original statements. Good for me to merge. As @CsatariGergely mentioned in the call we're still missing an issue or PR to have Multus as another may requirement.

tomkivlin

I have opened #1874 to address the question of whether we want to support / enforce the consistency of deployment artefacts as well as software / container images. Agree with @CsatariGergely to deal with this separately, get this wording in for now and address the additional requirements that @pgoyal01 has created an issue for in subsequent PRs.

tomkivlin · 2020-08-18T20:18:45Z

It's been a week since any further comments on this. @taylor @pgoyal01 could you review the current state and approve/feedback please?

pgoyal01 · 2020-08-18T20:25:46Z

@tomkivlin Before the restructuring, if I remember correctly, we had agreed to list both MULTUS and DANM as choices - @taylor had even provided language to that effect. If that is not to be addressed in the restructuring then we should open an issue for it.

pgoyal01

Approved subject to changes to be made w.r.t. the metaplugins to be mentioned in the text.

tomkivlin · 2020-08-18T20:27:15Z

@tomkivlin Before the restructuring, if I remember correctly, we had agreed to list both MULTUS and DANM as choices - @taylor had even provided language to that effect. If that is not to be addressed in the restructuring then we should open an issue for it.

Ah yes I remember. @CsatariGergely could you address this please?

tomkivlin · 2020-08-21T11:33:41Z

@tomkivlin Before the restructuring, if I remember correctly, we had agreed to list both MULTUS and DANM as choices - @taylor had even provided language to that effect. If that is not to be addressed in the restructuring then we should open an issue for it.

Ah yes I remember. @CsatariGergely could you address this please?

@CsatariGergely this was the proposed wording: #1532 (comment). Are you OK updating the wording in this PR, or shall I create a separate issue?

tomkivlin · 2020-08-25T09:56:59Z

#1911 created as per @pgoyal01 comments above.

[RA2 Ch4]: Rework of Chapter 4.5

a70240c

closes anuket-project#1637 Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>

CsatariGergely requested review from taylor, pgoyal01 and tomkivlin July 30, 2020 16:43

CsatariGergely requested a review from scottsteinbrueck as a code owner July 30, 2020 16:43

CsatariGergely added this to In Progress in old-RA2 via automation Jul 30, 2020

pgoyal01 changed the title ~~[RA2 Ch4]: Rework of Chapter 4.5~~ [RA2 Ch4]: Reinstate Chapter 4.5 content in new format Jul 31, 2020

[RA2 Ch4]: Corrections after restructuring

408f6cf

Separating SR-IOV CNI and SR-IOV device plugin requirements. Changing may-s to must-s to be in alignement with requirement wording. Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>

tomkivlin suggested changes Aug 4, 2020

View reviewed changes

doc/ref_arch/kubernetes/chapters/chapter04.md Outdated Show resolved Hide resolved

doc/ref_arch/kubernetes/chapters/chapter04.md Outdated Show resolved Hide resolved

doc/ref_arch/kubernetes/chapters/chapter04.md Outdated Show resolved Hide resolved

pgoyal01 reviewed Aug 4, 2020

View reviewed changes

pgoyal01 reviewed Aug 5, 2020

View reviewed changes

CsatariGergely and others added 2 commits August 6, 2020 14:52

[RA2 Ch4]: Typo correction

57b2834

Co-authored-by: Tom Kivlin <52716470+tomkivlin@users.noreply.github.com>

[RA2 Ch4]: Corrections of review comments for pr1854

adb2294

Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>

CsatariGergely requested review from tomkivlin and pgoyal01 August 6, 2020 13:55

CsatariGergely requested a review from peterwoerndle August 7, 2020 08:16

peterwoerndle approved these changes Aug 10, 2020

View reviewed changes

tomkivlin approved these changes Aug 10, 2020

View reviewed changes

pgoyal01 approved these changes Aug 18, 2020

View reviewed changes

tomkivlin merged commit e051746 into anuket-project:master Aug 25, 2020

old-RA2 automation moved this from In Progress to Done Aug 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RA2 Ch4]: Reinstate Chapter 4.5 content in new format #1854

[RA2 Ch4]: Reinstate Chapter 4.5 content in new format #1854

CsatariGergely commented Jul 30, 2020

pgoyal01 commented Jul 30, 2020

pgoyal01 commented Jul 30, 2020

CsatariGergely commented Jul 31, 2020

CsatariGergely commented Jul 31, 2020

pgoyal01 commented Jul 31, 2020

CsatariGergely commented Aug 4, 2020

pgoyal01 Aug 4, 2020

CsatariGergely Aug 6, 2020

peterwoerndle Aug 6, 2020

pgoyal01 Aug 6, 2020

taylor Aug 6, 2020

tomkivlin Aug 10, 2020

pgoyal01 Aug 5, 2020

CsatariGergely Aug 6, 2020

taylor Aug 6, 2020

CsatariGergely Aug 10, 2020

CsatariGergely commented Aug 6, 2020

CsatariGergely commented Aug 7, 2020

peterwoerndle commented Aug 10, 2020

tomkivlin left a comment

tomkivlin commented Aug 18, 2020

pgoyal01 commented Aug 18, 2020

pgoyal01 left a comment

tomkivlin commented Aug 18, 2020

tomkivlin commented Aug 21, 2020 •

edited

tomkivlin commented Aug 25, 2020

[RA2 Ch4]: Reinstate Chapter 4.5 content in new format #1854

[RA2 Ch4]: Reinstate Chapter 4.5 content in new format #1854

Conversation

CsatariGergely commented Jul 30, 2020

pgoyal01 commented Jul 30, 2020

pgoyal01 commented Jul 30, 2020

CsatariGergely commented Jul 31, 2020

CsatariGergely commented Jul 31, 2020

pgoyal01 commented Jul 31, 2020

CsatariGergely commented Aug 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CsatariGergely commented Aug 6, 2020

CsatariGergely commented Aug 7, 2020

peterwoerndle commented Aug 10, 2020

tomkivlin left a comment

Choose a reason for hiding this comment

tomkivlin commented Aug 18, 2020

pgoyal01 commented Aug 18, 2020

pgoyal01 left a comment

Choose a reason for hiding this comment

tomkivlin commented Aug 18, 2020

tomkivlin commented Aug 21, 2020 • edited

tomkivlin commented Aug 25, 2020

tomkivlin commented Aug 21, 2020 •

edited