From ba7e3e4d4c172ab7a68c00b5eaa694426db10cd8 Mon Sep 17 00:00:00 2001 From: Tomas Nozicka Date: Tue, 28 Nov 2017 17:49:28 +0100 Subject: [PATCH 1/2] 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.) --- pkg/apps/cmd/delete.go | 5 +++-- pkg/apps/registry/deploylog/rest.go | 2 +- pkg/apps/registry/rest.go | 4 +--- pkg/apps/strategy/recreate/recreate.go | 2 +- pkg/apps/util/util.go | 6 +++--- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/apps/cmd/delete.go b/pkg/apps/cmd/delete.go index b620c8b65024..e7f55e8d4ca1 100644 --- a/pkg/apps/cmd/delete.go +++ b/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 } diff --git a/pkg/apps/registry/deploylog/rest.go b/pkg/apps/registry/deploylog/rest.go index 6efea24e5bf3..09789da0e76f 100644 --- a/pkg/apps/registry/deploylog/rest.go +++ b/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) diff --git a/pkg/apps/registry/rest.go b/pkg/apps/registry/rest.go index e85cc787133d..4e816eb18b88 100644 --- a/pkg/apps/registry/rest.go +++ b/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 diff --git a/pkg/apps/strategy/recreate/recreate.go b/pkg/apps/strategy/recreate/recreate.go index ac072c943757..a3aea48ea678 100644 --- a/pkg/apps/strategy/recreate/recreate.go +++ b/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 { diff --git a/pkg/apps/util/util.go b/pkg/apps/util/util.go index 8b2b805e653a..05e30794e0b2 100644 --- a/pkg/apps/util/util.go +++ b/pkg/apps/util/util.go @@ -175,13 +175,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 @@ -837,7 +837,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 { From 7eb3628de136de2816cb0d7f26150d003ed8359d Mon Sep 17 00:00:00 2001 From: Tomas Nozicka Date: Wed, 29 Nov 2017 15:09:42 +0100 Subject: [PATCH 2/2] Fix TestWaitForRunningDeploymentSuccess race --- pkg/apps/registry/rest_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apps/registry/rest_test.go b/pkg/apps/registry/rest_test.go index 82e0116b0c08..0a307b6db2d6 100644 --- a/pkg/apps/registry/rest_test.go +++ b/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 }