Merge pull request #49861 from krmayankk/disrupt

Automatic merge from submit-queue

simplify disruption controller finder logic

**What this PR does / why we need it**:
Address some comments from https://github.com/kubernetes/kubernetes/pull/45003 and simplify the PDB controller logic as part of issue https://github.com/kubernetes/kubernetes/issues/42284

@enisoc @kargakis @caesarxuchao 

Also it feels like we can get rid of the finders all together since with controller ref, each pod has only controller. Let me know if i should remove that finders all together ?
This commit is contained in:
Kubernetes Submit Queue 2017-08-27 21:45:17 -07:00 committed by GitHub
commit ecd910ea6c

View File

@ -107,7 +107,7 @@ type controllerAndScale struct {
// podControllerFinder is a function type that maps a pod to a list of
// controllers and their scale.
type podControllerFinder func(*v1.Pod) ([]controllerAndScale, error)
type podControllerFinder func(*v1.Pod) (*controllerAndScale, error)
func NewDisruptionController(
podInformer coreinformers.PodInformer,
@ -170,8 +170,8 @@ func NewDisruptionController(
// and we may well need further tweaks just to be able to access scale
// subresources.
func (dc *DisruptionController) finders() []podControllerFinder {
return []podControllerFinder{dc.getPodReplicationControllers, dc.getPodDeployments, dc.getPodReplicaSets,
dc.getPodStatefulSets}
return []podControllerFinder{dc.getPodReplicationController, dc.getPodDeployment, dc.getPodReplicaSet,
dc.getPodStatefulSet}
}
var (
@ -181,9 +181,8 @@ var (
controllerKindDep = v1beta1.SchemeGroupVersion.WithKind("Deployment")
)
// getPodReplicaSets finds replicasets which have no matching deployments.
func (dc *DisruptionController) getPodReplicaSets(pod *v1.Pod) ([]controllerAndScale, error) {
var casSlice []controllerAndScale
// getPodReplicaSet finds a replicaset which has no matching deployments.
func (dc *DisruptionController) getPodReplicaSet(pod *v1.Pod) (*controllerAndScale, error) {
controllerRef := metav1.GetControllerOf(pod)
if controllerRef == nil {
return nil, nil
@ -204,13 +203,11 @@ func (dc *DisruptionController) getPodReplicaSets(pod *v1.Pod) ([]controllerAndS
// Skip RS if it's controlled by a Deployment.
return nil, nil
}
casSlice = append(casSlice, controllerAndScale{rs.UID, *(rs.Spec.Replicas)})
return casSlice, nil
return &controllerAndScale{rs.UID, *(rs.Spec.Replicas)}, nil
}
// getPodStatefulSet returns the statefulset managing the given pod.
func (dc *DisruptionController) getPodStatefulSets(pod *v1.Pod) ([]controllerAndScale, error) {
var casSlice []controllerAndScale
func (dc *DisruptionController) getPodStatefulSet(pod *v1.Pod) (*controllerAndScale, error) {
controllerRef := metav1.GetControllerOf(pod)
if controllerRef == nil {
return nil, nil
@ -227,13 +224,11 @@ func (dc *DisruptionController) getPodStatefulSets(pod *v1.Pod) ([]controllerAnd
return nil, nil
}
casSlice = append(casSlice, controllerAndScale{ss.UID, *(ss.Spec.Replicas)})
return casSlice, nil
return &controllerAndScale{ss.UID, *(ss.Spec.Replicas)}, nil
}
// getPodDeployments finds deployments for any replicasets which are being managed by deployments.
func (dc *DisruptionController) getPodDeployments(pod *v1.Pod) ([]controllerAndScale, error) {
var casSlice []controllerAndScale
func (dc *DisruptionController) getPodDeployment(pod *v1.Pod) (*controllerAndScale, error) {
controllerRef := metav1.GetControllerOf(pod)
if controllerRef == nil {
return nil, nil
@ -264,12 +259,10 @@ func (dc *DisruptionController) getPodDeployments(pod *v1.Pod) ([]controllerAndS
if deployment.UID != controllerRef.UID {
return nil, nil
}
casSlice = append(casSlice, controllerAndScale{deployment.UID, *(deployment.Spec.Replicas)})
return casSlice, nil
return &controllerAndScale{deployment.UID, *(deployment.Spec.Replicas)}, nil
}
func (dc *DisruptionController) getPodReplicationControllers(pod *v1.Pod) ([]controllerAndScale, error) {
var casSlice []controllerAndScale
func (dc *DisruptionController) getPodReplicationController(pod *v1.Pod) (*controllerAndScale, error) {
controllerRef := metav1.GetControllerOf(pod)
if controllerRef == nil {
return nil, nil
@ -285,8 +278,7 @@ func (dc *DisruptionController) getPodReplicationControllers(pod *v1.Pod) ([]con
if rc.UID != controllerRef.UID {
return nil, nil
}
casSlice = append(casSlice, controllerAndScale{rc.UID, *(rc.Spec.Replicas)})
return casSlice, nil
return &controllerAndScale{rc.UID, *(rc.Spec.Replicas)}, nil
}
func (dc *DisruptionController) Run(stopCh <-chan struct{}) {
@ -590,30 +582,26 @@ func (dc *DisruptionController) getExpectedScale(pdb *policy.PodDisruptionBudget
// A mapping from controllers to their scale.
controllerScale := map[types.UID]int32{}
// 1. Find the controller(s) for each pod. If any pod has 0 controllers,
// that's an error. If any pod has more than 1 controller, that's also an
// error.
// 1. Find the controller for each pod. If any pod has 0 controllers,
// that's an error. With ControllerRef, a pod can only have 1 controller.
for _, pod := range pods {
controllerCount := 0
foundController := false
for _, finder := range dc.finders() {
var controllers []controllerAndScale
controllers, err = finder(pod)
var controllerNScale *controllerAndScale
controllerNScale, err = finder(pod)
if err != nil {
return
}
for _, controller := range controllers {
controllerScale[controller.UID] = controller.scale
controllerCount++
if controllerNScale != nil {
controllerScale[controllerNScale.UID] = controllerNScale.scale
foundController = true
break
}
}
if controllerCount == 0 {
if !foundController {
err = fmt.Errorf("found no controllers for pod %q", pod.Name)
dc.recorder.Event(pdb, v1.EventTypeWarning, "NoControllers", err.Error())
return
} else if controllerCount > 1 {
err = fmt.Errorf("pod %q has %v>1 controllers", pod.Name, controllerCount)
dc.recorder.Event(pdb, v1.EventTypeWarning, "TooManyControllers", err.Error())
return
}
}