Skip to content

Commit

Permalink
Merge pull request #63979 from soltysh/drop_reapers
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 63859, 63979). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Drop reapers

/assign @deads2k @juanvallejo 

**Release note**:
```release-note
kubectl delete does not use reapers for removing objects anymore, but relies on server-side GC entirely
```
  • Loading branch information
Kubernetes Submit Queue committed May 26, 2018
2 parents 5e88401 + 2a76535 commit 5f578f3
Show file tree
Hide file tree
Showing 45 changed files with 253 additions and 1,889 deletions.
29 changes: 4 additions & 25 deletions hack/make-rules/test-cmd-util.sh
Expand Up @@ -1468,7 +1468,7 @@ __EOF__
# Test that we can list this new CustomResource
kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" ''
# Compare "old" output with experimental output and ensure both are the same
expected_output=$(kubectl get foos "${kube_flags[@]}")
expected_output=$(kubectl get foos "${kube_flags[@]}" | awk 'NF{NF--};1')
actual_output=$(kubectl get foos --server-print=false "${kube_flags[@]}" | awk 'NF{NF--};1')
kube::test::if_has_string "${actual_output}" "${expected_output}"

Expand All @@ -1480,6 +1480,9 @@ __EOF__
kubectl delete rc frontend "${kube_flags[@]}"
kubectl delete ds bind "${kube_flags[@]}"
kubectl delete pod valid-pod "${kube_flags[@]}"

set +o nounset
set +o errexit
}

run_kubectl_get_tests() {
Expand Down Expand Up @@ -3206,18 +3209,6 @@ run_deployment_tests() {
kubectl delete configmap test-set-env-config "${kube_flags[@]}"
kubectl delete secret test-set-env-secret "${kube_flags[@]}"

### Delete a deployment with initializer
# Pre-condition: no deployment exists
kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" ''
# Create a deployment
kubectl create --request-timeout=1 -f hack/testdata/deployment-with-initializer.yaml 2>&1 "${kube_flags[@]}" || true
kube::test::get_object_assert 'deployment web' "{{$id_field}}" 'web'
# Delete a deployment
kubectl delete deployment web "${kube_flags[@]}"
# Check Deployment web doesn't exist
output_message=$(! kubectl get deployment web 2>&1 "${kube_flags[@]}")
kube::test::if_has_string "${output_message}" '"web" not found'

set +o nounset
set +o errexit
}
Expand Down Expand Up @@ -3359,18 +3350,6 @@ run_rs_tests() {
# Post-condition: no replica set exists
kube::test::get_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" ''

### Delete a rs with initializer
# Pre-condition: no rs exists
kube::test::get_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" ''
# Create a rs
kubectl create --request-timeout=1 -f hack/testdata/replicaset-with-initializer.yaml 2>&1 "${kube_flags[@]}" || true
kube::test::get_object_assert 'rs nginx' "{{$id_field}}" 'nginx'
# Delete a rs
kubectl delete rs nginx "${kube_flags[@]}"
# check rs nginx doesn't exist
output_message=$(! kubectl get rs nginx 2>&1 "${kube_flags[@]}")
kube::test::if_has_string "${output_message}" '"nginx" not found'

if kube::test::if_supports_resource "${horizontalpodautoscalers}" ; then
### Auto scale replica set
# Pre-condition: no replica set exists
Expand Down
25 changes: 0 additions & 25 deletions hack/testdata/deployment-with-initializer.yaml

This file was deleted.

23 changes: 0 additions & 23 deletions hack/testdata/replicaset-with-initializer.yaml

This file was deleted.

12 changes: 0 additions & 12 deletions pkg/kubectl/BUILD
Expand Up @@ -12,7 +12,6 @@ go_test(
"autoscale_test.go",
"clusterrolebinding_test.go",
"configmap_test.go",
"delete_test.go",
"deployment_test.go",
"env_file_test.go",
"generate_test.go",
Expand Down Expand Up @@ -40,12 +39,9 @@ go_test(
"//pkg/api/legacyscheme:go_default_library",
"//pkg/api/testapi:go_default_library",
"//pkg/api/testing:go_default_library",
"//pkg/apis/batch:go_default_library",
"//pkg/apis/core:go_default_library",
"//pkg/apis/extensions: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/core/internalversion:go_default_library",
"//pkg/kubectl/util:go_default_library",
"//pkg/util/pointer:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
Expand All @@ -71,8 +67,6 @@ go_test(
"//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/kubernetes/fake:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
"//vendor/k8s.io/client-go/rest/fake:go_default_library",
Expand All @@ -92,7 +86,6 @@ go_library(
"clusterrolebinding.go",
"conditions.go",
"configmap.go",
"delete.go",
"deployment.go",
"doc.go",
"env_file.go",
Expand Down Expand Up @@ -123,19 +116,15 @@ go_library(
"//pkg/api/pod:go_default_library",
"//pkg/api/v1/pod:go_default_library",
"//pkg/apis/apps:go_default_library",
"//pkg/apis/batch:go_default_library",
"//pkg/apis/core:go_default_library",
"//pkg/apis/core/v1:go_default_library",
"//pkg/apis/extensions:go_default_library",
"//pkg/client/clientset_generated/internalclientset:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/apps/internalversion: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/controller/deployment/util:go_default_library",
"//pkg/credentialprovider:go_default_library",
"//pkg/kubectl/apps:go_default_library",
"//pkg/kubectl/cmd/scalejob:go_default_library",
"//pkg/kubectl/genericclioptions/resource:go_default_library",
"//pkg/kubectl/util:go_default_library",
"//pkg/kubectl/util/hash:go_default_library",
Expand Down Expand Up @@ -171,7 +160,6 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/watch:go_default_library",
Expand Down
6 changes: 3 additions & 3 deletions pkg/kubectl/cmd/BUILD
Expand Up @@ -105,6 +105,7 @@ go_library(
"//vendor/github.com/renstrom/dedent:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/github.com/spf13/pflag:go_default_library",
"//vendor/k8s.io/api/apps/v1:go_default_library",
"//vendor/k8s.io/api/autoscaling/v1:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/api/policy/v1beta1:go_default_library",
Expand All @@ -126,6 +127,7 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/yaml:go_default_library",
Expand Down Expand Up @@ -197,7 +199,6 @@ go_test(
"//pkg/apis/core:go_default_library",
"//pkg/apis/extensions:go_default_library",
"//pkg/client/clientset_generated/internalclientset:go_default_library",
"//pkg/kubectl:go_default_library",
"//pkg/kubectl/cmd/create:go_default_library",
"//pkg/kubectl/cmd/testing:go_default_library",
"//pkg/kubectl/cmd/util:go_default_library",
Expand All @@ -214,7 +215,6 @@ go_test(
"//vendor/github.com/googleapis/gnostic/OpenAPIv2:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/gopkg.in/yaml.v2:go_default_library",
"//vendor/k8s.io/api/autoscaling/v1:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/api/policy/v1beta1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
Expand All @@ -232,9 +232,9 @@ go_test(
"//vendor/k8s.io/apimachinery/pkg/util/strategicpatch/testing:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/version:go_default_library",
"//vendor/k8s.io/client-go/dynamic/fake: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/scale/fake:go_default_library",
"//vendor/k8s.io/client-go/testing:go_default_library",
"//vendor/k8s.io/client-go/tools/remotecommand:go_default_library",
"//vendor/k8s.io/metrics/pkg/apis/metrics/v1alpha1:go_default_library",
Expand Down
52 changes: 15 additions & 37 deletions pkg/kubectl/cmd/apply.go
Expand Up @@ -43,7 +43,6 @@ import (
scaleclient "k8s.io/client-go/scale"
oapi "k8s.io/kube-openapi/pkg/util/proto"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
Expand Down Expand Up @@ -82,7 +81,6 @@ type ApplyOptions struct {
Mapper meta.RESTMapper
Scaler scaleclient.ScalesGetter
DynamicClient dynamic.Interface
ClientSetFunc func() (internalclientset.Interface, error)
OpenAPISchema openapi.Resources

Namespace string
Expand Down Expand Up @@ -215,7 +213,6 @@ func (o *ApplyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
o.ShouldIncludeUninitialized = cmdutil.ShouldIncludeUninitialized(cmd, o.Prune)

o.OpenAPISchema, _ = f.OpenAPISchema()
o.ClientSetFunc = f.ClientSet
o.Validator, err = f.Validator(cmdutil.GetFlagBool(cmd, "validate"))
o.Builder = f.NewBuilder()
o.Mapper, err = f.ToRESTMapper()
Expand Down Expand Up @@ -406,15 +403,13 @@ func (o *ApplyOptions) Run() error {
mapping: info.Mapping,
helper: helper,
dynamicClient: o.DynamicClient,
clientsetFunc: o.ClientSetFunc,
overwrite: o.Overwrite,
backOff: clockwork.NewRealClock(),
force: o.DeleteOptions.ForceDeletion,
cascade: o.DeleteOptions.Cascade,
timeout: o.DeleteOptions.Timeout,
gracePeriod: o.DeleteOptions.GracePeriod,
openapiSchema: openapiSchema,
scaleClient: o.Scaler,
}

patchBytes, patchedObject, err := patcher.patch(info.Object, modified, info.Source, info.Namespace, info.Name, o.ErrOut)
Expand Down Expand Up @@ -491,7 +486,6 @@ func (o *ApplyOptions) Run() error {
p := pruner{
mapper: o.Mapper,
dynamicClient: o.DynamicClient,
clientsetFunc: o.ClientSetFunc,

labelSelector: o.Selector,
visitedUids: visitedUids,
Expand Down Expand Up @@ -580,7 +574,6 @@ func getRESTMappings(mapper meta.RESTMapper, pruneResources *[]pruneResource) (n
type pruner struct {
mapper meta.RESTMapper
dynamicClient dynamic.Interface
clientsetFunc func() (internalclientset.Interface, error)

visitedUids sets.String
labelSelector string
Expand Down Expand Up @@ -630,7 +623,7 @@ func (p *pruner) prune(namespace string, mapping *meta.RESTMapping, includeUnini
}
name := metadata.GetName()
if !p.dryRun {
if err := p.delete(namespace, name, mapping, p.scaler); err != nil {
if err := p.delete(namespace, name, mapping); err != nil {
return err
}
}
Expand All @@ -644,44 +637,31 @@ func (p *pruner) prune(namespace string, mapping *meta.RESTMapping, includeUnini
return nil
}

func (p *pruner) delete(namespace, name string, mapping *meta.RESTMapping, scaleClient scaleclient.ScalesGetter) error {
return runDelete(namespace, name, mapping, p.dynamicClient, p.cascade, p.gracePeriod, p.clientsetFunc, scaleClient)
func (p *pruner) delete(namespace, name string, mapping *meta.RESTMapping) error {
return runDelete(namespace, name, mapping, p.dynamicClient, p.cascade, p.gracePeriod)
}

func runDelete(namespace, name string, mapping *meta.RESTMapping, c dynamic.Interface, cascade bool, gracePeriod int, clientsetFunc func() (internalclientset.Interface, error), scaleClient scaleclient.ScalesGetter) error {
if !cascade {
return c.Resource(mapping.Resource).Namespace(namespace).Delete(name, nil)
}
cs, err := clientsetFunc()
if err != nil {
return err
}
r, err := kubectl.ReaperFor(mapping.GroupVersionKind.GroupKind(), cs, scaleClient)
if err != nil {
if _, ok := err.(*kubectl.NoSuchReaperError); !ok {
return err
}
return c.Resource(mapping.Resource).Namespace(namespace).Delete(name, nil)
}
var options *metav1.DeleteOptions
func runDelete(namespace, name string, mapping *meta.RESTMapping, c dynamic.Interface, cascade bool, gracePeriod int) error {
options := &metav1.DeleteOptions{}
if gracePeriod >= 0 {
options = metav1.NewDeleteOptions(int64(gracePeriod))
}
if err := r.Stop(namespace, name, 2*time.Minute, options); err != nil {
return err
policy := metav1.DeletePropagationForeground
if !cascade {
policy = metav1.DeletePropagationOrphan
}
return nil
options.PropagationPolicy = &policy
return c.Resource(mapping.Resource).Namespace(namespace).Delete(name, options)
}

func (p *patcher) delete(namespace, name string) error {
return runDelete(namespace, name, p.mapping, p.dynamicClient, p.cascade, p.gracePeriod, p.clientsetFunc, p.scaleClient)
return runDelete(namespace, name, p.mapping, p.dynamicClient, p.cascade, p.gracePeriod)
}

type patcher struct {
mapping *meta.RESTMapping
helper *resource.Helper
dynamicClient dynamic.Interface
clientsetFunc func() (internalclientset.Interface, error)

overwrite bool
backOff clockwork.Clock
Expand All @@ -692,7 +672,6 @@ type patcher struct {
gracePeriod int

openapiSchema openapi.Resources
scaleClient scaleclient.ScalesGetter
}

func (p *patcher) patchSimple(obj runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) {
Expand Down Expand Up @@ -790,17 +769,16 @@ func (p *patcher) patch(current runtime.Object, modified []byte, source, namespa
}

func (p *patcher) deleteAndCreate(original runtime.Object, modified []byte, namespace, name string) ([]byte, runtime.Object, error) {
err := p.delete(namespace, name)
if err != nil {
if err := p.delete(namespace, name); err != nil {
return modified, nil, err
}
err = wait.PollImmediate(kubectl.Interval, p.timeout, func() (bool, error) {
// TODO: use wait
if err := wait.PollImmediate(1*time.Second, p.timeout, func() (bool, error) {
if _, err := p.helper.Get(namespace, name, false); !errors.IsNotFound(err) {
return false, err
}
return true, nil
})
if err != nil {
}); err != nil {
return modified, nil, err
}
versionedObject, _, err := unstructured.UnstructuredJSONScheme.Decode(modified, nil, nil)
Expand Down

0 comments on commit 5f578f3

Please sign in to comment.