Fix terminal attach #32
Conversation
|
LGTM pending tests. @rhatdan PTAL |
|
@alexlarsson PTAL |
cmd/kpod/create.go
Outdated
There was a problem hiding this comment.
What happens if you set detach and tty?
cmd/kpod/run.go
Outdated
There was a problem hiding this comment.
Shouldn't this happen if you specify -d?
baude
force-pushed
the
attach
branch
2 times, most recently
from
6447483 to
f8312ed
Compare
cmd/kpod/run.go
Outdated
There was a problem hiding this comment.
Should we be waiting down here or up higher before the start?
|
How can we attach to the container before it is started? There is no conmon to attach to by then. I must be missing something here? |
|
@alexlarsson we dont use conmon to attach. We attach to the socket which is already created. |
|
bot, retest this please |
|
📌 Commit f8312ed has been approved by |
|
⌛ Testing commit f8312ed with merge 8cad4d9... |
Re-order the startup of a new container via run from initialize > start > attach to initialize > attach > start. This fixes output when running: kpod run -i -t IMAGE command and kpod run IMAGE command Signed-off-by: baude <bbaude@redhat.com> Closes: #32 Approved by: rhatdan
|
bot, retest this please |
baude
force-pushed
the
attach
branch
2 times, most recently
from
ea2e58b to
feb5aa4
Compare
|
LGTM. Let's give this another show |
|
📌 Commit feb5aa4 has been approved by |
|
⌛ Testing commit feb5aa4 with merge f50f308... |
|
@rh-atomic-bot retry |
|
⌛ Testing commit feb5aa4 with merge 4afb998... |
|
💥 Test timed out |
|
@rh-atomic-bot retry |
|
⌛ Testing commit feb5aa4 with merge 35ab89e... |
Re-order the startup of a new container via run from initialize > start > attach to initialize > attach > start. This fixes output when running: kpod run -i -t IMAGE command and kpod run IMAGE command Signed-off-by: baude <bbaude@redhat.com> Closes: #32 Approved by: mheon
|
@baude The tests are still not completing? |
|
💥 Test timed out |
|
This patch passed when I combined it with mine. I don't know why it is timing out here. |
|
@rh-atomic-bot retry |
|
⌛ Testing commit feb5aa4 with merge fbdb70c... |
|
💥 Test timed out |
.papr.yml
Outdated
There was a problem hiding this comment.
This is throwing off the parser (need to strengthen that...). It's expecting a number followed by one of s, m, or h. See https://github.com/projectatomic/papr/blob/master/docs/sample.papr.yml#L172.
baude
force-pushed
the
attach
branch
3 times, most recently
from
a26ae16 to
956a4ff
Compare
Re-order the startup of a new container via run from initialize > start > attach to initialize > attach > start. This fixes output when running: kpod run -i -t IMAGE command and kpod run IMAGE command Signed-off-by: baude <bbaude@redhat.com>
|
@rh-atomic-bot bot, retest this please |
|
bot, retest this please |
github-actions
bot
added
the
locked - please file new issue/PR
7 participants
Re-order the startup of a new container via run from
initialize > start > attach to initialize > attach > start.
This fixes output when running:
kpod run -i -t IMAGE command
and
kpod run IMAGE command
Signed-off-by: baude bbaude@redhat.com