From 88e17d74780c27fd90c94cc2f0942789637b17cf Mon Sep 17 00:00:00 2001 From: Michael Fraenkel Date: Wed, 31 Aug 2016 14:48:47 -0400 Subject: [PATCH 1/2] Require force when using grace period or timeout --- hack/make-rules/test-cmd.sh | 9 +++++++++ pkg/kubectl/cmd/replace.go | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index e77b06dfe9e..3fdbdbb6116 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -750,6 +750,15 @@ runTests() { kubectl replace "${kube_flags[@]}" --force -f /tmp/tmp-valid-pod.json # Post-condition: spec.container.name = "replaced-k8s-serve-hostname" kube::test::get_object_assert 'pod valid-pod' "{{(index .spec.containers 0).name}}" 'replaced-k8s-serve-hostname' + + ## check replace --grace-period requires --force + output_message=$(! kubectl replace "${kube_flags[@]}" --grace-period=1 -f /tmp/tmp-valid-pod.json 2>&1) + kube::test::if_has_string "${output_message}" '\-\-grace-period must have \-\-force specified' + + ## check replace --timeout requires --force + output_message=$(! kubectl replace "${kube_flags[@]}" --timeout=1s -f /tmp/tmp-valid-pod.json 2>&1) + kube::test::if_has_string "${output_message}" '\-\-timeout must have \-\-force specified' + #cleaning rm /tmp/tmp-valid-pod.json diff --git a/pkg/kubectl/cmd/replace.go b/pkg/kubectl/cmd/replace.go index a16d4591673..f75e94cf3f7 100644 --- a/pkg/kubectl/cmd/replace.go +++ b/pkg/kubectl/cmd/replace.go @@ -120,6 +120,14 @@ func RunReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []st return forceReplace(f, out, cmd, args, shortOutput, options) } + if cmdutil.GetFlagInt(cmd, "grace-period") >= 0 { + return fmt.Errorf("--grace-period must have --force specified") + } + + if cmdutil.GetFlagDuration(cmd, "timeout") != 0 { + return fmt.Errorf("--timeout must have --force specified") + } + mapper, typer := f.Object(cmdutil.GetIncludeThirdPartyAPIs(cmd)) r := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)). Schema(schema). From c036d28689fb00f1033dc108338d76fa1a7dd56e Mon Sep 17 00:00:00 2001 From: Michael Fraenkel Date: Wed, 31 Aug 2016 20:12:19 -0400 Subject: [PATCH 2/2] Wait until resource is deleted before creating --- pkg/kubectl/cmd/replace.go | 21 +++++++++++++- pkg/kubectl/cmd/replace_test.go | 51 +++++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/pkg/kubectl/cmd/replace.go b/pkg/kubectl/cmd/replace.go index f75e94cf3f7..f1bd13e4f12 100644 --- a/pkg/kubectl/cmd/replace.go +++ b/pkg/kubectl/cmd/replace.go @@ -27,10 +27,12 @@ import ( "github.com/spf13/cobra" "github.com/golang/glog" + "k8s.io/kubernetes/pkg/api/errors" "k8s.io/kubernetes/pkg/kubectl" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/wait" ) // ReplaceOptions is the start of the data required to perform the operation. As new fields are added, add them here instead of @@ -213,10 +215,11 @@ func forceReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args [] } //Replace will create a resource if it doesn't exist already, so ignore not found error ignoreNotFound := true + timeout := cmdutil.GetFlagDuration(cmd, "timeout") // By default use a reaper to delete all related resources. if cmdutil.GetFlagBool(cmd, "cascade") { glog.Warningf("\"cascade\" is set, kubectl will delete and re-create all resources managed by this resource (e.g. Pods created by a ReplicationController). Consider using \"kubectl rolling-update\" if you want to update a ReplicationController together with its Pods.") - err = ReapResult(r, f, out, cmdutil.GetFlagBool(cmd, "cascade"), ignoreNotFound, cmdutil.GetFlagDuration(cmd, "timeout"), cmdutil.GetFlagInt(cmd, "grace-period"), shortOutput, mapper, false) + err = ReapResult(r, f, out, cmdutil.GetFlagBool(cmd, "cascade"), ignoreNotFound, timeout, cmdutil.GetFlagInt(cmd, "grace-period"), shortOutput, mapper, false) } else { err = DeleteResult(r, out, ignoreNotFound, shortOutput, mapper) } @@ -224,6 +227,22 @@ func forceReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args [] return err } + if timeout == 0 { + timeout = kubectl.Timeout + } + r.Visit(func(info *resource.Info, err error) error { + if err != nil { + return err + } + + return wait.PollImmediate(kubectl.Interval, timeout, func() (bool, error) { + if err := info.Get(); !errors.IsNotFound(err) { + return false, err + } + return true, nil + }) + }) + r = resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), runtime.UnstructuredJSONScheme). Schema(schema). ContinueOnError(). diff --git a/pkg/kubectl/cmd/replace_test.go b/pkg/kubectl/cmd/replace_test.go index 992729079db..bfdfab53d2f 100644 --- a/pkg/kubectl/cmd/replace_test.go +++ b/pkg/kubectl/cmd/replace_test.go @@ -32,12 +32,22 @@ func TestReplaceObject(t *testing.T) { f, tf, codec, _ := NewAPIFactory() ns := dynamic.ContentConfig().NegotiatedSerializer tf.Printer = &testPrinter{} + deleted := false tf.Client = &fake.RESTClient{ NegotiatedSerializer: ns, Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { switch p, m := req.URL.Path, req.Method; { - case p == "/namespaces/test/replicationcontrollers/redis-master" && (m == http.MethodGet || m == http.MethodPut || m == http.MethodDelete): + case p == "/namespaces/test/replicationcontrollers/redis-master" && m == http.MethodDelete: + deleted = true + fallthrough + case p == "/namespaces/test/replicationcontrollers/redis-master" && m == http.MethodPut: return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil + case p == "/namespaces/test/replicationcontrollers/redis-master" && m == http.MethodGet: + statusCode := http.StatusOK + if deleted { + statusCode = http.StatusNotFound + } + return &http.Response{StatusCode: statusCode, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil case p == "/namespaces/test/replicationcontrollers" && m == http.MethodPost: return &http.Response{StatusCode: http.StatusCreated, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil default: @@ -76,16 +86,36 @@ func TestReplaceMultipleObject(t *testing.T) { f, tf, codec, _ := NewAPIFactory() ns := dynamic.ContentConfig().NegotiatedSerializer tf.Printer = &testPrinter{} + redisMasterDeleted := false + frontendDeleted := false tf.Client = &fake.RESTClient{ NegotiatedSerializer: ns, Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { switch p, m := req.URL.Path, req.Method; { - case p == "/namespaces/test/replicationcontrollers/redis-master" && (m == http.MethodGet || m == http.MethodPut || m == http.MethodDelete): + case p == "/namespaces/test/replicationcontrollers/redis-master" && m == http.MethodDelete: + redisMasterDeleted = true + fallthrough + case p == "/namespaces/test/replicationcontrollers/redis-master" && m == http.MethodPut: return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil + case p == "/namespaces/test/replicationcontrollers/redis-master" && m == http.MethodGet: + statusCode := http.StatusOK + if redisMasterDeleted { + statusCode = http.StatusNotFound + } + return &http.Response{StatusCode: statusCode, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil case p == "/namespaces/test/replicationcontrollers" && m == http.MethodPost: return &http.Response{StatusCode: http.StatusCreated, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil - case p == "/namespaces/test/services/frontend" && (m == http.MethodGet || m == http.MethodPut || m == http.MethodDelete): + case p == "/namespaces/test/services/frontend" && m == http.MethodDelete: + frontendDeleted = true + fallthrough + case p == "/namespaces/test/services/frontend" && m == http.MethodPut: return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &svc.Items[0])}, nil + case p == "/namespaces/test/services/frontend" && m == http.MethodGet: + statusCode := http.StatusOK + if frontendDeleted { + statusCode = http.StatusNotFound + } + return &http.Response{StatusCode: statusCode, Header: defaultHeader(), Body: objBody(codec, &svc.Items[0])}, nil case p == "/namespaces/test/services" && m == http.MethodPost: return &http.Response{StatusCode: http.StatusCreated, Header: defaultHeader(), Body: objBody(codec, &svc.Items[0])}, nil default: @@ -124,11 +154,22 @@ func TestReplaceDirectory(t *testing.T) { f, tf, codec, _ := NewAPIFactory() ns := dynamic.ContentConfig().NegotiatedSerializer tf.Printer = &testPrinter{} + created := map[string]bool{} tf.Client = &fake.RESTClient{ NegotiatedSerializer: ns, Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { switch p, m := req.URL.Path, req.Method; { - case strings.HasPrefix(p, "/namespaces/test/replicationcontrollers/") && (m == http.MethodGet || m == http.MethodPut || m == http.MethodDelete): + case strings.HasPrefix(p, "/namespaces/test/replicationcontrollers/") && m == http.MethodPut: + created[p] = true + return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil + case strings.HasPrefix(p, "/namespaces/test/replicationcontrollers/") && m == http.MethodGet: + statusCode := http.StatusNotFound + if created[p] { + statusCode = http.StatusOK + } + return &http.Response{StatusCode: statusCode, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil + case strings.HasPrefix(p, "/namespaces/test/replicationcontrollers/") && m == http.MethodDelete: + delete(created, p) return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil case strings.HasPrefix(p, "/namespaces/test/replicationcontrollers") && m == http.MethodPost: return &http.Response{StatusCode: http.StatusCreated, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil @@ -172,7 +213,7 @@ func TestForceReplaceObjectNotFound(t *testing.T) { NegotiatedSerializer: ns, Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { switch p, m := req.URL.Path, req.Method; { - case p == "/namespaces/test/replicationcontrollers/redis-master" && m == http.MethodDelete: + case p == "/namespaces/test/replicationcontrollers/redis-master" && (m == http.MethodGet || m == http.MethodDelete): return &http.Response{StatusCode: http.StatusNotFound, Header: defaultHeader(), Body: stringBody("")}, nil case p == "/namespaces/test/replicationcontrollers" && m == http.MethodPost: return &http.Response{StatusCode: http.StatusCreated, Header: defaultHeader(), Body: objBody(codec, &rc.Items[0])}, nil