Skip to content
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.

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

Update statefulset reaper use StatefulSetHasDesiredReplicas #46699

Merged
merged 1 commit into from
Jun 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 11 additions & 3 deletions pkg/client/unversioned/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,23 @@ func ReplicaSetHasDesiredReplicas(rsClient extensionsclient.ReplicaSetsGetter, r
}
}

// StatefulSetHasDesiredReplicas returns a conditon that checks the number of statefulset replicas
// StatefulSetHasDesiredReplicas returns a condition that checks the number of StatefulSet replicas
func StatefulSetHasDesiredReplicas(ssClient appsclient.StatefulSetsGetter, ss *apps.StatefulSet) wait.ConditionFunc {
// TODO: Differentiate between 0 statefulset pods and a really quick scale down using generation.
// If we're given a StatefulSet where the status lags the spec, it either means that the
// StatefulSet is stale, or that the StatefulSet manager hasn't noticed the update yet.
// Polling status.Replicas is not safe in the latter case.
desiredGeneration := ss.Generation
return func() (bool, error) {
ss, err := ssClient.StatefulSets(ss.Namespace).Get(ss.Name, metav1.GetOptions{})
if err != nil {
return false, err
}
return ss.Status.Replicas == ss.Spec.Replicas, nil
// There's a chance a concurrent update modifies the Spec.Replicas causing this check to
// pass, or, after this check has passed, a modification causes the StatefulSet manager to
// create more pods. This will not be an issue once we've implemented graceful delete for
// StatefulSet, but till then concurrent stop operations on the same StatefulSet might have
// unintended side effects.
return ss.Status.ObservedGeneration != nil && *ss.Status.ObservedGeneration >= desiredGeneration && ss.Status.Replicas == ss.Spec.Replicas, nil
}
}

Expand Down
142 changes: 69 additions & 73 deletions pkg/kubectl/BUILD
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
package(default_visibility = ["//visibility:public"])

licenses(["notice"])

load(
Expand All @@ -6,6 +8,72 @@ load(
"go_test",
)

go_test(
name = "go_default_test",
srcs = [
"cluster_test.go",
"configmap_test.go",
"deployment_test.go",
"generate_test.go",
"kubectl_test.go",
"namespace_test.go",
"proxy_server_test.go",
"quota_test.go",
"rolebinding_test.go",
"rolling_updater_test.go",
"rollout_status_test.go",
"run_test.go",
"scale_test.go",
"secret_for_docker_registry_test.go",
"secret_for_tls_test.go",
"secret_test.go",
"service_basic_test.go",
"service_test.go",
"serviceaccount_test.go",
"sorting_printer_test.go",
"stop_test.go",
],
library = ":go_default_library",
tags = ["automanaged"],
deps = [
"//federation/apis/federation/v1beta1:go_default_library",
"//pkg/api:go_default_library",
"//pkg/api/testapi:go_default_library",
"//pkg/api/testing:go_default_library",
"//pkg/api/v1:go_default_library",
"//pkg/apis/apps:go_default_library",
"//pkg/apis/apps/v1beta1:go_default_library",
"//pkg/apis/batch:go_default_library",
"//pkg/apis/batch/v1:go_default_library",
"//pkg/apis/batch/v2alpha1:go_default_library",
"//pkg/apis/extensions:go_default_library",
"//pkg/apis/extensions/v1beta1:go_default_library",
"//pkg/apis/rbac:go_default_library",
"//pkg/client/clientset_generated/internalclientset:go_default_library",
"//pkg/client/clientset_generated/internalclientset/fake:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/batch/internalversion:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/extensions/internalversion:go_default_library",
"//pkg/kubectl/util:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/watch:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
"//vendor/k8s.io/client-go/rest/fake:go_default_library",
"//vendor/k8s.io/client-go/testing:go_default_library",
"//vendor/k8s.io/client-go/util/testing:go_default_library",
],
)

go_library(
name = "go_default_library",
srcs = [
Expand Down Expand Up @@ -45,10 +113,6 @@ go_library(
"versioned_client.go",
],
tags = ["automanaged"],
visibility = [
"//build/visible_to:COMMON_testing",
"//build/visible_to:pkg_kubectl_CONSUMERS",
],
deps = [
"//federation/apis/federation/v1beta1:go_default_library",
"//pkg/api:go_default_library",
Expand Down Expand Up @@ -115,76 +179,11 @@ go_library(
],
)

go_test(
name = "go_default_test",
srcs = [
"cluster_test.go",
"configmap_test.go",
"deployment_test.go",
"generate_test.go",
"kubectl_test.go",
"namespace_test.go",
"proxy_server_test.go",
"quota_test.go",
"rolebinding_test.go",
"rolling_updater_test.go",
"rollout_status_test.go",
"run_test.go",
"scale_test.go",
"secret_for_docker_registry_test.go",
"secret_for_tls_test.go",
"secret_test.go",
"service_basic_test.go",
"service_test.go",
"serviceaccount_test.go",
"sorting_printer_test.go",
"stop_test.go",
],
library = ":go_default_library",
tags = ["automanaged"],
deps = [
"//federation/apis/federation/v1beta1:go_default_library",
"//pkg/api:go_default_library",
"//pkg/api/testapi:go_default_library",
"//pkg/api/testing:go_default_library",
"//pkg/api/v1:go_default_library",
"//pkg/apis/apps:go_default_library",
"//pkg/apis/apps/v1beta1:go_default_library",
"//pkg/apis/batch:go_default_library",
"//pkg/apis/batch/v1:go_default_library",
"//pkg/apis/batch/v2alpha1:go_default_library",
"//pkg/apis/extensions:go_default_library",
"//pkg/apis/extensions/v1beta1:go_default_library",
"//pkg/apis/rbac:go_default_library",
"//pkg/client/clientset_generated/internalclientset:go_default_library",
"//pkg/client/clientset_generated/internalclientset/fake:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/batch/internalversion:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/extensions/internalversion:go_default_library",
"//pkg/kubectl/util:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/watch:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
"//vendor/k8s.io/client-go/rest/fake:go_default_library",
"//vendor/k8s.io/client-go/testing:go_default_library",
"//vendor/k8s.io/client-go/util/testing:go_default_library",
],
)

filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)

filegroup(
Expand All @@ -199,7 +198,4 @@ filegroup(
"//pkg/kubectl/util:all-srcs",
],
tags = ["automanaged"],
visibility = [
"//build/visible_to:pkg_kubectl_CONSUMERS",
],
)
29 changes: 0 additions & 29 deletions pkg/kubectl/stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
batchclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/batch/internalversion"
coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
extensionsclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/extensions/internalversion"
"k8s.io/kubernetes/pkg/controller"
deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
"k8s.io/kubernetes/pkg/util"
)
Expand Down Expand Up @@ -345,34 +344,6 @@ func (reaper *StatefulSetReaper) Stop(namespace, name string, timeout time.Durat
return err
}

// TODO: This shouldn't be needed, see corresponding TODO in StatefulSetHasDesiredReplicas.
// StatefulSet should track generation number.
pods := reaper.podClient.Pods(namespace)
selector, _ := metav1.LabelSelectorAsSelector(ss.Spec.Selector)
options := metav1.ListOptions{LabelSelector: selector.String()}
podList, err := pods.List(options)
if err != nil {
return err
}

errList := []error{}
for i := range podList.Items {
pod := &podList.Items[i]
controllerRef := controller.GetControllerOf(pod)
// Ignore Pod if it's an orphan or owned by someone else.
if controllerRef == nil || controllerRef.UID != ss.UID {
continue
}
if err := pods.Delete(pod.Name, gracePeriod); err != nil {
if !errors.IsNotFound(err) {
errList = append(errList, err)
}
}
}
if len(errList) > 0 {
return utilerrors.NewAggregate(errList)
}

// TODO: Cleanup volumes? We don't want to accidentally delete volumes from
// stop, so just leave this up to the statefulset.
falseVar := false
Expand Down