Skip to content

Commit

Permalink
fix: delete home pod when the foreign one is deleted
Browse files Browse the repository at this point in the history
Pod deletion initiated from the home pod was not completed correctly.
Indeed, the pod's container statuses in the home was never set to
terminated so the virtual kubelet was not able to delete pod.

This fix updates the container statuses in one go, when the foreign pod
is deleted, ie. we miss to update the home pod when the foreign
containers are terminated. Fixing this problem is out of scope and would
probably require modifying the pod black listing mechanism which seems
too risky at this time. Issue described here:
#721.

Resolves: #598
  • Loading branch information
fprojetto committed Jul 5, 2021
1 parent 037fff9 commit 4e92df3
Show file tree
Hide file tree
Showing 6 changed files with 425 additions and 37 deletions.
Expand Up @@ -13,7 +13,9 @@ var ReflectorBuilder = map[apimgmt.ApiType]func(reflector ri.APIReflector, opts

func podsReflectorBuilder(reflector ri.APIReflector, opts map[options.OptionKey]options.Option) ri.IncomingAPIReflector {
return &PodsIncomingReflector{
APIReflector: reflector}
APIReflector: reflector,
HomePodGetter: GetHomePodFunc,
}
}

func replicaSetsReflectorBuilder(reflector ri.APIReflector, _ map[options.OptionKey]options.Option) ri.IncomingAPIReflector {
Expand Down
79 changes: 56 additions & 23 deletions pkg/virtualKubelet/apiReflection/reflectors/incoming/pods.go
Expand Up @@ -12,7 +12,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/util/retry"
"k8s.io/klog"
"k8s.io/klog/v2"

"github.com/liqotech/liqo/pkg/virtualKubelet"
apimgmt "github.com/liqotech/liqo/pkg/virtualKubelet/apiReflection"
Expand All @@ -22,10 +22,14 @@ import (
"github.com/liqotech/liqo/pkg/virtualKubelet/forge"
)

// HomePodGetter function to retrieve the shadow home pod given the foreign pod.
type HomePodGetter func(reflector ri.APIReflector, foreignPod *corev1.Pod) (*corev1.Pod, error)

// PodsIncomingReflector is the incoming reflector in charge of detecting status change in foreign pods
// and pushing the updated object to the vk internals.
type PodsIncomingReflector struct {
ri.APIReflector
HomePodGetter HomePodGetter
}

// SetSpecializedPreProcessingHandlers allows to set the pre-routine handlers for the PodsIncomingReflector.
Expand Down Expand Up @@ -90,46 +94,41 @@ func (r *PodsIncomingReflector) PreUpdate(newObj, _ interface{}) (interface{}, w
// sharedPreRoutine is a common function used by both PreAdd and PreUpdate. It is in charge of fetching the home pod from
// the internal caches, updating its status and returning to the calling function.
func (r *PodsIncomingReflector) sharedPreRoutine(foreignPod *corev1.Pod) *corev1.Pod {
if foreignPod.Labels == nil {
return nil
}

homePodName, ok := foreignPod.Labels[virtualKubelet.ReflectedpodKey]
if !ok {
return nil
}

homeNamespace, err := r.NattingTable().DeNatNamespace(foreignPod.Namespace)
homePod, err := r.GetHomePod(foreignPod)
if err != nil {
klog.Error(err)
return nil
}

homePod, err := r.GetCacheManager().GetHomeNamespacedObject(apimgmt.Pods, homeNamespace, homePodName)
if err != nil {
err = errors.Wrap(err, "local pod not found, incoming update blocked")
klog.V(4).Info(err)
klog.Error(errors.Wrap(err, "cannot get home pod"))
return nil
}

var homePodObj interface{} = homePod
// forge.ForeignToHomeStatus blacklists the received pod if the deletionTimestamp is set
homePod, err = forge.ForeignToHomeStatus(foreignPod, homePod.(runtime.Object).DeepCopyObject())
homePodObj, err = forge.ForeignToHomeStatus(foreignPod, homePodObj.(runtime.Object).DeepCopyObject())
if err != nil {
klog.Error(err)
return nil
}

return homePod.(*corev1.Pod)
return homePodObj.(*corev1.Pod)
}

// PreDelete removes the received object from the blacklist for freeing the occupied space.
func (r *PodsIncomingReflector) PreDelete(obj interface{}) (interface{}, watch.EventType) {
foreignPod := obj.(*corev1.Pod)
foreignKey := fmt.Sprintf("%s/%s", foreignPod.Namespace, foreignPod.Name)
foreignKey := r.Keyer(foreignPod.Namespace, foreignPod.Name)
delete(reflectors.Blacklist[apimgmt.Pods], foreignKey)
klog.V(3).Infof("pod %s removed from blacklist because deleted", foreignKey)

return nil, watch.Deleted
homePod, err := r.GetHomePod(foreignPod)
if err != nil {
klog.Error(errors.Wrapf(err, "cannot get home pod for foreign pod: %s", foreignKey))
return nil, watch.Deleted
}

homePod = forge.ForeignReplicasetDeleted(homePod)

klog.V(3).Infof("%s, home pod for foreign pod: %s, set to be deleted", homePod.ObjectMeta.Name, foreignKey)

return homePod, watch.Deleted
}

// CleanupNamespace is in charge of cleaning a local namespace from all the reflected objects. All the home objects in
Expand Down Expand Up @@ -210,3 +209,37 @@ func (r *PodsIncomingReflector) isAllowed(ctx context.Context, obj interface{})
}
return !ok
}

// GetHomePod retrieves the shadow home pod given the foreign pod.
func (r *PodsIncomingReflector) GetHomePod(foreignPod *corev1.Pod) (*corev1.Pod, error) {
return r.HomePodGetter(r, foreignPod)
}

// GetHomePodFunc retrieves the shadow home pod given the foreign pod.
func GetHomePodFunc(reflector ri.APIReflector, foreignPod *corev1.Pod) (*corev1.Pod, error) {
if foreignPod.Labels == nil {
return nil, errors.New("foreign pod labels not found")
}

homePodName, ok := foreignPod.Labels[virtualKubelet.ReflectedpodKey]
if !ok {
return nil, fmt.Errorf("foreign pod label with key: %s, not found", virtualKubelet.ReflectedpodKey)
}

homeNamespace, err := reflector.NattingTable().DeNatNamespace(foreignPod.Namespace)
if err != nil {
return nil, errors.Wrap(err, "cannot get home pod namespace")
}

homePodObj, err := reflector.GetCacheManager().GetHomeNamespacedObject(apimgmt.Pods, homeNamespace, homePodName)
if err != nil {
return nil, errors.Wrap(err, "cannot get home pod from cache manager")
}

homePod, ok := homePodObj.(*corev1.Pod)
if !ok {
return nil, fmt.Errorf("could not execute type conversion: GetHomeNamespacedObject expected to return a Pod object")
}

return homePod, nil
}

0 comments on commit 4e92df3

Please sign in to comment.