[Multitenancy]: Add NodeInfo crd #2113

aggarwal0009 · 2023-08-09T15:39:10Z

Reason for Change:

This CRD is added to enable VNET multitenancy – which will be watched and managed by the control plane.

NodeInfo objects are created by CNS as part of the node registration flow, and is used to pass any metadata from the VM needed by control plane.

Issue Fixed:

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
relevant PR labels added

Notes:

thatmattlong · 2023-08-09T16:23:27Z

I'm no expert on the rule here (is it like potato: potatoes or like radio: radios?), but this feels like it should be infos:

aggarwal0009 · 2023-08-09T16:26:43Z

I'm no expert on the rule here (is it like potato: potatoes or like radio: radios?), but this feels like it should be infos:
It was autogen. Need to check how to fix this

rbtr

we probably need to copy the embed boilerplate from the nnc/css pkgs also since these are supposed to be installed by the controlplane

crd/nodeinfo/manifests/acn.azure.com_nodeinfoes.yaml

crd/nodeinfo/api/v1alpha/groupversion_info.go

nddq · 2023-08-09T16:44:26Z

Are we going to need a client for this CRD?

tamilmani1989 · 2023-08-09T18:19:46Z

@aggarwal0009 please make sure you add label to PR that you open in github indicating which component it is

aggarwal0009 · 2023-08-09T21:40:35Z

we probably need to copy the embed boilerplate from the nnc/css pkgs also since these are supposed to be installed by the controlplane

Added

aggarwal0009 · 2023-08-09T21:42:22Z

Are we going to need a client for this CRD?

I think we will. Added

thatmattlong · 2023-08-10T00:28:47Z

crd/nodeinfo/client.go

+}
+
+// SetOwnerRef sets the controller of the NodeInfo to the given object atomically, using HTTP Patch.
+// Deprecated: SetOwnerRef is deprecated, use the more correctly named SetControllerRef.


let's not add new funcs that are immediately deprecated

thatmattlong · 2023-08-10T00:29:00Z

crd/nodeinfo/client.go

+}
+
+// PatchSpec performs a server-side patch of the passed NodeInfoSpec to the NodeInfo specified by the NamespacedName.
+func (c *Client) PatchSpec(ctx context.Context, key types.NamespacedName, spec *v1alpha1.NodeInfoSpec, fieldManager string) (*v1alpha1.NodeInfo, error) {


not necessary as we will not be patching the spec

thatmattlong · 2023-08-10T00:29:09Z

crd/nodeinfo/client.go

+
+// UpdateSpec does a fetch, deepcopy, and update of the NodeInfo with the passed spec.
+// Deprecated: UpdateSpec is deprecated and usage should migrate to PatchSpec.
+func (c *Client) UpdateSpec(ctx context.Context, key types.NamespacedName, spec *v1alpha1.NodeInfoSpec) (*v1alpha1.NodeInfo, error) {


not necessary as we will not be updating the spec

thatmattlong · 2023-08-10T00:30:46Z

crd/nodeinfo/client.go

+}
+
+// SetControllerRef sets the controller of the NodeInfo to the given object atomically, using HTTP Patch.
+func (c *Client) SetControllerRef(ctx context.Context, key types.NamespacedName, owner metav1.Object, fieldManager string) (*v1alpha1.NodeInfo, error) {


Probably not needed, client can just create the NodeInfo CRD with ownership at time of creation, don't need these funcs which mutate the ownership over time

thatmattlong · 2023-08-10T00:30:53Z

crd/nodeinfo/client.go

+	return obj, nil
+}
+
+func genPatchSkel(key types.NamespacedName) *v1alpha1.NodeInfo {


thatmattlong · 2023-08-10T03:31:22Z

crd/nodeinfo/api/v1alpha1/nodeinfo.go

+// +kubebuilder:resource:scope=Namespaced
+// +kubebuilder:resource:shortName=ni
+// +kubebuilder:resource:path=nodeinfo
+// +kubebuilder:subresource:status


status subresource not needed

right. removed

rbtr

nits except for the embed doc.go which is blocking

crd/nodeinfo/api/v1alpha1/groupversion_info.go

crd/nodeinfo/api/v1alpha1/nodeinfo.go

crd/nodeinfo/embed.go

crd/nodeinfo/manifests/acn.azure.com_nodeinfo.yaml

rbtr

needs CRD regen then lgtm

Co-authored-by: Evan Baker <[email protected]>

Signed-off-by: aggarwal0009 <[email protected]>

aggarwal0009 · 2023-08-11T06:16:04Z

needs CRD regen then lgtm

Fixed

aggarwal0009 requested a review from a team as a code owner August 9, 2023 15:39

aggarwal0009 requested a review from aegal August 9, 2023 15:39

rbtr requested a review from nddq August 9, 2023 16:19

rbtr requested changes Aug 9, 2023

View reviewed changes

crd/nodeinfo/manifests/acn.azure.com_nodeinfoes.yaml Outdated Show resolved Hide resolved

crd/nodeinfo/api/v1alpha/groupversion_info.go Outdated Show resolved Hide resolved

tamilmani1989 added the cns Related to CNS. label Aug 9, 2023

aggarwal0009 added multitenancy swift-v2 labels Aug 9, 2023

aggarwal0009 self-assigned this Aug 9, 2023

aggarwal0009 requested review from thatmattlong, miguelgoms and rbtr August 9, 2023 21:50

aggarwal0009 requested a review from a team as a code owner August 9, 2023 23:43

thatmattlong reviewed Aug 10, 2023

View reviewed changes

miguelgoms previously approved these changes Aug 10, 2023

View reviewed changes

thatmattlong reviewed Aug 10, 2023

View reviewed changes

rbtr requested changes Aug 10, 2023

View reviewed changes

crd/nodeinfo/api/v1alpha1/groupversion_info.go Outdated Show resolved Hide resolved

crd/nodeinfo/api/v1alpha1/nodeinfo.go Outdated Show resolved Hide resolved

crd/nodeinfo/embed.go Outdated Show resolved Hide resolved

crd/nodeinfo/manifests/acn.azure.com_nodeinfo.yaml Show resolved Hide resolved

aggarwal0009 requested a review from rbtr August 10, 2023 21:55

rbtr previously approved these changes Aug 10, 2023

View reviewed changes

neaggarwMS previously approved these changes Aug 10, 2023

View reviewed changes

add nodeinfo crd

cbc41a5

aggarwal0009 and others added 16 commits August 10, 2023 16:28

update manifest default value

48813f6

address pr comments

61234f5

update readme

65c07a6

update nodesInfo to nodeInfo

ce0e221

add crd to crdgen check file

b2f8a43

update manifest file

cc4e6ab

remove whitespaces from manifest

8b0e7e8

remove whitespaces from crdgen

32f8f1c

remove unnecessary helper funcs from CRD client

3b664ef

address pr comments

51916b7

remove status subresource from nodeinfo crd

2657290

update manifest

418db48

Update crd/nodeinfo/api/v1alpha1/nodeinfo.go

68cce3a

Co-authored-by: Evan Baker <[email protected]>

Update crd/nodeinfo/api/v1alpha1/groupversion_info.go

b3e0e05

Co-authored-by: Evan Baker <[email protected]>

Update crd/nodeinfo/embed.go

d37ce11

Co-authored-by: Evan Baker <[email protected]>

add doc.go

228cc09

aggarwal0009 force-pushed the ankaggar/NodeInfo-CRD branch from 163c254 to 228cc09 Compare August 10, 2023 23:28

update manifest

65464a1

aggarwal0009 dismissed stale reviews from neaggarwMS, rbtr, and miguelgoms via 65464a1 August 11, 2023 00:21

Merge branch 'master' into ankaggar/NodeInfo-CRD

fa2632e

Signed-off-by: aggarwal0009 <[email protected]>

rbtr approved these changes Aug 11, 2023

View reviewed changes

neaggarwMS approved these changes Aug 11, 2023

View reviewed changes

aggarwal0009 merged commit 6a9fddb into master Aug 11, 2023

aggarwal0009 deleted the ankaggar/NodeInfo-CRD branch August 11, 2023 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Multitenancy]: Add NodeInfo crd #2113

[Multitenancy]: Add NodeInfo crd #2113

aggarwal0009 commented Aug 9, 2023 •

edited

Loading

thatmattlong commented Aug 9, 2023

aggarwal0009 commented Aug 9, 2023 •

edited

Loading

rbtr left a comment •

edited

Loading

nddq commented Aug 9, 2023

tamilmani1989 commented Aug 9, 2023

aggarwal0009 commented Aug 9, 2023

aggarwal0009 commented Aug 9, 2023

thatmattlong Aug 10, 2023

aggarwal0009 Aug 10, 2023

thatmattlong Aug 10, 2023

aggarwal0009 Aug 10, 2023

thatmattlong Aug 10, 2023

aggarwal0009 Aug 10, 2023

thatmattlong Aug 10, 2023

aggarwal0009 Aug 10, 2023

thatmattlong Aug 10, 2023

aggarwal0009 Aug 10, 2023

aggarwal0009 Aug 10, 2023

thatmattlong Aug 10, 2023

aggarwal0009 Aug 10, 2023

rbtr left a comment

rbtr left a comment

aggarwal0009 commented Aug 11, 2023

[Multitenancy]: Add NodeInfo crd #2113

[Multitenancy]: Add NodeInfo crd #2113

Conversation

aggarwal0009 commented Aug 9, 2023 • edited Loading

thatmattlong commented Aug 9, 2023

aggarwal0009 commented Aug 9, 2023 • edited Loading

rbtr left a comment • edited Loading

Choose a reason for hiding this comment

nddq commented Aug 9, 2023

tamilmani1989 commented Aug 9, 2023

aggarwal0009 commented Aug 9, 2023

aggarwal0009 commented Aug 9, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbtr left a comment

Choose a reason for hiding this comment

rbtr left a comment

Choose a reason for hiding this comment

aggarwal0009 commented Aug 11, 2023

aggarwal0009 commented Aug 9, 2023 •

edited

Loading

aggarwal0009 commented Aug 9, 2023 •

edited

Loading

rbtr left a comment •

edited

Loading