CKO: Changes added to handle cko-calico flavor configuration #17

abhis2112 · 2021-11-19T20:49:34Z

Added a new task file which run cko calico side changes via acc-provision
Added the new net-operator-controller deployment alongwith the CRD and CR in templates folder.
Dockerfile acc-provision branch changes

abhis2112 · 2021-11-19T20:50:33Z

It's a First Draft. Kindly review with all possible comments

roles/accprovision/templates/net-operator-deployment.yaml

snaiksat · 2021-11-19T21:01:20Z

roles/accprovision/templates/net-operator-deployment.yaml

+      image_prefix: quay.io/noirolabs
+      image_pull_secret: noiro-docker-registry-secret
+      openvswitch_version: kmr2-test
+      opflex_agent_version: kmr2-test


drop this section

roles/accprovision/templates/net-operator-deployment.yaml

snaiksat · 2021-11-19T21:03:54Z

roles/accprovision/tasks/net-operator-controller.yml

+- name: Collecting config-map spec
+  set_fact:
+    cmap_input: "{{ cmap_data['resources'][0]['data']['spec'] }}"
+


L8 to L28 are not relevant (there is no config-map that is going to be used)

the config-map holds the original structure of acc_provision_input yaml, which will be used to create a new input yaml from the CR if we update any value in the CR. The values in CR will be deep merged with its original content. Although, we can discuss it if we are going to have just calico config and nothing else as per related to acc_provision_input.yaml

roles/accprovision/tasks/net-operator-controller.yml

snaiksat · 2021-11-24T06:06:06Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+apiVersion: v1
+kind: Namespace
+metadata:
+  name: aci-containers-system


this is okay for now, but over time we should change this to be network-operator-controller

roles/accprovision/manifests/net-operator-controller-deployment.yaml

snaiksat · 2021-11-24T06:07:08Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+  - '*'
+  verbs:
+  - '*'
+- apiGroups:


all the aci-cni specific details should be removed

apiGroups:

aci.ctrl
resources:

accprovisioninputs

accprovisioninputs/status

accprovisioninputs/finalizers

these are only the ones that are there as we have kept the name the same for the CR

Check the ones below, they are all aci-cni specific and unnecessary...

L55-61 and L85-122 should be removed.

snaiksat · 2021-11-24T06:08:37Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+    \        \"node_svc_subnet\": \"15.5.168.1/21\",\n            \"interface_mtu\"\
+    : 1400,\n            \"pod_subnet_chunk_size\": 24,\n            \"node_subnet\"\
+    : \"15.11.0.1/27\",\n            \"pod_subnet\": \"15.128.0.1/16\"\n        }\n\
+    \    }\n }"


i think this spec string should be different, in fact it should ideally be empty to begin with...

Checking....

Checked. Currently needed the pod_subnet and rest other fields in net_config. Otherwise, it says missing option. We need to support the downstream acc-provision to remove the rest of the irrelevant fields.

HI @abhis2112, I have removed the "node_svc_subnet", "pod_subnet_chunk_size" and "extern_static" fields from the required options in my acc-provision branch. When using "cko-calico" flavor, these fields are no longer required and can be safely removed.

snaiksat · 2021-12-02T04:01:23Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+  - '*'
+  verbs:
+  - '*'
+- apiGroups:


L55-61 and L85-122 should be removed.

snaiksat · 2021-12-02T04:02:45Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+    \            \"extern_dynamic\"\
+    : \"151.9.1.1/24\",\n\
+    \            \"node_subnet\": \"15.11.0.2/27\",\n            \"pod_subnet\": \"\
+    15.128.0.1/27\"\n        }\n    }\n }"


The above spec should not have any fabric-specific hard-coded values.

Putting "dummy" as the value

snaiksat · 2021-12-02T04:03:26Z

roles/accprovision/templates/net-operator-cr.yaml

+        encap_type: vxlan
+        mcast_range:
+          start: 225.20.1.1
+          end: 225.20.255.255


L12-16 should be removed

removing assuming backend acc-provision supports the removal

snaiksat · 2021-12-02T04:04:04Z

roles/accprovision/templates/net-operator-cr.yaml

+    net_config: {}
+    nodepodif_config: {}
+    operator_managed_config:
+      enable_updates: false


L35-36 should be removed

snaiksat · 2021-12-02T04:04:24Z

roles/accprovision/templates/net-operator-cr.yaml

+      cnideploy_version: kmr2-test
+      opflex_agent_version: kmr2-test
+      openvswitch_version: kmr2-test
+      aci_containers_operator_version: kmr2-test


L39-44 should be removed

roles/accprovision/manifests/net-operator-controller-deployment.yaml

1. Added a new task file which run cko calico side changes via acc-provision 2. Created a new namespace net-operator-controller 3. Added the new net-operator-controller deployment in manifest folder alongwith the CRD, and CR in templates folder. 4. Updated CRD schema according to control cluster deployment 5. Dockerfile acc-provision branch changes 6. Provided sample CR with all dummy details

snaiksat · 2022-01-12T03:08:37Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+kind: ClusterRole
+metadata:
+  labels:
+    network-plugin: calico


This label is not relevant.

snaiksat · 2022-01-12T03:14:44Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: net-operator-controller


Lets keep the namespace name as "net-operator-controller" but change all other occurrences below to network-operator-fabric-controller.

I think the "controller" terminology is confusing. Let's use "manager" instead. Apologies for the repeated changes, so lets call this net-operator-manager.

snaiksat · 2022-01-12T03:16:16Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+  verbs:
+  - '*'
+- apiGroups:
+  - aci.ctrl


lets call this something that reflects "network operator fabric manager"

snaiksat · 2022-01-12T03:20:29Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+      status: {}
+    schema:
+      openAPIV3Schema:
+        description: accprovisioninput defines the input configuration for ACI CNI


Update the description here and below to remove references to ACI CNI.

snaiksat · 2022-01-12T03:21:01Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+  spec: "{}"
+kind: ConfigMap
+metadata:
+  name: acc-provision-config


cluster-input-config

snaiksat · 2022-01-12T03:21:37Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+- apiGroups:
+  - aci.ctrl
+  resources:
+  - accprovisioninputs


Let's replace all occurrences of "accprovision" here and below with "clusterinput" or "ClusterInput" or "cluster-input" as relevant.

snaiksat · 2022-01-12T03:22:37Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+        scheduler.alpha.kubernetes.io/critical-pod: ''
+      labels:
+        name: net-operator-controller
+        network-plugin: aci-containers


snaiksat · 2022-01-12T03:23:40Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+        - name: ACC_PROVISION_FLAVOR
+          value: cko-calico
+        - name: ACC_PROVISION_INPUT_CR_NAME
+          value: accprovisioninput


change this name and value to reflect "cluster input" naming convention

snaiksat · 2022-01-12T03:25:14Z

roles/accprovision/manifests/net-operator-controller-deployment.yaml

+          value: accprovisioninput
+        image: quay.io/noirolabs/acc-provision-operator:abhishek
+        imagePullPolicy: Always
+        name: acc-provision-operator


network-operator-fabric-controller

snaiksat · 2022-01-12T03:31:53Z

roles/accprovision/tasks/net-operator-controller.yml

+
+
+- name: Generate new deployment yaml
+  shell: "acc-provision -c {{ acc_provision_file_name }} -f {{ lookup('env', 'ACC_PROVISION_FLAVOR') }} -o {{ aci_cni_deployment_file }}"


can we rename aci_cni_deployment_file to: network_operator_deployment_file

snaiksat · 2022-01-12T03:32:49Z

roles/accprovision/templates/net-operator-cr.yaml

@@ -0,0 +1,94 @@
+apiVersion: aci.ctrl/v1alpha1
+kind: AccProvisionInput


Update as per earlier changes here and below

snaiksat · 2022-01-12T03:38:38Z

Dockerfile

@@ -13,11 +13,11 @@ COPY docker/licenses /licenses
 # Export http and https proxy here if building locally for dev


We need a new docker file for this, we keep the acc-provision-operator docker file as is

abhis2112 requested review from snaiksat, anmol372 and tanyatukade November 19, 2021 20:49

snaiksat reviewed Nov 19, 2021

View reviewed changes

roles/accprovision/templates/net-operator-deployment.yaml Outdated Show resolved Hide resolved

snaiksat requested changes Nov 19, 2021

View reviewed changes

abhis2112 force-pushed the abhi-cko-calico branch 5 times, most recently from b25e637 to 58c3012 Compare November 23, 2021 21:11

snaiksat requested changes Nov 24, 2021

View reviewed changes

abhis2112 force-pushed the abhi-cko-calico branch 2 times, most recently from 07544b1 to 66846fb Compare December 1, 2021 07:10

snaiksat requested changes Dec 2, 2021

View reviewed changes

abhis2112 force-pushed the abhi-cko-calico branch from 66846fb to f2f9317 Compare December 2, 2021 12:03

tanyatukade reviewed Dec 15, 2021

View reviewed changes

roles/accprovision/manifests/net-operator-controller-deployment.yaml Show resolved Hide resolved

abhis2112 force-pushed the abhi-cko-calico branch from f2f9317 to e6d2f88 Compare January 5, 2022 20:34

abhis2112 force-pushed the abhi-cko-calico branch from e6d2f88 to 542c43d Compare January 11, 2022 19:49

snaiksat reviewed Jan 12, 2022

View reviewed changes

snaiksat requested changes Jan 12, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CKO: Changes added to handle cko-calico flavor configuration #17

CKO: Changes added to handle cko-calico flavor configuration #17

abhis2112 commented Nov 19, 2021

abhis2112 commented Nov 19, 2021

snaiksat Nov 19, 2021

snaiksat Nov 19, 2021

abhis2112 Nov 22, 2021

snaiksat Nov 24, 2021

snaiksat Nov 24, 2021

abhis2112 Nov 24, 2021

snaiksat Nov 29, 2021

snaiksat Dec 2, 2021

snaiksat Nov 24, 2021

abhis2112 Nov 24, 2021

abhis2112 Nov 30, 2021

tanyatukade Dec 1, 2021 •

edited

Loading

snaiksat Dec 2, 2021

snaiksat Dec 2, 2021

abhis2112 Dec 2, 2021

snaiksat Dec 2, 2021

abhis2112 Dec 2, 2021

snaiksat Dec 2, 2021

snaiksat Dec 2, 2021

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022 •

edited

Loading

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022

snaiksat Jan 12, 2022



		- name: Generate new deployment yaml
		shell: "acc-provision -c {{ acc_provision_file_name }} -f {{ lookup('env', 'ACC_PROVISION_FLAVOR') }} -o {{ aci_cni_deployment_file }}"

		@@ -0,0 +1,94 @@
		apiVersion: aci.ctrl/v1alpha1
		kind: AccProvisionInput

		@@ -13,11 +13,11 @@ COPY docker/licenses /licenses
		# Export http and https proxy here if building locally for dev

CKO: Changes added to handle cko-calico flavor configuration #17

Are you sure you want to change the base?

CKO: Changes added to handle cko-calico flavor configuration #17

Conversation

abhis2112 commented Nov 19, 2021

abhis2112 commented Nov 19, 2021

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

Choose a reason for hiding this comment

tanyatukade Dec 1, 2021 • 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

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

snaiksat Jan 12, 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

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

tanyatukade Dec 1, 2021 •

edited

Loading

snaiksat Jan 12, 2022 •

edited

Loading