Extending the MTPNC status #4058
Conversation
| // +kubebuilder:validation:Optional | ||
| NodeName string `json:"nodeName,omitempty"` | ||
| // PodName is the name of the pod | ||
| PodName string `json:"podName,omitempty"` |
There was a problem hiding this comment.
is this ownerKey ( podname + podnamespace ) or just podname?
There was a problem hiding this comment.
I would assume just podName because MTPNC is also created in the same namepace as the pod.
| // PodName is the name of the pod | ||
| PodName string `json:"podName,omitempty"` | ||
| // PodUID is the uid of the pod | ||
| PodUID string `json:"podUID,omitempty"` |
There was a problem hiding this comment.
curious to know what this would be useful for?
There was a problem hiding this comment.
To begin with we want to query MTPNC by podUID, because we have seen some cases where statefulSets create pods with same namd and the second pod sometimes reuses the MTPNC from the first pod. In the future we want name MTPNC based on podUID, so it is unique per pod.
| // +kubebuilder:validation:Optional | ||
| NodeName string `json:"nodeName,omitempty"` | ||
| // PodName is the name of the pod | ||
| PodName string `json:"podName,omitempty"` |
There was a problem hiding this comment.
I would assume just podName because MTPNC is also created in the same namepace as the pod.
| // PodName is the name of the pod | ||
| PodName string `json:"podName,omitempty"` | ||
| // PodUID is the uid of the pod | ||
| PodUID string `json:"podUID,omitempty"` |
There was a problem hiding this comment.
To begin with we want to query MTPNC by podUID, because we have seen some cases where statefulSets create pods with same namd and the second pod sometimes reuses the MTPNC from the first pod. In the future we want name MTPNC based on podUID, so it is unique per pod.
There was a problem hiding this comment.
Pull Request Overview
This PR extends the MultiTenantPodNetworkConfig (MTPNC) CRD to include additional pod-related fields for better tracking and management. The changes add pod metadata to both the specification and status sections of the CRD.
- Adds
PodUIDfield to the MTPNC spec for pod identification - Adds
NodeNamefield to the MTPNC status for tracking pod placement
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crd/multitenancy/manifests/multitenancy.acn.azure.com_multitenantpodnetworkconfigs.yaml | Updates CRD manifest to include new podUID and nodeName fields |
| crd/multitenancy/api/v1alpha1/multitenantpodnetworkconfig.go | Adds PodUID to spec struct and NodeName to status struct |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
crd/multitenancy/manifests/multitenancy.acn.azure.com_multitenantpodnetworkconfigs.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]> Signed-off-by: shreyashastantram <[email protected]>
…antpodnetworkconfigs.yaml Co-authored-by: Copilot <[email protected]> Signed-off-by: shreyashastantram <[email protected]>
shreyashastantram
force-pushed
the
swiftv2/mtpnc_changes
branch
from
aafb373 to
f7df550
Compare
Reason for Change:
This PR focuses on extending the mtpnc spec and status fields to include the Pod UID in the spec and the Node name in the status.
Pod UID is added to the spec because controllers are aware of the pod’s unique identifier at the time of mtpnc creation.
Node name is added to the status because scheduling decisions do not depend on the node name parameter. The node name becomes available only after the scheduling decision has been made.
If Node name were part of the spec, it would act as an optional input from the creator of the mtpnc. This could lead to inconsistent states—for example, if the creator specifies a node name that differs from the actual node where the pod is scheduled.
Issue Fixed:
Requirements:
Notes: