Skip to content

Conversation

@aggarwal0009
Copy link
Contributor

@aggarwal0009 aggarwal0009 commented Aug 8, 2023

Reason for Change:

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

Issue Fixed:

Requirements:

Notes:

@aggarwal0009 aggarwal0009 requested a review from a team as a code owner August 8, 2023 22:43
@aggarwal0009 aggarwal0009 requested a review from vakalapa August 8, 2023 22:43
@aggarwal0009 aggarwal0009 changed the title Add MT PodNetworkConfig CRD [DRAFT] : Add MT PodNetworkConfig CRD Aug 8, 2023
@aggarwal0009 aggarwal0009 changed the title [DRAFT] : Add MT PodNetworkConfig CRD [Multitenancy] : Add MT PodNetworkConfig CRD Aug 8, 2023
@aggarwal0009 aggarwal0009 assigned rbtr and aggarwal0009 and unassigned rbtr Aug 8, 2023
@rbtr rbtr requested a review from nddq August 9, 2023 16:16
@tamilmani1989 tamilmani1989 added cns Related to CNS. and removed cns Related to CNS. labels Aug 9, 2023
@aggarwal0009 aggarwal0009 requested a review from a team as a code owner August 9, 2023 19:29
}

// SetOwnerRef sets the controller of the MultitenantPodNetworkConfig to the given object atomically, using HTTP Patch.
// Deprecated: SetOwnerRef is deprecated, use the more correctly named SetControllerRef.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not add a new function that's already deprecated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

}

// PatchSpec performs a server-side patch of the passed MultitenantPodNetworkConfigSpec to the MultitenantPodNetworkConfig specified by the NamespacedName.
func (c *Client) PatchSpec(ctx context.Context, key types.NamespacedName, spec *v1alpha1.MultitenantPodNetworkConfigSpec, fieldManager string) (*v1alpha1.MultitenantPodNetworkConfig, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this necessary? MTPNC will get created with known values for spec and I don't expect the spec to need to get updated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed


// UpdateSpec does a fetch, deepcopy, and update of the MultitenantPodNetworkConfig 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.MultitenantPodNetworkConfigSpec) (*v1alpha1.MultitenantPodNetworkConfig, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I think this func is not needed for same reason as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Copy link
Collaborator

@thatmattlong thatmattlong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not add the helper funcs to the client which mutate the spec until we know we need them

miguelgoms
miguelgoms previously approved these changes Aug 10, 2023
@@ -0,0 +1,3 @@
# MultitenantPodNetworkConfig CRDs

MPNC objects represent the network configuration goal state for a pod running a multitenant networked container and are created and managed by control plane as part of the network configuration, during Pod lifecycle events.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also add that it is created by AKS Customers. So any change in the CRD definition has to be backward compatible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MPNC object will be created by the swift control plane, though - the crds created by the AKS customers will be on a separate PR. The point on backward compat is valid, but the interface is between CNS and control plane.

// MultitenantPodNetworkConfigStatus defines the observed state of PodNetworkConfig
type MultitenantPodNetworkConfigStatus struct {
// network container id
UUID string `json:"uuid,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets call it NCId only or CompartmentId

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated


// MultitenantPodNetworkConfig is the Schema for the multitenantpodnetworkconfigs API
// +kubebuilder:resource:scope=Namespaced
// +kubebuilder:resource:shortName=mpnc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest mtpnc for shotname since that's what we've been saying out loud

@aggarwal0009 aggarwal0009 merged commit e1581a1 into master Aug 11, 2023
@aggarwal0009 aggarwal0009 deleted the ankaggar/MTPC-CRD branch August 11, 2023 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants