Merge pull request #17492 from tnozicka/fix-dc-reapers

Automatic merge from submit-queue.

Fix DC reaper to deal with invalid resource name.

It was deleting all RCs because apimachinery .AsSelector() ignores errors and returns MatchAll selector in that case.)

apimachinery issue: kubernetes/apimachinery#31

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1517700

Author: openshift-merge-robot

pkg/apps/cmd/delete.go

@@ -110,8 +110,9 @@ func (reaper *DeploymentConfigReaper) Stop(namespace, name string, timeout time.
 		}
 
 		// Delete all deployer and hook pods
-		options = metav1.ListOptions{LabelSelector: util.DeployerPodSelector(rc.Name).String()}
-		podList, err := reaper.kc.Core().Pods(rc.Namespace).List(options)
+		podList, err := reaper.kc.Core().Pods(rc.Namespace).List(metav1.ListOptions{
+			LabelSelector: util.DeployerPodSelector(rc.Name).String(),
+		})
 		if err != nil {
 			return err
 		}

pkg/apps/registry/deploylog/rest.go

@@ -221,7 +221,7 @@ func (r *REST) waitForExistingDeployment(namespace, name string) (*kapi.Replicat
 // returnApplicationPodName returns the best candidate pod for the target deployment in order to
 // view its logs.
 func (r *REST) returnApplicationPodName(target *kapi.ReplicationController) (string, error) {
-	selector := labels.Set(target.Spec.Selector).AsSelector()
+	selector := labels.SelectorFromValidatedSet(labels.Set(target.Spec.Selector))
 	sortBy := func(pods []*kapiv1.Pod) sort.Interface { return controller.ByLogging(pods) }
 
 	pod, _, err := kcmdutil.GetFirstPod(r.pn, target.Namespace, selector, r.timeout, sortBy)

pkg/apps/registry/rest.go

@@ -6,7 +6,6 @@ import (
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/watch"
 	kapi "k8s.io/kubernetes/pkg/api"
 	kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
@@ -26,8 +25,7 @@ var (
 // the deployment became running, complete, or failed within timeout, false if it did not, and an error if any
 // other error state occurred. The last observed deployment state is returned.
 func WaitForRunningDeployment(rn kcoreclient.ReplicationControllersGetter, observed *kapi.ReplicationController, timeout time.Duration) (*kapi.ReplicationController, bool, error) {
-	fieldSelector := fields.Set{"metadata.name": observed.Name}.AsSelector()
-	options := metav1.ListOptions{FieldSelector: fieldSelector.String(), ResourceVersion: observed.ResourceVersion}
+	options := metav1.SingleObject(observed.ObjectMeta)
 	w, err := rn.ReplicationControllers(observed.Namespace).Watch(options)
 	if err != nil {
 		return observed, false, err

pkg/apps/registry/rest_test.go

@@ -18,6 +18,7 @@ func TestWaitForRunningDeploymentSuccess(t *testing.T) {
 	fakeController := &kapi.ReplicationController{}
 	fakeController.Name = "test-1"
 	fakeController.Namespace = "test"
+	fakeController.Annotations = map[string]string{deployapi.DeploymentStatusAnnotation: string(deployapi.DeploymentStatusRunning)}
 
 	kubeclient := fake.NewSimpleClientset([]runtime.Object{fakeController}...)
 	fakeWatch := watch.NewFake()
@@ -38,7 +39,6 @@ func TestWaitForRunningDeploymentSuccess(t *testing.T) {
 		}
 	}()
 
-	fakeController.Annotations = map[string]string{deployapi.DeploymentStatusAnnotation: string(deployapi.DeploymentStatusRunning)}
 	fakeWatch.Modify(fakeController)
 	<-stopChan
 }

pkg/apps/strategy/recreate/recreate.go

@@ -255,7 +255,7 @@ func (s *RecreateDeploymentStrategy) scaleAndWait(deployment *kapi.ReplicationCo
 
 // waitForTerminatedPods waits until all pods for the provided replication controller are terminated.
 func (s *RecreateDeploymentStrategy) waitForTerminatedPods(from *kapi.ReplicationController, timeout time.Duration) {
-	selector := labels.Set(from.Spec.Selector).AsSelector()
+	selector := labels.SelectorFromValidatedSet(labels.Set(from.Spec.Selector))
 	options := metav1.ListOptions{LabelSelector: selector.String()}
 	podList, err := s.podClient.Pods(from.Namespace).List(options)
 	if err != nil {

pkg/apps/util/util.go

@@ -176,13 +176,13 @@ func DeploymentNameForConfigVersion(name string, version int64) string {
 // TODO: Using the annotation constant for now since the value is correct
 // but we could consider adding a new constant to the public types.
 func ConfigSelector(name string) labels.Selector {
-	return labels.Set{deployapi.DeploymentConfigAnnotation: name}.AsSelector()
+	return labels.SelectorFromValidatedSet(labels.Set{deployapi.DeploymentConfigAnnotation: name})
 }
 
 // DeployerPodSelector returns a label Selector which can be used to find all
 // deployer pods associated with a deployment with name.
 func DeployerPodSelector(name string) labels.Selector {
-	return labels.Set{deployapi.DeployerPodForDeploymentLabel: name}.AsSelector()
+	return labels.SelectorFromValidatedSet(labels.Set{deployapi.DeployerPodForDeploymentLabel: name})
 }
 
 // AnyDeployerPodSelector returns a label Selector which can be used to find
@@ -802,7 +802,7 @@ func WaitForRunningDeployerPod(podClient kcoreclient.PodsGetter, rc *api.Replica
 	}
 	watcher, err := podClient.Pods(rc.Namespace).Watch(
 		metav1.ListOptions{
-			FieldSelector: fields.Set{"metadata.name": podName}.AsSelector().String(),
+			FieldSelector: fields.OneTermEqualSelector("metadata.name", podName).String(),
 		},
 	)
 	if err != nil {