Skip to content

[TCL 2925] Modify removeNADFinalizerIfSafe to ignore pods without GUID protection finalizer instead of pods with deletionTimestamp set #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Part-TimeWizard wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[TCL 2925] Modify removeNADFinalizerIfSafe to ignore pods without GUID protection finalizer instead of pods with deletionTimestamp set #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions pkg/daemon/daemon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ func NewDaemon() (Daemon, error) {

// Pass configuration from daemon to the plugin
pluginConfig := map[string]interface{}{
"ENABLE_IP_OVER_IB": daemonConfig.EnableIPOverIB,
"DEFAULT_LIMITED_PARTITION": daemonConfig.DefaultLimitedPartition,
"ENABLE_INDEX0_FOR_PRIMARY_PKEY": daemonConfig.EnableIndex0ForPrimaryPkey,
"ENABLE_IP_OVER_IB": daemonConfig.EnableIPOverIB,
"DEFAULT_LIMITED_PARTITION": daemonConfig.DefaultLimitedPartition,
"ENABLE_INDEX0_FOR_PRIMARY_PKEY": daemonConfig.EnableIndex0ForPrimaryPkey,
}
if err := smClient.SetConfig(pluginConfig); err != nil {
log.Warn().Msgf("Failed to set configuration on subnet manager plugin: %v", err)
Expand Down Expand Up @@ -714,15 +714,6 @@ func (d *daemon) DeletePeriodicUpdate() {
log.Warn().Msgf("%v", err)
continue
}

// Check if NAD finalizer can be safely removed
networkNamespace, networkName, _ := utils.ParseNetworkID(networkID)
if err := d.removeNADFinalizerIfSafe(networkNamespace, networkName); err != nil {
log.Error().Msgf("failed to remove NAD finalizer for %s/%s: %v", networkNamespace, networkName, err)
} else {
log.Info().Msgf("checked and potentially removed finalizer %s from NetworkAttachmentDefinition %s/%s",
GUIDInUFMFinalizer, networkNamespace, networkName)
}
}

for _, guidAddr := range guidList {
Expand All @@ -746,6 +737,17 @@ func (d *daemon) DeletePeriodicUpdate() {
}
}
}

// Once all pod GUIDs in the batch have been handled, check if NAD finalizer can be safely removed
if ibCniSpec.PKey != "" && len(guidList) != 0 {
networkNamespace, networkName, _ := utils.ParseNetworkID(networkID)
if err := d.removeNADFinalizerIfSafe(networkNamespace, networkName); err != nil {
log.Error().Msgf("failed to remove NAD finalizer for %s/%s: %v", networkNamespace, networkName, err)
} else {
log.Info().Msgf("checked and potentially removed finalizer %s from NetworkAttachmentDefinition %s/%s",
GUIDInUFMFinalizer, networkNamespace, networkName)
}
}
deleteMap.UnSafeRemove(networkID)
}

Expand Down Expand Up @@ -810,6 +812,16 @@ func (d *daemon) initPool() error {
return nil
}

// hasPodFinalizer checks if the pod has the GUID finalizer (i.e. the guid is still registered with UFM)
func hasPodFinalizer(pod *kapi.Pod) bool {
for _, finalizer := range pod.Finalizers {
if finalizer == PodGUIDFinalizer {
return true
}
}
return false
}

// checkIfAnyPodsUsingNetwork checks if there are any pods still using the given network
func (d *daemon) checkIfAnyPodsUsingNetwork(networkNamespace, networkName string) (bool, error) {
pods, err := d.kubeClient.GetPods(kapi.NamespaceAll)
Expand All @@ -820,11 +832,6 @@ func (d *daemon) checkIfAnyPodsUsingNetwork(networkNamespace, networkName string
for i := range pods.Items {
pod := &pods.Items[i]

// Skip pods that are being deleted (have deletion timestamp)
if pod.DeletionTimestamp != nil {
continue
}

if !utils.HasNetworkAttachmentAnnot(pod) {
continue
}
Expand All @@ -837,8 +844,8 @@ func (d *daemon) checkIfAnyPodsUsingNetwork(networkNamespace, networkName string
for _, network := range networks {
// Check if this pod uses the network we're checking
if network.Namespace == networkNamespace && network.Name == networkName {
// Check if this network is configured with InfiniBand and has a GUID
if utils.IsPodNetworkConfiguredWithInfiniBand(network) && utils.PodNetworkHasGUID(network) {
// Check if this network is configured with InfiniBand and has a still registered GUID
if utils.IsPodNetworkConfiguredWithInfiniBand(network) && utils.PodNetworkHasGUID(network) && hasPodFinalizer(pod) {
log.Debug().Msgf("Found pod %s/%s still using network %s/%s",
pod.Namespace, pod.Name, networkNamespace, networkName)
return true, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/k8s-client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

type Client interface {
GetPods(namespace string) (*kapi.PodList, error)
GetPods(namespace string) (*kapi.PodList, error) // NOTE: List pods need strong consistency guarantees, ListOptions cannot be modified to use non-quorum reads.
SetAnnotationsOnPod(pod *kapi.Pod, annotations map[string]string) error
PatchPod(pod *kapi.Pod, patchType types.PatchType, patchData []byte) error
GetNetworkAttachmentDefinition(namespace, name string) (*netapi.NetworkAttachmentDefinition, error)
Expand Down