mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-31 15:25:57 +00:00
Remove unused flags from kubectl run
The following flags, which do not apply to kubectl run, have been removed: --cascade --filename --force --grace-period --kustomize --recursive --timeout --wait These flags were being added to the run command to support pod deletion after attach, but they are not used if set, so they effectively do nothing. This PR also displays an error message if the pod fails to be deleted (when the --rm flag is used). Previously any error during deletion would be suppressed and the pod would remain. This PR also adds some unit tests for run and attach with and without the --rm flag. As such, some minor refactoring of the run command has been done to support mocking dependencies.
This commit is contained in:
Brian Pursley
parent
4407a02aef
commit
25e713ba77
@ -36,7 +36,6 @@ import (
|
||||
"k8s.io/apimachinery/pkg/watch"
|
||||
"k8s.io/cli-runtime/pkg/genericclioptions"
|
||||
"k8s.io/cli-runtime/pkg/resource"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
|
||||
"k8s.io/client-go/tools/cache"
|
||||
watchtools "k8s.io/client-go/tools/watch"
|
||||
@ -94,6 +93,8 @@ const (
|
||||
|
||||
var metadataAccessor = meta.NewAccessor()
|
||||
|
||||
var attachFunc = attach.DefaultAttachFunc
|
||||
|
||||
type RunObject struct {
|
||||
Object runtime.Object
|
||||
Mapping *meta.RESTMapping
|
||||
@ -105,7 +106,6 @@ type RunOptions struct {
|
||||
PrintFlags *genericclioptions.PrintFlags
|
||||
RecordFlags *genericclioptions.RecordFlags
|
||||
|
||||
DeleteFlags *cmddelete.DeleteFlags
|
||||
DeleteOptions *cmddelete.DeleteOptions
|
||||
|
||||
DryRunStrategy cmdutil.DryRunStrategy
|
||||
@ -135,7 +135,6 @@ type RunOptions struct {
|
||||
func NewRunOptions(streams genericclioptions.IOStreams) *RunOptions {
|
||||
return &RunOptions{
|
||||
PrintFlags: genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme),
|
||||
DeleteFlags: cmddelete.NewDeleteFlags("to use to replace the resource."),
|
||||
RecordFlags: genericclioptions.NewRecordFlags(),
|
||||
|
||||
Recorder: genericclioptions.NoopRecorder{},
|
||||
@ -159,7 +158,6 @@ func NewCmdRun(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Co
|
||||
},
|
||||
}
|
||||
|
||||
o.DeleteFlags.AddFlags(cmd)
|
||||
o.PrintFlags.AddFlags(cmd)
|
||||
o.RecordFlags.AddFlags(cmd)
|
||||
|
||||
@ -226,18 +224,17 @@ func (o *RunOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
|
||||
return printer.PrintObj(obj, o.Out)
|
||||
}
|
||||
|
||||
deleteOpts, err := o.DeleteFlags.ToOptions(dynamicClient, o.IOStreams)
|
||||
if err != nil {
|
||||
return err
|
||||
o.DeleteOptions = &cmddelete.DeleteOptions{
|
||||
CascadingStrategy: metav1.DeletePropagationBackground,
|
||||
DynamicClient: dynamicClient,
|
||||
GracePeriod: -1,
|
||||
IgnoreNotFound: true,
|
||||
IOStreams: o.IOStreams,
|
||||
Quiet: o.Quiet,
|
||||
Timeout: time.Duration(0),
|
||||
WaitForDeletion: false,
|
||||
}
|
||||
|
||||
deleteOpts.IgnoreNotFound = true
|
||||
deleteOpts.WaitForDeletion = false
|
||||
deleteOpts.GracePeriod = -1
|
||||
deleteOpts.Quiet = o.Quiet
|
||||
|
||||
o.DeleteOptions = deleteOpts
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -325,7 +322,11 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
|
||||
|
||||
if o.Attach {
|
||||
if remove {
|
||||
defer o.removeCreatedObjects(f, createdObjects)
|
||||
defer func() {
|
||||
if err := o.removeCreatedObjects(f, createdObjects); err != nil {
|
||||
fmt.Fprintf(o.ErrOut, "Delete failed: %v\n", err)
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
opts := &attach.AttachOptions{
|
||||
@ -345,9 +346,9 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
|
||||
return err
|
||||
}
|
||||
opts.Config = config
|
||||
opts.AttachFunc = attach.DefaultAttachFunc
|
||||
opts.AttachFunc = attachFunc
|
||||
|
||||
clientset, err := kubernetes.NewForConfig(config)
|
||||
clientset, err := f.KubernetesClientSet()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@ -33,10 +33,13 @@ import (
|
||||
apiequality "k8s.io/apimachinery/pkg/api/equality"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
"k8s.io/apimachinery/pkg/util/intstr"
|
||||
"k8s.io/cli-runtime/pkg/genericclioptions"
|
||||
restclient "k8s.io/client-go/rest"
|
||||
"k8s.io/client-go/rest/fake"
|
||||
"k8s.io/client-go/tools/remotecommand"
|
||||
"k8s.io/kubectl/pkg/cmd/attach"
|
||||
"k8s.io/kubectl/pkg/cmd/delete"
|
||||
cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
|
||||
cmdutil "k8s.io/kubectl/pkg/cmd/util"
|
||||
@ -640,6 +643,193 @@ func TestExpose(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunAttach(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
rm bool
|
||||
quiet bool
|
||||
deleteErrorMessage string
|
||||
expectedDeleteCount int
|
||||
expectedOut string
|
||||
expectedErrOut string
|
||||
}{
|
||||
{
|
||||
name: "test attach",
|
||||
rm: false,
|
||||
quiet: false,
|
||||
expectedDeleteCount: 0,
|
||||
expectedOut: "",
|
||||
expectedErrOut: "If you don't see a command prompt, try pressing enter.\n",
|
||||
},
|
||||
{
|
||||
name: "test attach with quiet",
|
||||
rm: false,
|
||||
quiet: true,
|
||||
expectedDeleteCount: 0,
|
||||
expectedOut: "",
|
||||
expectedErrOut: "",
|
||||
},
|
||||
{
|
||||
name: "test attach with rm",
|
||||
rm: true,
|
||||
quiet: false,
|
||||
expectedDeleteCount: 1,
|
||||
expectedOut: "pod \"foo\" deleted\n",
|
||||
expectedErrOut: "If you don't see a command prompt, try pressing enter.\n",
|
||||
},
|
||||
{
|
||||
name: "test attach with rm should not print message if quiet is specified",
|
||||
rm: true,
|
||||
quiet: true,
|
||||
expectedDeleteCount: 1,
|
||||
expectedOut: "",
|
||||
expectedErrOut: "",
|
||||
},
|
||||
{
|
||||
name: "error should be displayed if delete fails",
|
||||
rm: true,
|
||||
quiet: false,
|
||||
deleteErrorMessage: "delete error message",
|
||||
expectedDeleteCount: 1,
|
||||
expectedOut: "",
|
||||
expectedErrOut: "If you don't see a command prompt, try pressing enter.\nDelete failed: delete error message\n",
|
||||
},
|
||||
}
|
||||
|
||||
fakePod := &corev1.Pod{
|
||||
TypeMeta: metav1.TypeMeta{
|
||||
APIVersion: "v1",
|
||||
Kind: "Pod",
|
||||
},
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: "foo",
|
||||
Namespace: "default",
|
||||
},
|
||||
Spec: corev1.PodSpec{
|
||||
Containers: []corev1.Container{
|
||||
{
|
||||
Name: "bar",
|
||||
},
|
||||
},
|
||||
},
|
||||
Status: corev1.PodStatus{
|
||||
Phase: corev1.PodRunning,
|
||||
Conditions: []corev1.PodCondition{
|
||||
{
|
||||
Type: corev1.PodReady,
|
||||
Status: corev1.ConditionTrue,
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(tt *testing.T) {
|
||||
postCount := 0
|
||||
attachCount := 0
|
||||
deleteCount := 0
|
||||
|
||||
attachFunc = func(o *attach.AttachOptions, containerToAttach *corev1.Container, raw bool, sizeQueue remotecommand.TerminalSizeQueue) func() error {
|
||||
if containerToAttach.Name != "bar" {
|
||||
tt.Fatalf("expected attach to container name \"bar\", but got %q", containerToAttach.Name)
|
||||
}
|
||||
return func() error {
|
||||
attachCount++
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
tf := cmdtesting.NewTestFactory().WithNamespace("test")
|
||||
defer tf.Cleanup()
|
||||
|
||||
codec := scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...)
|
||||
ns := scheme.Codecs.WithoutConversion()
|
||||
tf.Client = &fake.RESTClient{
|
||||
GroupVersion: schema.GroupVersion{Version: ""},
|
||||
NegotiatedSerializer: ns,
|
||||
Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
|
||||
switch p, m := req.URL.Path, req.Method; {
|
||||
case m == "POST" && p == "/namespaces/test/pods":
|
||||
postCount++
|
||||
body := cmdtesting.ObjBody(codec, fakePod)
|
||||
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
|
||||
case m == "GET" && p == "/api/v1/namespaces/default/pods":
|
||||
event := &metav1.WatchEvent{
|
||||
Type: "ADDED",
|
||||
Object: runtime.RawExtension{Object: fakePod},
|
||||
}
|
||||
body := cmdtesting.ObjBody(codec, event)
|
||||
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
|
||||
case m == "GET" && p == "/namespaces/default/pods/foo":
|
||||
body := cmdtesting.ObjBody(codec, fakePod)
|
||||
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
|
||||
case m == "DELETE" && p == "/namespaces/default/pods/foo":
|
||||
deleteCount++
|
||||
if test.deleteErrorMessage != "" {
|
||||
body := cmdtesting.ObjBody(codec, &metav1.Status{
|
||||
Status: metav1.StatusFailure,
|
||||
Message: test.deleteErrorMessage,
|
||||
})
|
||||
return &http.Response{StatusCode: http.StatusInternalServerError, Header: cmdtesting.DefaultHeader(), Body: body}, nil
|
||||
} else {
|
||||
body := cmdtesting.ObjBody(codec, fakePod)
|
||||
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
|
||||
}
|
||||
default:
|
||||
tt.Errorf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req)
|
||||
return nil, fmt.Errorf("unexpected request")
|
||||
}
|
||||
}),
|
||||
}
|
||||
|
||||
tf.ClientConfigVal = &restclient.Config{
|
||||
ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}, NegotiatedSerializer: ns},
|
||||
}
|
||||
|
||||
streams, _, bufOut, bufErr := genericclioptions.NewTestIOStreams()
|
||||
cmdutil.BehaviorOnFatal(func(str string, code int) {
|
||||
bufErr.Write([]byte(str))
|
||||
})
|
||||
|
||||
cmd := NewCmdRun(tf, streams)
|
||||
cmd.Flags().Set("image", "test-image")
|
||||
cmd.Flags().Set("attach", "true")
|
||||
if test.rm {
|
||||
cmd.Flags().Set("rm", "true")
|
||||
}
|
||||
if test.quiet {
|
||||
cmd.Flags().Set("quiet", "true")
|
||||
}
|
||||
|
||||
parentCmd := cobra.Command{}
|
||||
parentCmd.AddCommand(cmd)
|
||||
|
||||
cmd.Run(cmd, []string{"test-pod"})
|
||||
|
||||
if postCount != 1 {
|
||||
tt.Fatalf("expected 1 post request, but got %d", postCount)
|
||||
}
|
||||
|
||||
if attachCount != 1 {
|
||||
tt.Fatalf("expected 1 attach call, but got %d", attachCount)
|
||||
}
|
||||
|
||||
if deleteCount != test.expectedDeleteCount {
|
||||
tt.Fatalf("expected %d delete requests, but got %d", test.expectedDeleteCount, deleteCount)
|
||||
}
|
||||
|
||||
if bufErr.String() != test.expectedErrOut {
|
||||
tt.Fatalf("unexpected error. got: %q, expected: %q", bufErr.String(), test.expectedErrOut)
|
||||
}
|
||||
|
||||
if bufOut.String() != test.expectedOut {
|
||||
tt.Fatalf("unexpected output. got: %q, expected: %q", bufOut.String(), test.expectedOut)
|
||||
}
|
||||
})
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunOverride(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
Loading…
Reference in New Issue
Block a user