From 9ba13ff3616c18220de923c7094b55fb9e34ffc0 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Thu, 10 Sep 2015 20:54:22 -0700 Subject: [PATCH] Address comments --- contrib/completions/bash/kubectl | 6 ++-- docs/man/man1/kubectl-create.1 | 8 ++--- docs/man/man1/kubectl-replace.1 | 8 ++--- docs/man/man1/kubectl-rolling-update.1 | 8 ++--- docs/user-guide/kubectl/kubectl_create.md | 4 +-- docs/user-guide/kubectl/kubectl_replace.md | 4 +-- .../kubectl/kubectl_rolling-update.md | 4 +-- hack/verify-flags/known-flags.txt | 1 + pkg/kubectl/cmd/cmd_test.go | 4 +-- pkg/kubectl/cmd/create.go | 2 +- pkg/kubectl/cmd/replace.go | 4 +-- pkg/kubectl/cmd/rollingupdate.go | 2 +- pkg/kubectl/cmd/util/factory.go | 30 ++++++++++++++----- pkg/kubectl/cmd/util/helpers.go | 2 +- 14 files changed, 52 insertions(+), 35 deletions(-) diff --git a/contrib/completions/bash/kubectl b/contrib/completions/bash/kubectl index 7f3529fd3e2..462b3a5a986 100644 --- a/contrib/completions/bash/kubectl +++ b/contrib/completions/bash/kubectl @@ -351,7 +351,6 @@ _kubectl_create() flags_with_completion=() flags_completion=() - flags+=("--cache-schema") flags+=("--filename=") flags_with_completion+=("--filename") flags_completion+=("__handle_filename_extension_flag json|stdin|yaml|yml") @@ -360,6 +359,7 @@ _kubectl_create() flags_completion+=("__handle_filename_extension_flag json|stdin|yaml|yml") flags+=("--output=") two_word_flags+=("-o") + flags+=("--schema-cache-dir=") flags+=("--validate") must_have_one_flag=() @@ -378,7 +378,6 @@ _kubectl_replace() flags_with_completion=() flags_completion=() - flags+=("--cache-schema") flags+=("--cascade") flags+=("--filename=") flags_with_completion+=("--filename") @@ -390,6 +389,7 @@ _kubectl_replace() flags+=("--grace-period=") flags+=("--output=") two_word_flags+=("-o") + flags+=("--schema-cache-dir=") flags+=("--timeout=") flags+=("--validate") @@ -521,7 +521,6 @@ _kubectl_rolling-update() flags_with_completion=() flags_completion=() - flags+=("--cache-schema") flags+=("--deployment-label-key=") flags+=("--dry-run") flags+=("--filename=") @@ -537,6 +536,7 @@ _kubectl_rolling-update() flags+=("--output-version=") flags+=("--poll-interval=") flags+=("--rollback") + flags+=("--schema-cache-dir=") flags+=("--show-all") flags+=("-a") flags+=("--sort-by=") diff --git a/docs/man/man1/kubectl-create.1 b/docs/man/man1/kubectl-create.1 index 2392fdf8a27..2c28bd1fabd 100644 --- a/docs/man/man1/kubectl-create.1 +++ b/docs/man/man1/kubectl-create.1 @@ -20,10 +20,6 @@ JSON and YAML formats are accepted. .SH OPTIONS -.PP -\fB\-\-cache\-schema\fP=true - If true, use/store local schema files - .PP \fB\-f\fP, \fB\-\-filename\fP=[] Filename, directory, or URL to file to use to create the resource @@ -32,6 +28,10 @@ JSON and YAML formats are accepted. \fB\-o\fP, \fB\-\-output\fP="" Output mode. Use "\-o name" for shorter output (resource/name). +.PP +\fB\-\-schema\-cache\-dir\fP="/tmp/kubectl.schema" + If non\-empty, load/store cached API schemas in this directory, default is '/tmp/kubectl.schema' + .PP \fB\-\-validate\fP=true If true, use a schema to validate the input before sending it diff --git a/docs/man/man1/kubectl-replace.1 b/docs/man/man1/kubectl-replace.1 index 679e22616f2..c3124c575ee 100644 --- a/docs/man/man1/kubectl-replace.1 +++ b/docs/man/man1/kubectl-replace.1 @@ -26,10 +26,6 @@ Please refer to the models in .SH OPTIONS -.PP -\fB\-\-cache\-schema\fP=true - If true, use/store local schema files - .PP \fB\-\-cascade\fP=false Only relevant during a force replace. If true, cascade the deletion of the resources managed by this resource (e.g. Pods created by a ReplicationController). @@ -50,6 +46,10 @@ Please refer to the models in \fB\-o\fP, \fB\-\-output\fP="" Output mode. Use "\-o name" for shorter output (resource/name). +.PP +\fB\-\-schema\-cache\-dir\fP="/tmp/kubectl.schema" + If non\-empty, load/store cached API schemas in this directory, default is '/tmp/kubectl.schema' + .PP \fB\-\-timeout\fP=0 Only relevant during a force replace. The length of time to wait before giving up on a delete of the old resource, zero means determine a timeout from the size of the object diff --git a/docs/man/man1/kubectl-rolling-update.1 b/docs/man/man1/kubectl-rolling-update.1 index e6a402be924..f0da1054ab2 100644 --- a/docs/man/man1/kubectl-rolling-update.1 +++ b/docs/man/man1/kubectl-rolling-update.1 @@ -22,10 +22,6 @@ existing replication controller and overwrite at least one (common) label in its .SH OPTIONS -.PP -\fB\-\-cache\-schema\fP=true - If true, use/store local schema files - .PP \fB\-\-deployment\-label\-key\fP="deployment" The key to use to differentiate between two different controllers, default 'deployment'. Only relevant when \-\-image is specified, ignored otherwise @@ -64,6 +60,10 @@ existing replication controller and overwrite at least one (common) label in its \fB\-\-rollback\fP=false If true, this is a request to abort an existing rollout that is partially rolled out. It effectively reverses current and next and runs a rollout +.PP +\fB\-\-schema\-cache\-dir\fP="/tmp/kubectl.schema" + If non\-empty, load/store cached API schemas in this directory, default is '/tmp/kubectl.schema' + .PP \fB\-a\fP, \fB\-\-show\-all\fP=false When printing, show all resources (default hide terminated pods.) diff --git a/docs/user-guide/kubectl/kubectl_create.md b/docs/user-guide/kubectl/kubectl_create.md index 44e0eb99cd3..a5d3d9621db 100644 --- a/docs/user-guide/kubectl/kubectl_create.md +++ b/docs/user-guide/kubectl/kubectl_create.md @@ -59,9 +59,9 @@ $ cat pod.json | kubectl create -f - ### Options ``` - --cache-schema[=true]: If true, use/store local schema files -f, --filename=[]: Filename, directory, or URL to file to use to create the resource -o, --output="": Output mode. Use "-o name" for shorter output (resource/name). + --schema-cache-dir="/tmp/kubectl.schema": If non-empty, load/store cached API schemas in this directory, default is '/tmp/kubectl.schema' --validate[=true]: If true, use a schema to validate the input before sending it ``` @@ -97,7 +97,7 @@ $ cat pod.json | kubectl create -f - * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra at 2015-09-10 22:01:09.789168223 +0000 UTC +###### Auto generated by spf13/cobra at 2015-09-11 20:48:33.289761103 +0000 UTC [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_create.md?pixel)]() diff --git a/docs/user-guide/kubectl/kubectl_replace.md b/docs/user-guide/kubectl/kubectl_replace.md index 649c3aae318..60485a4c0aa 100644 --- a/docs/user-guide/kubectl/kubectl_replace.md +++ b/docs/user-guide/kubectl/kubectl_replace.md @@ -69,12 +69,12 @@ kubectl replace --force -f ./pod.json ### Options ``` - --cache-schema[=true]: If true, use/store local schema files --cascade[=false]: Only relevant during a force replace. If true, cascade the deletion of the resources managed by this resource (e.g. Pods created by a ReplicationController). -f, --filename=[]: Filename, directory, or URL to file to use to replace the resource. --force[=false]: Delete and re-create the specified resource --grace-period=-1: Only relevant during a force replace. Period of time in seconds given to the old resource to terminate gracefully. Ignored if negative. -o, --output="": Output mode. Use "-o name" for shorter output (resource/name). + --schema-cache-dir="/tmp/kubectl.schema": If non-empty, load/store cached API schemas in this directory, default is '/tmp/kubectl.schema' --timeout=0: Only relevant during a force replace. The length of time to wait before giving up on a delete of the old resource, zero means determine a timeout from the size of the object --validate[=true]: If true, use a schema to validate the input before sending it ``` @@ -111,7 +111,7 @@ kubectl replace --force -f ./pod.json * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra at 2015-09-10 22:01:09.789498374 +0000 UTC +###### Auto generated by spf13/cobra at 2015-09-11 20:48:33.290279625 +0000 UTC [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_replace.md?pixel)]() diff --git a/docs/user-guide/kubectl/kubectl_rolling-update.md b/docs/user-guide/kubectl/kubectl_rolling-update.md index 89252361930..07cbaebc177 100644 --- a/docs/user-guide/kubectl/kubectl_rolling-update.md +++ b/docs/user-guide/kubectl/kubectl_rolling-update.md @@ -69,7 +69,6 @@ $ kubectl rolling-update frontend --image=image:v2 ### Options ``` - --cache-schema[=true]: If true, use/store local schema files --deployment-label-key="deployment": The key to use to differentiate between two different controllers, default 'deployment'. Only relevant when --image is specified, ignored otherwise --dry-run[=false]: If true, print out the changes that would be made, but don't actually make them. -f, --filename=[]: Filename or URL to file to use to create the new replication controller. @@ -79,6 +78,7 @@ $ kubectl rolling-update frontend --image=image:v2 --output-version="": Output the formatted object with the given version (default api-version). --poll-interval=3s: Time delay between polling for replication controller status after the update. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". --rollback[=false]: If true, this is a request to abort an existing rollout that is partially rolled out. It effectively reverses current and next and runs a rollout + --schema-cache-dir="/tmp/kubectl.schema": If non-empty, load/store cached API schemas in this directory, default is '/tmp/kubectl.schema' -a, --show-all[=false]: When printing, show all resources (default hide terminated pods.) --sort-by="": If non-empty, sort list types using this field specification. The field specification is expressed as a JSONPath expression (e.g. 'ObjectMeta.Name'). The field in the API resource specified by this JSONPath expression must be an integer or a string. --template="": Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview]. @@ -119,7 +119,7 @@ $ kubectl rolling-update frontend --image=image:v2 * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra at 2015-09-10 22:01:09.791014946 +0000 UTC +###### Auto generated by spf13/cobra at 2015-09-11 20:48:33.293748592 +0000 UTC [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_rolling-update.md?pixel)]() diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index c082aa7a767..576f9b07e82 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -229,6 +229,7 @@ root-dir run-proxy runtime-config scheduler-config +schema-cache-dir secure-port service-account-key-file service-account-lookup diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index da2f137e31e..bd7c08b4550 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -160,7 +160,7 @@ func NewTestFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) { Printer: func(mapping *meta.RESTMapping, noHeaders, withNamespace bool, wide bool, showAll bool, columnLabels []string) (kubectl.ResourcePrinter, error) { return t.Printer, t.Err }, - Validator: func(validate, cacheSchema bool) (validation.Schema, error) { + Validator: func(validate bool, cacheDir string) (validation.Schema, error) { return t.Validator, t.Err }, DefaultNamespace: func() (string, bool, error) { @@ -215,7 +215,7 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) { Printer: func(mapping *meta.RESTMapping, noHeaders, withNamespace bool, wide bool, showAll bool, columnLabels []string) (kubectl.ResourcePrinter, error) { return t.Printer, t.Err }, - Validator: func(validate, cacheSchema bool) (validation.Schema, error) { + Validator: func(validate bool, cacheDir string) (validation.Schema, error) { return t.Validator, t.Err }, DefaultNamespace: func() (string, bool, error) { diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index e392d5d4e90..4753340886e 100644 --- a/pkg/kubectl/cmd/create.go +++ b/pkg/kubectl/cmd/create.go @@ -78,7 +78,7 @@ func ValidateArgs(cmd *cobra.Command, args []string) error { } func RunCreate(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *CreateOptions) error { - schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "cache-schema")) + schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir")) if err != nil { return err } diff --git a/pkg/kubectl/cmd/replace.go b/pkg/kubectl/cmd/replace.go index 48694c8460b..34a0fcc2e79 100644 --- a/pkg/kubectl/cmd/replace.go +++ b/pkg/kubectl/cmd/replace.go @@ -90,7 +90,7 @@ func RunReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []st if len(os.Args) > 1 && os.Args[1] == "update" { printDeprecationWarning("replace", "update") } - schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "cache-schema")) + schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir")) if err != nil { return err } @@ -143,7 +143,7 @@ func RunReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []st } func forceReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, shortOutput bool, options *ReplaceOptions) error { - schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "cache-schema")) + schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir")) if err != nil { return err } diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index fceb4e904fd..1f86bc21b45 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -172,7 +172,7 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg mapper, typer := f.Object() if len(filename) != 0 { - schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagBool(cmd, "cache-schema")) + schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir")) if err != nil { return err } diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 6675a4ce4c6..2d90df2ade5 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "bytes" "flag" "fmt" "io" @@ -78,7 +79,7 @@ type Factory struct { // LabelsForObject returns the labels associated with the provided object LabelsForObject func(object runtime.Object) (map[string]string, error) // Returns a schema that can validate objects stored on disk. - Validator func(validate, cacheSchema bool) (validation.Schema, error) + Validator func(validate bool, cacheDir string) (validation.Schema, error) // Returns the default namespace to use in cases where no // other namespace is specified and whether the namespace was // overriden. @@ -216,17 +217,25 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory { } return kubectl.ReaperFor(mapping.Kind, client) }, - Validator: func(validate, cacheSchema bool) (validation.Schema, error) { + Validator: func(validate bool, cacheDir string) (validation.Schema, error) { if validate { client, err := clients.ClientForVersion("") if err != nil { return nil, err } - var cacheDir string - if cacheSchema { - cacheDir = "/tmp" + dir := cacheDir + if len(dir) > 0 { + version, err := client.ServerVersion() + if err != nil { + return nil, err + } + dir = path.Join(cacheDir, version.String()) } - return &clientSwaggerSchema{client, client.ExperimentalClient, cacheDir}, nil + return &clientSwaggerSchema{ + c: client, + ec: client.ExperimentalClient, + cacheDir: dir, + }, nil } return validation.NullSchema{}, nil }, @@ -311,7 +320,14 @@ func getSchemaAndValidate(c schemaClient, data []byte, group, version, cacheDir if err = os.MkdirAll(path.Join(cacheDir, group, version), 0755); err != nil { return err } - if err = ioutil.WriteFile(cacheFile, schemaData, 0644); err != nil { + tmpFile, err := ioutil.TempFile(cacheDir, "schema") + if err != nil { + return err + } + if _, err := io.Copy(tmpFile, bytes.NewBuffer(schemaData)); err != nil { + return err + } + if err := os.Link(tmpFile.Name(), cacheFile); err != nil && !os.IsExist(err) { return err } } diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index 6a94c0df833..3a2b07a42ee 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -270,7 +270,7 @@ func GetFlagDuration(cmd *cobra.Command, flag string) time.Duration { func AddValidateFlags(cmd *cobra.Command) { cmd.Flags().Bool("validate", true, "If true, use a schema to validate the input before sending it") - cmd.Flags().Bool("cache-schema", true, "If true, use/store local schema files") + cmd.Flags().String("schema-cache-dir", "/tmp/kubectl.schema", "If non-empty, load/store cached API schemas in this directory, default is '/tmp/kubectl.schema'") } func ReadConfigDataFromReader(reader io.Reader, source string) ([]byte, error) {