mirror of https://github.com/k3s-io/k3s
Error out on non-existing container in kubectl attach
parent
f3359fe134
commit
0acca44dc1
|
@ -184,7 +184,10 @@ func (p *AttachOptions) Run() error {
|
|||
|
||||
// check for TTY
|
||||
tty := p.TTY
|
||||
containerToAttach := p.GetContainer(pod)
|
||||
containerToAttach, err := p.containerToAttachTo(pod)
|
||||
if err != nil {
|
||||
return fmt.Errorf("cannot attach to the container: %v", err)
|
||||
}
|
||||
if tty && !containerToAttach.TTY {
|
||||
tty = false
|
||||
fmt.Fprintf(p.Err, "Unable to use a TTY - container %s did not allocate one\n", containerToAttach.Name)
|
||||
|
@ -229,26 +232,32 @@ func (p *AttachOptions) Run() error {
|
|||
return nil
|
||||
}
|
||||
|
||||
// GetContainer returns the container to attach to, with a fallback.
|
||||
func (p *AttachOptions) GetContainer(pod *api.Pod) api.Container {
|
||||
// containerToAttach returns a reference to the container to attach to, given
|
||||
// by name or the first container if name is empty.
|
||||
func (p *AttachOptions) containerToAttachTo(pod *api.Pod) (*api.Container, error) {
|
||||
if len(p.ContainerName) > 0 {
|
||||
for _, container := range pod.Spec.Containers {
|
||||
if container.Name == p.ContainerName {
|
||||
return container
|
||||
for i := range pod.Spec.Containers {
|
||||
if pod.Spec.Containers[i].Name == p.ContainerName {
|
||||
return &pod.Spec.Containers[i], nil
|
||||
}
|
||||
}
|
||||
for _, container := range pod.Spec.InitContainers {
|
||||
if container.Name == p.ContainerName {
|
||||
return container
|
||||
for i := range pod.Spec.InitContainers {
|
||||
if pod.Spec.InitContainers[i].Name == p.ContainerName {
|
||||
return &pod.Spec.InitContainers[i], nil
|
||||
}
|
||||
}
|
||||
return nil, fmt.Errorf("container not found (%s)", p.ContainerName)
|
||||
}
|
||||
|
||||
glog.V(4).Infof("defaulting container name to %s", pod.Spec.Containers[0].Name)
|
||||
return pod.Spec.Containers[0]
|
||||
return &pod.Spec.Containers[0], nil
|
||||
}
|
||||
|
||||
// GetContainerName returns the name of the container to attach to, with a fallback.
|
||||
func (p *AttachOptions) GetContainerName(pod *api.Pod) string {
|
||||
return p.GetContainer(pod).Name
|
||||
func (p *AttachOptions) GetContainerName(pod *api.Pod) (string, error) {
|
||||
c, err := p.containerToAttachTo(pod)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return c.Name, nil
|
||||
}
|
||||
|
|
|
@ -35,15 +35,15 @@ import (
|
|||
)
|
||||
|
||||
type fakeRemoteAttach struct {
|
||||
method string
|
||||
url *url.URL
|
||||
attachErr error
|
||||
method string
|
||||
url *url.URL
|
||||
err error
|
||||
}
|
||||
|
||||
func (f *fakeRemoteAttach) Attach(method string, url *url.URL, config *restclient.Config, stdin io.Reader, stdout, stderr io.Writer, tty bool) error {
|
||||
f.method = method
|
||||
f.url = url
|
||||
return f.attachErr
|
||||
return f.err
|
||||
}
|
||||
|
||||
func TestPodAndContainerAttach(t *testing.T) {
|
||||
|
@ -86,6 +86,12 @@ func TestPodAndContainerAttach(t *testing.T) {
|
|||
expectedContainer: "initfoo",
|
||||
name: "init container in flag",
|
||||
},
|
||||
{
|
||||
p: &AttachOptions{ContainerName: "bar"},
|
||||
args: []string{"foo", "-c", "wrong"},
|
||||
expectError: true,
|
||||
name: "non-existing container in flag",
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
|
@ -123,7 +129,8 @@ func TestAttach(t *testing.T) {
|
|||
tests := []struct {
|
||||
name, version, podPath, attachPath, container string
|
||||
pod *api.Pod
|
||||
attachErr bool
|
||||
remoteAttachErr bool
|
||||
exepctedErr string
|
||||
}{
|
||||
{
|
||||
name: "pod attach",
|
||||
|
@ -131,14 +138,26 @@ func TestAttach(t *testing.T) {
|
|||
podPath: "/api/" + version + "/namespaces/test/pods/foo",
|
||||
attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach",
|
||||
pod: attachPod(),
|
||||
container: "bar",
|
||||
},
|
||||
{
|
||||
name: "pod attach error",
|
||||
version: version,
|
||||
podPath: "/api/" + version + "/namespaces/test/pods/foo",
|
||||
attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach",
|
||||
pod: attachPod(),
|
||||
attachErr: true,
|
||||
name: "pod attach error",
|
||||
version: version,
|
||||
podPath: "/api/" + version + "/namespaces/test/pods/foo",
|
||||
attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach",
|
||||
pod: attachPod(),
|
||||
remoteAttachErr: true,
|
||||
container: "bar",
|
||||
exepctedErr: "attach error",
|
||||
},
|
||||
{
|
||||
name: "container not found error",
|
||||
version: version,
|
||||
podPath: "/api/" + version + "/namespaces/test/pods/foo",
|
||||
attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach",
|
||||
pod: attachPod(),
|
||||
container: "foo",
|
||||
exepctedErr: "cannot attach to the container: container not found (foo)",
|
||||
},
|
||||
}
|
||||
for _, test := range tests {
|
||||
|
@ -162,42 +181,42 @@ func TestAttach(t *testing.T) {
|
|||
bufOut := bytes.NewBuffer([]byte{})
|
||||
bufErr := bytes.NewBuffer([]byte{})
|
||||
bufIn := bytes.NewBuffer([]byte{})
|
||||
ex := &fakeRemoteAttach{}
|
||||
if test.attachErr {
|
||||
ex.attachErr = fmt.Errorf("attach error")
|
||||
remoteAttach := &fakeRemoteAttach{}
|
||||
if test.remoteAttachErr {
|
||||
remoteAttach.err = fmt.Errorf("attach error")
|
||||
}
|
||||
params := &AttachOptions{
|
||||
ContainerName: "bar",
|
||||
ContainerName: test.container,
|
||||
In: bufIn,
|
||||
Out: bufOut,
|
||||
Err: bufErr,
|
||||
Attach: ex,
|
||||
Attach: remoteAttach,
|
||||
}
|
||||
cmd := &cobra.Command{}
|
||||
if err := params.Complete(f, cmd, []string{"foo"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
err := params.Run()
|
||||
if test.attachErr && err != ex.attachErr {
|
||||
if test.exepctedErr != "" && err.Error() != test.exepctedErr {
|
||||
t.Errorf("%s: Unexpected exec error: %v", test.name, err)
|
||||
continue
|
||||
}
|
||||
if !test.attachErr && err != nil {
|
||||
if test.exepctedErr == "" && err != nil {
|
||||
t.Errorf("%s: Unexpected error: %v", test.name, err)
|
||||
continue
|
||||
}
|
||||
if test.attachErr {
|
||||
if test.exepctedErr != "" {
|
||||
continue
|
||||
}
|
||||
if ex.url.Path != test.attachPath {
|
||||
if remoteAttach.url.Path != test.attachPath {
|
||||
t.Errorf("%s: Did not get expected path for exec request", test.name)
|
||||
continue
|
||||
}
|
||||
if ex.method != "POST" {
|
||||
t.Errorf("%s: Did not get method for attach request: %s", test.name, ex.method)
|
||||
if remoteAttach.method != "POST" {
|
||||
t.Errorf("%s: Did not get method for attach request: %s", test.name, remoteAttach.method)
|
||||
}
|
||||
if ex.url.Query().Get("container") != "bar" {
|
||||
t.Errorf("%s: Did not have query parameters: %s", test.name, ex.url.Query())
|
||||
if remoteAttach.url.Query().Get("container") != "bar" {
|
||||
t.Errorf("%s: Did not have query parameters: %s", test.name, remoteAttach.url.Query())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -332,8 +332,12 @@ func handleAttachPod(f *cmdutil.Factory, c *client.Client, pod *api.Pod, opts *A
|
|||
if err != nil {
|
||||
return err
|
||||
}
|
||||
ctrName, err := opts.GetContainerName(pod)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if status == api.PodSucceeded || status == api.PodFailed {
|
||||
req, err := f.LogsForObject(pod, &api.PodLogOptions{Container: opts.GetContainerName(pod)})
|
||||
req, err := f.LogsForObject(pod, &api.PodLogOptions{Container: ctrName})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
@ -350,7 +354,7 @@ func handleAttachPod(f *cmdutil.Factory, c *client.Client, pod *api.Pod, opts *A
|
|||
opts.Namespace = pod.Namespace
|
||||
if err := opts.Run(); err != nil {
|
||||
fmt.Fprintf(opts.Out, "Error attaching, falling back to logs: %v\n", err)
|
||||
req, err := f.LogsForObject(pod, &api.PodLogOptions{Container: opts.GetContainerName(pod)})
|
||||
req, err := f.LogsForObject(pod, &api.PodLogOptions{Container: ctrName})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue