Execute into a pod with API for runtime V1 #596
Conversation
shawn-hurley
requested review from
jmrodri,
eriknelson,
djzager and
rthallisey
openshift-ci-robot
added
the
size/XXL
shawn-hurley
added
bug
needs-review
and removed
size/XXL
|
Changes Unknown when pulling 99b1f92 on shawn-hurley:remote-execution into ** on openshift:master**. |
pkg/apb/ext_creds.go
Outdated
| log.Warning("[%s] Retry attempt %d: Failed to exec into the container", podname, r) | ||
| default: | ||
| log.Infof("command output: %v - err: %v", stdoutBuffer.String(), stderrBuffer.String()) | ||
| log.Warningf("retry attempt: %v pod: %v in namespace: %v failed to exec into the container", r, podname, namespace) |
There was a problem hiding this comment.
I noticed this got bumped up to a Warning level a while back in the old code with previous iterations of the credential extractions. I don't think this should be of a warning level since the "tried to get credentials but they weren't ready yet" branch of execution is expected to get hit a few times during a normal run of an APB.
If we agree this isn't a warning, probably drop v2 back down to Info as well, either in this PR or a small follow up.
pkg/apb/ext_creds.go
Outdated
|
|
||
| const ( | ||
| // GatherCredentialsCommand - Command used when execing for bind credentials | ||
| // moving this constant here because eventuall Extrating creds will |
There was a problem hiding this comment.
Thank you for putting the reason for the move here.
openshift-ci-robot
added
the
size/XXL
|
@djzager @eriknelson I could not find the |
|
Changes Unknown when pulling d217b66 on shawn-hurley:remote-execution into ** on openshift:master**. |
|
Changes Unknown when pulling d217b66 on shawn-hurley:remote-execution into ** on openshift:master**. |
|
@shawn-hurley it's possible it got removed, I just remember it being I think in the old |
pkg/apb/ext_creds.go
Outdated
| podFailed := strings.Contains(string(output), "current phase is Failed") | ||
| podCompleted := strings.Contains(string(output), "current phase is Succeeded") || | ||
| strings.Contains(string(output), "cannot exec into a container in a completed pod") | ||
| // NOTE: kubectl exec simply sets the API path to /api when where is no |
|
Changes Unknown when pulling cebac6c on shawn-hurley:remote-execution into ** on openshift:master**. |
shawn-hurley
force-pushed
the
remote-execution
branch
from
cebac6c
to
e0a0d64
Compare
|
Changes Unknown when pulling e0a0d64 on shawn-hurley:remote-execution into ** on openshift:master**. |
* first pass rebase out * Adding vendor dep changes for remotecommand * adding ability to remote execute with client-go api * fixing log level for expected log. * left over fmt instead of log statements * fixing a comment typo
Describe what this PR does and why we need it:
moves runtime v1 credential extraction to the remote API
Changes proposed in this pull request