UPSTREAM: attach must only allow a tty when container supports it

[fabianofranz]

Fixes bug 1270685 https://bugzilla.redhat.com/show_bug.cgi?id=1270685

If a container does not support TTY, warn the user if -t was provided to attach.

[smarterclayton]

We're doing what docker does. You should ask @ncdc or possibly @mrunalp

[fabianofranz]

For the record, edit doesn't make it raw (but I guess the underlying editor, like vi, does).

[ncdc]

@fabianofranz I don't think this is the right fix.

if the container in the pod isn't configured with TTY=true (https://github.com/kubernetes/kubernetes/blob/master/pkg/api/v1/types.go#L993), then we shouldn't allow attach -t. We can get the pod, check the container's configuration, and either reject the attach request, or log a warning that tty isn't supported in the container, and "remove" the -t flag.

[fabianofranz]

PR refactored, @ncdc please take a look.

[ncdc]

@fabianofranz would you mind updating the title and description of this PR?

Does attach work if container.Stdin is false? Should we check for that too?

[fabianofranz]

@ncdc updated. stdin works fine, tested with https://raw.githubusercontent.com/openshift-qe/v3-testfiles/master/pods/pod_with_two_containers.json.

[fabianofranz]

[test]

[smarterclayton]

Attach allows stdin=false.

On Thu, Oct 29, 2015 at 2:33 PM, Andy Goldstein [email protected]
wrote:

@fabianofranz https://github.com/fabianofranz would you mind updating
the title and description of this PR?

Does attach work if container.Stdin is false? Should we check for that too?

—
Reply to this email directly or view it on GitHub
#5497 (comment).

[fabianofranz]

Upstream PR: kubernetes/kubernetes#16537
Updated this one to refer the upstream PR #.

[openshift-bot]

Evaluated for origin test up to 398ad3a

[fabianofranz]

@openshift/team-project-committers upstream PR is merging.

[openshift-bot]

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6597/)

[smarterclayton]

LGTM [merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/3913/) (Image: devenv-rhel7_2632)

[openshift-bot]

Evaluated for origin merge up to 398ad3a