diff --git a/hack/lib/test.sh b/hack/lib/test.sh index 1650a38d729..1771a82160f 100644 --- a/hack/lib/test.sh +++ b/hack/lib/test.sh @@ -42,30 +42,60 @@ kube::test::get_caller() { # Force exact match of a returned result for a object query. Wrap this with || to support multiple # valid return types. +# This runs `kubectl get` once and asserts that the result is as expected. +## $1: Object on which get should be run +# $2: The go-template to run on the result +# $3: The expected output +# $4: Additional args to be passed to kubectl kube::test::get_object_assert() { - local object=$1 - local request=$2 - local expected=$3 - local args=${4:-} + kube::test::object_assert 1 "$@" +} - res=$(eval kubectl get "${kube_flags[@]}" ${args} $object -o go-template=\"$request\") +# Asserts that the output of a given get query is as expected. +# Runs the query multiple times before failing it. +# $1: Object on which get should be run +# $2: The go-template to run on the result +# $3: The expected output +# $4: Additional args to be passed to kubectl +kube::test::wait_object_assert() { + kube::test::object_assert 10 "$@" +} - if [[ "$res" =~ ^$expected$ ]]; then - echo -n ${green} - echo "$(kube::test::get_caller): Successful get $object $request: $res" - echo -n ${reset} - return 0 - else - echo ${bold}${red} - echo "$(kube::test::get_caller): FAIL!" - echo "Get $object $request" - echo " Expected: $expected" - echo " Got: $res" - echo ${reset}${red} - caller - echo ${reset} - return 1 - fi +# Asserts that the output of a given get query is as expected. +# Can run the query multiple times before failing it. +# $1: Number of times the query should be run before failing it. +# $2: Object on which get should be run +# $3: The go-template to run on the result +# $4: The expected output +# $5: Additional args to be passed to kubectl +kube::test::object_assert() { + local tries=$1 + local object=$2 + local request=$3 + local expected=$4 + local args=${5:-} + + for j in $(seq 1 ${tries}); do + res=$(eval kubectl get "${kube_flags[@]}" ${args} $object -o go-template=\"$request\") + if [[ "$res" =~ ^$expected$ ]]; then + echo -n ${green} + echo "$(kube::test::get_caller 3): Successful get $object $request: $res" + echo -n ${reset} + return 0 + fi + echo "Waiting for Get $object $request $args: expected: $expected, got: $res" + sleep $((${j}-1)) + done + + echo ${bold}${red} + echo "$(kube::test::get_caller 3): FAIL!" + echo "Get $object $request" + echo " Expected: $expected" + echo " Got: $res" + echo ${reset}${red} + caller + echo ${reset} + return 1 } kube::test::get_object_jsonpath_assert() { diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 76a1d63f0d9..5dbf12a8dbe 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -947,8 +947,8 @@ run_kubectl_apply_deployments_tests() { # need to explicitly remove replicasets and pods because we changed the deployment selector and orphaned things kubectl delete deployments,rs,pods --all --cascade=false --grace-period=0 # Post-Condition: no Deployments, ReplicaSets, Pods exist - kube::test::get_object_assert deployments "{{range.items}}{{$id_field}}:{{end}}" '' - kube::test::get_object_assert replicasets "{{range.items}}{{$id_field}}:{{end}}" '' + kube::test::wait_object_assert deployments "{{range.items}}{{$id_field}}:{{end}}" '' + kube::test::wait_object_assert replicasets "{{range.items}}{{$id_field}}:{{end}}" '' kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' } @@ -2273,6 +2273,37 @@ run_deployment_tests() { # Clean up kubectl delete deployment test-nginx "${kube_flags[@]}" + ### Test cascading deletion + ## Test that rs is deleted when deployment is deleted. + # Pre-condition: no deployment exists + kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" '' + # Create deployment + kubectl create -f test/fixtures/doc-yaml/user-guide/deployment.yaml "${kube_flags[@]}" + # Wait for rs to come up. + kube::test::wait_object_assert rs "{{range.items}}{{$rs_replicas_field}}{{end}}" '3' + # Deleting the deployment should delete the rs. + kubectl delete deployment nginx-deployment "${kube_flags[@]}" + kube::test::wait_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" '' + + ## Test that rs is not deleted when deployment is deleted with cascade set to false. + # Pre-condition: no deployment and rs exist + kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" '' + kube::test::get_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" '' + # Create deployment + kubectl create deployment nginx-deployment --image=gcr.io/google-containers/nginx:test-cmd + # Wait for rs to come up. + kube::test::wait_object_assert rs "{{range.items}}{{$rs_replicas_field}}{{end}}" '1' + # Delete the deployment with cascade set to false. + kubectl delete deployment nginx-deployment "${kube_flags[@]}" --cascade=false + # Wait for the deployment to be deleted and then verify that rs is not + # deleted. + kube::test::wait_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" '' + kube::test::get_object_assert rs "{{range.items}}{{$rs_replicas_field}}{{end}}" '1' + # Cleanup + # Find the name of the rs to be deleted. + output_message=$(kubectl get rs "${kube_flags[@]}" -o template --template={{range.items}}{{$id_field}}{{end}}) + kubectl delete rs ${output_message} "${kube_flags[@]}" + ### Auto scale deployment # Pre-condition: no deployment exists kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" '' @@ -2385,6 +2416,21 @@ run_rs_tests() { # Post-condition: no pods from frontend replica set kube::test::get_object_assert 'pods -l "tier=frontend"' "{{range.items}}{{$id_field}}:{{end}}" '' + ### Create and then delete a replica set with cascade=false, make sure it doesn't delete pods. + # Pre-condition: no replica set exists + kube::test::get_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" '' + # Command + kubectl create -f hack/testdata/frontend-replicaset.yaml "${kube_flags[@]}" + kube::log::status "Deleting rs" + kubectl delete rs frontend "${kube_flags[@]}" --cascade=false + # Wait for the rs to be deleted. + kube::test::wait_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" '' + # Post-condition: All 3 pods still remain from frontend replica set + kube::test::get_object_assert 'pods -l "tier=frontend"' "{{range.items}}{{$pod_container_name_field}}:{{end}}" 'php-redis:php-redis:php-redis:' + # Cleanup + kubectl delete pods -l "tier=frontend" "${kube_flags[@]}" + kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' + ### Create replica set frontend from YAML # Pre-condition: no replica set exists kube::test::get_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" '' @@ -2662,7 +2708,7 @@ runTests() { i=0 create_and_use_new_namespace() { i=$(($i+1)) - kube::log::status "Creating namespace" + kube::log::status "Creating namespace namespace${i}" kubectl create namespace "namespace${i}" kubectl config set-context "${CONTEXT}" --namespace="namespace${i}" } @@ -2690,6 +2736,7 @@ runTests() { second_port_field="(index .spec.ports 1).port" second_port_name="(index .spec.ports 1).name" image_field="(index .spec.containers 0).image" + pod_container_name_field="(index .spec.containers 0).name" container_name_field="(index .spec.template.spec.containers 0).name" hpa_min_field=".spec.minReplicas" hpa_max_field=".spec.maxReplicas" diff --git a/pkg/kubectl/cmd/delete.go b/pkg/kubectl/cmd/delete.go index 1eada79b4b4..908a1304378 100644 --- a/pkg/kubectl/cmd/delete.go +++ b/pkg/kubectl/cmd/delete.go @@ -253,7 +253,8 @@ func ReapResult(r *resource.Result, f cmdutil.Factory, out io.Writer, isDefaultD if err != nil { // If there is no reaper for this resources and the user didn't explicitly ask for stop. if kubectl.IsNoSuchReaperError(err) && isDefaultDelete { - return deleteResource(info, out, shortOutput, mapper) + // No client side reaper found. Let the server do cascading deletion. + return cascadingDeleteResource(info, out, shortOutput, mapper) } return cmdutil.AddSourceToErr("reaping", info.Source, err) } @@ -293,7 +294,7 @@ func DeleteResult(r *resource.Result, out io.Writer, ignoreNotFound bool, shortO return err } found++ - return deleteResource(info, out, shortOutput, mapper) + return deleteResource(info, out, shortOutput, mapper, nil) }) if err != nil { return err @@ -304,8 +305,14 @@ func DeleteResult(r *resource.Result, out io.Writer, ignoreNotFound bool, shortO return nil } -func deleteResource(info *resource.Info, out io.Writer, shortOutput bool, mapper meta.RESTMapper) error { - if err := resource.NewHelper(info.Client, info.Mapping).Delete(info.Namespace, info.Name); err != nil { +func cascadingDeleteResource(info *resource.Info, out io.Writer, shortOutput bool, mapper meta.RESTMapper) error { + falseVar := false + deleteOptions := &metav1.DeleteOptions{OrphanDependents: &falseVar} + return deleteResource(info, out, shortOutput, mapper, deleteOptions) +} + +func deleteResource(info *resource.Info, out io.Writer, shortOutput bool, mapper meta.RESTMapper, deleteOptions *metav1.DeleteOptions) error { + if err := resource.NewHelper(info.Client, info.Mapping).DeleteWithOptions(info.Namespace, info.Name, deleteOptions); err != nil { return cmdutil.AddSourceToErr("deleting", info.Source, err) } cmdutil.PrintSuccess(mapper, shortOutput, out, info.Mapping.Resource, info.Name, false, "deleted") diff --git a/pkg/kubectl/cmd/delete_test.go b/pkg/kubectl/cmd/delete_test.go index 016785afc63..203861e8e9f 100644 --- a/pkg/kubectl/cmd/delete_test.go +++ b/pkg/kubectl/cmd/delete_test.go @@ -18,6 +18,9 @@ package cmd import ( "bytes" + "encoding/json" + "io" + "io/ioutil" "net/http" "strings" "testing" @@ -88,6 +91,68 @@ func TestDeleteObjectByTuple(t *testing.T) { } } +func hasExpectedOrphanDependents(body io.ReadCloser, expectedOrphanDependents *bool) bool { + if body == nil || expectedOrphanDependents == nil { + return body == nil && expectedOrphanDependents == nil + } + var parsedBody metav1.DeleteOptions + rawBody, _ := ioutil.ReadAll(body) + json.Unmarshal(rawBody, &parsedBody) + if parsedBody.OrphanDependents == nil { + return false + } + return *expectedOrphanDependents == *parsedBody.OrphanDependents +} + +// Tests that DeleteOptions.OrphanDependents is appropriately set while deleting objects. +func TestOrphanDependentsInDeleteObject(t *testing.T) { + initTestErrorHandler(t) + _, _, rc := testData() + + f, tf, codec, _ := cmdtesting.NewAPIFactory() + tf.Printer = &testPrinter{} + var expectedOrphanDependents *bool + tf.UnstructuredClient = &fake.RESTClient{ + APIRegistry: api.Registry, + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m, b := req.URL.Path, req.Method, req.Body; { + + case p == "/namespaces/test/secrets/mysecret" && m == "DELETE" && hasExpectedOrphanDependents(b, expectedOrphanDependents): + + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil + default: + return nil, nil + } + }), + } + tf.Namespace = "test" + + // DeleteOptions.OrphanDependents should be false, when cascade is true (default). + falseVar := false + expectedOrphanDependents = &falseVar + buf, errBuf := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) + cmd := NewCmdDelete(f, buf, errBuf) + cmd.Flags().Set("namespace", "test") + cmd.Flags().Set("output", "name") + cmd.Run(cmd, []string{"secrets/mysecret"}) + if buf.String() != "secret/mysecret\n" { + t.Errorf("unexpected output: %s", buf.String()) + } + + // Test that delete options should be nil when cascade is false. + expectedOrphanDependents = nil + buf, errBuf = bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) + cmd = NewCmdDelete(f, buf, errBuf) + cmd.Flags().Set("namespace", "test") + cmd.Flags().Set("cascade", "false") + cmd.Flags().Set("output", "name") + cmd.Run(cmd, []string{"secrets/mysecret"}) + if buf.String() != "secret/mysecret\n" { + t.Errorf("unexpected output: %s", buf.String()) + } +} + func TestDeleteNamedObject(t *testing.T) { initTestErrorHandler(t) initTestErrorHandler(t) diff --git a/pkg/kubectl/resource/helper.go b/pkg/kubectl/resource/helper.go index 5bb68018387..2cbdd847471 100644 --- a/pkg/kubectl/resource/helper.go +++ b/pkg/kubectl/resource/helper.go @@ -20,6 +20,7 @@ import ( "strconv" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -95,10 +96,15 @@ func (m *Helper) WatchSingle(namespace, name, resourceVersion string) (watch.Int } func (m *Helper) Delete(namespace, name string) error { + return m.DeleteWithOptions(namespace, name, nil) +} + +func (m *Helper) DeleteWithOptions(namespace, name string, options *metav1.DeleteOptions) error { return m.RESTClient.Delete(). NamespaceIfScoped(namespace, m.NamespaceScoped). Resource(m.Resource). Name(name). + Body(options). Do(). Error() } diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index 1924a4b6133..1fb7181c1f0 100644 --- a/pkg/kubectl/stop.go +++ b/pkg/kubectl/stop.go @@ -322,7 +322,9 @@ func (reaper *DaemonSetReaper) Stop(namespace, name string, timeout time.Duratio return err } - return reaper.client.DaemonSets(namespace).Delete(name, nil) + falseVar := false + deleteOptions := &metav1.DeleteOptions{OrphanDependents: &falseVar} + return reaper.client.DaemonSets(namespace).Delete(name, deleteOptions) } func (reaper *StatefulSetReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *metav1.DeleteOptions) error { @@ -366,7 +368,9 @@ func (reaper *StatefulSetReaper) Stop(namespace, name string, timeout time.Durat // TODO: Cleanup volumes? We don't want to accidentally delete volumes from // stop, so just leave this up to the statefulset. - return statefulsets.Delete(name, nil) + falseVar := false + deleteOptions := &metav1.DeleteOptions{OrphanDependents: &falseVar} + return statefulsets.Delete(name, deleteOptions) } func (reaper *JobReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *metav1.DeleteOptions) error { @@ -408,8 +412,10 @@ func (reaper *JobReaper) Stop(namespace, name string, timeout time.Duration, gra if len(errList) > 0 { return utilerrors.NewAggregate(errList) } - // once we have all the pods removed we can safely remove the job itself - return jobs.Delete(name, nil) + // once we have all the pods removed we can safely remove the job itself. + falseVar := false + deleteOptions := &metav1.DeleteOptions{OrphanDependents: &falseVar} + return jobs.Delete(name, deleteOptions) } func (reaper *DeploymentReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *metav1.DeleteOptions) error { @@ -509,5 +515,7 @@ func (reaper *ServiceReaper) Stop(namespace, name string, timeout time.Duration, if err != nil { return err } - return services.Delete(name, nil) + falseVar := false + deleteOptions := &metav1.DeleteOptions{OrphanDependents: &falseVar} + return services.Delete(name, deleteOptions) }