Handle "origin" container like a k8s container on cleanup #13952
Conversation
When we start the master using `oc cluster up`, we run a container called "origin" that was previously not being considered a container we needed to be getting logs for or stopping and cleaning up. We should consider this container the same as any other and not have custom logic for it. Signed-off-by: Steve Kuznetsov <[email protected]>
|
[test] |
| @@ -86,18 +101,33 @@ readonly -f os::cleanup::dump_container_logs | |||
| # Returns: | |||
| # None | |||
| function os::cleanup::internal::list_k8s_containers() { | |||
There was a problem hiding this comment.
We have some downstream users of this code, could remove this func but didn't want to be backwards incompatible in case someone was using it directly.
stevekuznetsov
changed the title
hack/lib/cleanup.sh
Outdated
| # os::cleanup::internal::list_k8s_containers returns a space-delimited list of | ||
| # docker containers that belonged to k8s. | ||
| # docker containers that belonging to k8s. |
There was a problem hiding this comment.
I'm not sure you want this grammatically incorrect change? :-p
There was a problem hiding this comment.
whoa. embarrassing
hack/lib/cleanup.sh
Outdated
| local name; name="$( docker inspect --format '{{ .Name }}' "${id}" )" | ||
| os::log::debug "Dumping logs for ${id} to ${name}.log" | ||
| docker logs "${id}" >"${container_log_dir}/${name}.log" 2>&1 | ||
| done | ||
| } | ||
| readonly -f os::cleanup::dump_container_logs | ||
|
|
||
| # os::cleanup::internal::list_our_containers returns a space-delimited list of | ||
| # docker containers that belonging to some part of the Origin deployment. |
|
Other than some grammar, LGTM |
The job scripts we use to wrap tests in Jenkins will gather the `docker` system journal where necessary, so we do not need to be gathering it as well explicitly in the test scripts themselves. Signed-off-by: Steve Kuznetsov <[email protected]>
stevekuznetsov
force-pushed
the
skuznets/cluster-up-logs
branch
from
b8057fd to
dacfb85
Compare
|
Updated grammar. [merge] |
|
Evaluated for origin merge up to dacfb85 |
|
#13942 [test] |
|
Evaluated for origin test up to dacfb85 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1050/) (Base Commit: 03beec0) |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/520/) (Base Commit: fed098c) (Image: devenv-rhel7_6189) |
Remove Docker journal gathering steps
The job scripts we use to wrap tests in Jenkins will gather the
dockersystem journal where necessary, so we do not need to be gathering it as
well explicitly in the test scripts themselves.
Signed-off-by: Steve Kuznetsov [email protected]
Handle "origin" container like a k8s container on cleanup
When we start the master using
oc cluster up, we run a containercalled "origin" that was previously not being considered a container
we needed to be getting logs for or stopping and cleaning up. We should
consider this container the same as any other and not have custom logic
for it.
Signed-off-by: Steve Kuznetsov [email protected]
Fixes #13949
@ncdc PTAL