[Mizar-Arktos-Integration]Tenant-vpc-support: finalize code changes for mizar pod controller and mizar arktos network controller after successful integration tests #1281

q131172019 · 2022-01-17T07:28:52Z

What type of PR is this?

/kind feature

What this PR does / why we need it:
In terms of the design document at https://github.com/CentaurusInfra/arktos/blob/poc-2022-01-30/docs/design-proposals/multi-tenancy/multi-tenancy-network-mizar-integration.md, this PR implements the following two functions:

Mizar-arktos-network-controller (existing controller) automatically creates Mizar CRDs - VPC and subnet, based on vpcID coming from corresponding default arktos network object, which is in Ready state, using Dynamic Client technology including Discovery REST mapper and Dynamic REST interface.

The json template files for Mizar CRDs - VPC and Subnet are located as default at the directory ./hack/runtime/.

./hack/runtime/default_mizar_network_subnet_template.json (the below is an example)

{
   "apiVersion": "mizar.com/v1",
   "kind": "Vpc",
   "metadata": {
      "name": "{{.Tenant}}-default-network"
   },
   "spec": {
      "ip": "21.0.0.0",
      "prefix": "16",
      "dividers": 1,
      "status": "Init"
   }
}

./hack/runtime/default_mizar_network_vpc_template.json (the below is an example)

{
   "apiVersion": "mizar.com/v1",
   "kind": "Subnet",
   "metadata": {
      "name": "{{.Tenant}}-default-network-subnet"
   },
   "spec": {
      "ip": "21.0.21.0",
      "prefix": "24",
      "bouncers": 1,
      "vpc": "{{.Tenant}}-default-network",
      "status": "Init"
   }
}

Mizar-pod-controller (existing controller) gets vpcID information from arktos network object in Ready state, automatically creates the following annotations, then patches the objects(pods) with correct annotations and updates to API server, finally notifies the annotations to Mizar via grpcAdaptor.

    mizar.com/subnet: <tenant name>-default-network-subnet
    mizar.com/vpc: <tenant name>-default-network

Which issue(s) this PR fixes:.
The new feature is part of Mizar Arktos integration.

Fixes # N/A

Special notes for your reviewer:

This PR is clean version based on working PR 1248, which is used for integration tests between arktos team and mizar team.

Test steps:
Note: These steps have been tested with system tenant in Carl's local dev environment which runs the codes in poc-2022-01-30 branch q131172019:CarlXie-poc-2022-01-30-tenant-vpc-support,

OS: AWS EC2 instance running Ubuntu 20.04
Protobuf-Compiler: 3.6.1 (sudo apt install -y protobuf-compiler; protoc --version)

Prerequisite step
1)Configure two template files of vpc and subnet
1. ./hack/runtime/default_mizar_network_subnet_template.json
  - change the parameters - ip, prefix, dividers based on above example
2. ./hack/runtime/default_mizar_network_vpc_template.json
  - change the parameters - ip, prefix, bouncers based on above example

1.2) Run script "hack/update-generated-mizar.sh'" to regenerate gRPC codes including the file pkg/controller/mizar/builtins.pb.go if you make the changes in the mizar grpc proto file pkg/controller/mizar/builtins.proto

1.3) Run command "CNIPLUGIN=mizar ./hack/arktos-up.sh" to start Arktos scale-up cluster with Mizar. Or follow up the steps 0.6) through step 1 at https://github.com/q131172019/arktos/blob/CarlXie_singleNodeArktosCluster/docs/setup-guide/single-node-dev-scale-up-cluster-with-Mizar.md to ensure Mizar CRDs and pods are in Running state, especially check whether CRDs - vpc (system-default-network) and subnet (system-default-network-subnet) as well as dividers and bouncers of tenant 'system' are created correctly based on above two template files of VPC and Subnet .

./cluster/kubectl.sh get pods
./cluster/kubectl.sh get crds
./cluster/kubectl.sh get vpcs
./cluster/kubectl.sh get dividers
./cluster/kubectl.sh get subnets
./cluster/kubectl.sh get bouncers
./cluster/kubectl.sh get droplets
./cluster/kubectl.sh get endpoints.mizar.com
./cluster/kubectl.sh get network -AT

Test whether system tenant's two nginx pods can talk with each other via 'curl'
2.1) Use the following yaml file to create two nginx pods of tenant 'system'
~/TMP/mizar/pod-vpc-1-without-annotation.yaml

apiVersion: v1
kind: Pod
metadata:
  name: ying-nginx-system-1
  tenant: system
spec:
  containers:
  - name: nginx
    image: nginx
    ports:
      - containerPort: 443
---
apiVersion: v1
kind: Pod
metadata:
  name: ying-nginx-system-2
  tenant: system
spec:
  containers:
  - name: nginx
    image: nginx
    ports:
      - containerPort: 443

./cluster/kubectl.sh apply -f ~/TMP/mizar/pod-vpc-1-without-annotation.yaml

2.2) Check whether the annotations including two mizar CRDs - vpc and subnet are correctly added into two nginx pods

./cluster/kubectl.sh get pods/ying-nginx-system-1 --tenant system -o yaml |head -15
./cluster/kubectl.sh get pods/ying-nginx-system-2 --tenant system -o yaml |head -15

  annotations:
    <snip>
    mizar.com/vpc: system-default-network
    mizar.com/subnet: system-default-network-subnet

2.3) Check whether two nginx pods can communicate each other via curl

POD1="ying-nginx-system-1"
POD2="ying-nginx-system-2"
IP_POD1=`./cluster/kubectl.sh get pods --tenant system -o wide |grep $POD1 |awk '{print $7}'`
IP_POD2=`./cluster/kubectl.sh get pods --tenant system -o wide |grep $POD2 |awk '{print $7}'`
echo "$POD1: $IP_POD1"
echo "$POD2: $IP_POD2"
./cluster/kubectl.sh exec -ti $POD1 --tenant system -- curl $IP_POD2
./cluster/kubectl.sh exec -ti $POD2 --tenant system -- curl $IP_POD1

Test whether non-system tenant aaa's two nginx pods can talk with each other via 'curl'
3.1) Create non-system tenant 'aaa'

cat  ~/TMP/mizar/tenant-aaa.yaml

apiVersion: v1
kind: Tenant
metadata:
  name: aaa
spec:
  storageClusterId: "1"

./cluster/kubectl.sh apply -f ~/TMP/mizar/tenant-aaa.yaml

3.2) Check whether non-system tenant aaa's mizar CRDs are created successfully

./cluster/kubectl.sh get tenants
./cluster/kubectl.sh get networks -AT
./cluster/kubectl.sh get vpcs
./cluster/kubectl.sh get subnets

3.3) Use the following yaml file to create two nginx pods of non-system tenant 'aaa'
~/TMP/mizar/pod-vpc-1-without-annotation.aaa.yaml

apiVersion: v1
kind: Pod
metadata:
  name: ying-nginx-aaa-1
  tenant: aaa
spec:
  containers:
  - name: nginx
    image: nginx
    ports:
      - containerPort: 443
---
apiVersion: v1
kind: Pod
metadata:
  name: ying-nginx-aaa-2
  tenant: aaa
spec:
  containers:
  - name: nginx
    image: nginx
    ports:
      - containerPort: 443

./cluster/kubectl.sh apply -f ~/TMP/mizar/pod-vpc-1-without-annotation.aaa.yaml

3.4) Check whether the annotations including two mizar CRDs - vpc and subnet are correctly added into non-system tenant aaa's two nginx pods

./cluster/kubectl.sh get pods/ying-nginx-aaa-1 --tenant aaa -o yaml |head -15
./cluster/kubectl.sh get pods/ying-nginx-aaa-2 --tenant aaa -o yaml |head -15

  annotations:
    <snip>
    mizar.com/vpc: aaa-default-network
    mizar.com/subnet: aaa-default-network-subnet

3.5) Check whether non-system tenant aaa's two nginx pods can communicate each other via curl

POD1="ying-nginx-aaa-1"
POD2="ying-nginx-aaa-2"
IP_POD1=`./cluster/kubectl.sh get pods --tenant aaa -o wide |grep $POD1 |awk '{print $7}'`
IP_POD2=`./cluster/kubectl.sh get pods --tenant aaa -o wide |grep $POD2 |awk '{print $7}'`
echo "$POD1: $IP_POD1"
echo "$POD2: $IP_POD2"
./cluster/kubectl.sh exec -ti $POD1 --tenant aaa -- curl $IP_POD2
./cluster/kubectl.sh exec -ti $POD2 --tenant aaa -- curl $IP_POD1

Test to ensure system tenant's two nginx pods SHOULD not talk with non-system tenant aaa's two nginx pods
4.1) Get IPs of all pods including system tenant's nginx pods and non-system tenant's nginx pods

./cluster/kubectl.sh get pods -AT -o wide

4.2) Test whether system tenant's two nginx pods SHOULD not talk with non-system tenant aaa's two nginx pods

./cluster/kubectl.sh exec -ti ying-nginx-system-1 -n default --tenant system -- curl <IP of ying-nginx-aaa-1 pod>
./cluster/kubectl.sh exec -ti ying-nginx-system-1 -n default --tenant system -- curl <IP of ying-nginx-aaa-2 pod>
./cluster/kubectl.sh exec -ti ying-nginx-system-2 -n default --tenant system -- curl <IP of ying-nginx-aaa-1 pod>
./cluster/kubectl.sh exec -ti ying-nginx-system-2 -n default --tenant system -- curl <IP of ying-nginx-aaa-2 pod>

4.3) Test whether non-system tenant aaa's two nginx pods SHOULD not talk with system tenant's two nginx pods

./cluster/kubectl.sh exec -ti ying-nginx-aaa-1 -n default --tenant aaa -- curl <IP of ying-nginx-system-1 pod>
./cluster/kubectl.sh exec -ti ying-nginx-aaa-1 -n default --tenant aaa -- curl <IP of ying-nginx-system-2 pod>
./cluster/kubectl.sh exec -ti ying-nginx-aaa-2 -n default --tenant aaa -- curl <IP of ying-nginx-system-1 pod>
./cluster/kubectl.sh exec -ti ying-nginx-aaa-2 -n default --tenant aaa -- curl <IP of ying-nginx-system-2 pod>

Clean-up after verification
5.1) Delete non-system tenant 'aaa'

./cluster/kubectl.sh delete -f ~/TMP/mizar/tenant-aaa.yaml

5.2) Delete system tenant's two nginx pods

./cluster/kubectl.sh delete -f ~/TMP/mizar/pod-vpc-1-without-annotation.yaml

Does this PR introduce a user-facing change?:
No.

…etwork controller after successful integration tests between mizar team and arktos team

h-w-chen

lgtm - it has been verified working; already met bar of poc.

the PR description claims that it YES has user exposable changes - in that case, please provide more detail about the change.

hack/lib/protoc-mizar.sh

h-w-chen · 2022-01-17T17:51:42Z

hack/update-generated-mizar.sh

@@ -0,0 +1,26 @@
+#!/usr/bin/env bash


just for curiosity: is this file auto generated, or authored by hand?

This file is authored by hand after I see the file 'hack/update-generated.sh' as benchmark for quickly generating the file pkg/controller/mizar/builtins.pb.go after the file pkg/controller/mizar/builtins.proto is changed.

I can see a lot of similarity of hack/update-generated-protobuf.sh and this file. Note update-generated-protobuf is automatically called in make update but this script needs to be manually run for code generation. We need to make this code generation automated.

Will do in master branch

I updated builtins.proto for type definition, run update-generated-mizar.sh, there is no code update. I wonder how your script work?

Needs ubuntu 18.04 or upper, install protobuf-compiler first: "sudo apt install -y protobuf-compiler". "protoc --version" needs to be 3.0 or upper.

pkg/controller/mizar/mizar-arktos-network-controller.go

cmd/kube-controller-manager/app/mizarcontrollers.go

Sindica · 2022-01-17T20:11:43Z

hack/lib/protoc-mizar.sh

@@ -0,0 +1,93 @@
+#!/usr/bin/env bash


Did you come up the entire script of copied from somewhere? If later, please specify the source.

Add the comments below in this script.

"This cript is created based on hack/lib/protoc.sh and is made with minor changes"

Is there a reason that we cann't make minor change to hack/lib/protoc.sh and reuse the code. I can see a lot of code is same between the two files. No reason to maintain two copies of similar code.

I ever attempted to try this idea. But the new code changes for hack/lib/protoc.sh are related to too many other codes because other codes will call the functions inside hack/lib/protoc.sh, which needs more time to do tests. So I temporarily give up this idea in POC.

Will do in master branch.

pkg/controller/mizar/mizar-arktos-network-controller.go

pkg/controller/mizar/mizar-pod-controller.go

Sindica

Please note many of the suggestions are for master code standard. They should not block the code from checking into POC branch. Feel free to merge the code and refactor the code later.

cmd/kube-controller-manager/app/mizarcontrollers.go

Sindica · 2022-01-18T16:28:33Z

hack/lib/protoc-mizar.sh

@@ -0,0 +1,93 @@
+#!/usr/bin/env bash


Is there a reason that we cann't make minor change to hack/lib/protoc.sh and reuse the code. I can see a lot of code is same between the two files. No reason to maintain two copies of similar code.

Sindica · 2022-01-18T16:36:14Z

hack/update-generated-mizar.sh

@@ -0,0 +1,26 @@
+#!/usr/bin/env bash


I can see a lot of similarity of hack/update-generated-protobuf.sh and this file. Note update-generated-protobuf is automatically called in make update but this script needs to be manually run for code generation. We need to make this code generation automated.

pkg/controller/mizar/mizar-arktos-network-controller.go

pkg/controller/mizar/mizar-pod-controller.go

centaurus-cloud-bot · 2022-01-18T16:48:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: h-w-chen, Sindica
To complete the pull request process, please assign xiaoningding
You can assign the PR to them by writing /assign @xiaoningding in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

q131172019 · 2022-01-18T19:09:53Z

I can see a lot of similarity of hack/update-generated-protobuf.sh and this file. Note update-generated-protobuf is >>automatically called in make update but this script needs to be manually run for code generation. We need to make this >>code generation automated.

Yes, we need time to figure out how to automatically generate.

Sindica · 2022-01-18T19:25:02Z

I can see a lot of similarity of hack/update-generated-protobuf.sh and this file. Note update-generated-protobuf is >>automatically called in make update but this script needs to be manually run for code generation. We need to make this >>code generation automated.

Yes, we need time to figure out how to automatically generate.

If only for auto code generating, just add your function into hack/make-rules/update.sh

q131172019 · 2022-01-20T03:37:02Z

ToDo for master branch:

cmd/kube-controller-manager/app/mizarcontrollers.go - all mizar controller should share one informerFactory
hack/lib/protoc-mizar.sh - Is there a reason that we cann't make minor change to hack/lib/protoc.sh and reuse the code. I can see a lot of code is same between the two files. No reason to maintain two copies of similar code.
hack/update-generated-mizar.sh - I can see a lot of similarity of hack/update-generated-protobuf.sh and this file. Note update-generated-protobuf is automatically called in make update but this script needs to be manually run for code generation. We need to make this code generation automated.

Sindica · 2022-01-20T22:06:38Z

ToDo for master branch:

cmd/kube-controller-manager/app/mizarcontrollers.go - all mizar controller should share one informerFactory

hack/lib/protoc-mizar.sh - Is there a reason that we cann't make minor change to hack/lib/protoc.sh and reuse the code. I can see a lot of code is same between the two files. No reason to maintain two copies of similar code.

hack/update-generated-mizar.sh - I can see a lot of similarity of hack/update-generated-protobuf.sh and this file. Note update-generated-protobuf is automatically called in make update but this script needs to be manually run for code generation. We need to make this code generation automated.

Please hold on for item 1,2,3, as Vinay's proof of concept demo for option 3, there is no need for Mizar controllers. Hence, all the above suggested improvement won't be needed.

Finalize the code changes for mizar pod controller and mizar arktos n…

04bb125

…etwork controller after successful integration tests between mizar team and arktos team

centaurus-cloud-bot added the size/XL label Jan 17, 2022

q131172019 requested review from h-w-chen and Sindica January 17, 2022 07:29

h-w-chen approved these changes Jan 17, 2022

View reviewed changes

Sindica requested changes Jan 17, 2022

View reviewed changes

q131172019 added 2 commits January 18, 2022 04:26

Code changes based on HongWei's feedback and YingHuang's feedback

6d66a5b

Add TODO comment in mizarcontrollers.go

b8c1332

Sindica approved these changes Jan 18, 2022

View reviewed changes

quick change for app/mizarcontrollers.go

b642990

Sindica merged commit 2ab1801 into CentaurusInfra:poc-2022-01-30 Jan 19, 2022

Sindica mentioned this pull request Jan 21, 2022

Share same networkInformerFactory in mizar controllers #1294

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mizar-Arktos-Integration]Tenant-vpc-support: finalize code changes for mizar pod controller and mizar arktos network controller after successful integration tests #1281

[Mizar-Arktos-Integration]Tenant-vpc-support: finalize code changes for mizar pod controller and mizar arktos network controller after successful integration tests #1281

q131172019 commented Jan 17, 2022 •

edited

Loading

h-w-chen left a comment

h-w-chen Jan 17, 2022

q131172019 Jan 17, 2022

Sindica Jan 18, 2022

q131172019 Jan 20, 2022

Sindica Feb 2, 2022

Sindica Feb 3, 2022

Sindica Jan 17, 2022

q131172019 Jan 17, 2022 •

edited

Loading

Sindica Jan 18, 2022

q131172019 Jan 18, 2022

q131172019 Jan 20, 2022

Sindica left a comment

Sindica Jan 18, 2022

Sindica Jan 18, 2022

centaurus-cloud-bot commented Jan 18, 2022

q131172019 commented Jan 18, 2022

Sindica commented Jan 18, 2022

q131172019 commented Jan 20, 2022

Sindica commented Jan 20, 2022

[Mizar-Arktos-Integration]Tenant-vpc-support: finalize code changes for mizar pod controller and mizar arktos network controller after successful integration tests #1281

[Mizar-Arktos-Integration]Tenant-vpc-support: finalize code changes for mizar pod controller and mizar arktos network controller after successful integration tests #1281

Conversation

q131172019 commented Jan 17, 2022 • edited Loading

h-w-chen left a 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

q131172019 Jan 17, 2022 • edited Loading

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

Sindica left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

centaurus-cloud-bot commented Jan 18, 2022

q131172019 commented Jan 18, 2022

Sindica commented Jan 18, 2022

q131172019 commented Jan 20, 2022

Sindica commented Jan 20, 2022

q131172019 commented Jan 17, 2022 •

edited

Loading

q131172019 Jan 17, 2022 •

edited

Loading