diagnostic reorg and NetworkCheck fix #18709
Conversation
openshift-ci-robot
added
the
approved
openshift-ci-robot
added
the
size/L
sosiouxme
force-pushed
the
20180221-diagnostic-reorg
branch
from
9ccc2ec to
4c41254
Compare
sosiouxme
force-pushed
the
20180221-diagnostic-reorg
branch
from
4c41254 to
bae0c9f
Compare
Use convenience function that could be refactored to be based on something else, rather than directly depending on docker.
Formerly, in order to run diagnostics inside a pod, NetworkCheck looked up the user's kubeconfig file and wrote it to a secret for the pod to use as its kubeconfig. However, this only worked if the current context is the cluster-admin. Cluster diagnostics can work even if the cluster-admin context is not currently selected. For NetworkCheck to work the same it is now a cluster diagnostic, and it writes a new pod kubeconfig with only the cluster-admin context.
sosiouxme
force-pushed
the
20180221-diagnostic-reorg
branch
from
bae0c9f to
4213489
Compare
pkg/oc/admin/diagnostics/cluster.go
Outdated
| nd.Level = o.LogOptions.Level | ||
| nd.Factory = o.Factory | ||
| nd.PreventModification = o.PreventModification | ||
| diagnostics = append(diagnostics, nd) |
There was a problem hiding this comment.
this could cause empty d to be added to diagnostics in this iteration.
pkg/oc/admin/diagnostics/cluster.go
Outdated
| nd.PreventModification = o.PreventModification | ||
| diagnostics = append(diagnostics, nd) | ||
| d = nd |
There was a problem hiding this comment.
ok, issue in previous commit got fixed here
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pravisankar, sosiouxme The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
/cherrypick release-3.9 |
|
@sosiouxme: once the present PR merges, I will cherry-pick it on top of release-3.9 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/hold |
openshift-ci-robot
added
do-not-merge/hold
|
Automatic merge from submit-queue (batch tested with PRs 18778, 18709, 18876, 18897, 18652). |
|
@sosiouxme: new pull request created: #18903 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
The leading commits of this PR change nothing visible to the user. Internally, they remove a direct dependency on docker and reorganize the two diagnostics that currently run diagnostics in a pod, PodDiagnostic and NetworkCheck. The reorg brings files for each under one directory so it's easier to find and understand them.
The final two commits address #16596 by making NetworkCheck a cluster diagnostic and constructing the pod kubeconfig from the (now guaranteed to be an admin) config.
Commits can be squashed or split out into separate PRs if preferable. I would expect it to be rather confusing to review as a whole, especially since a lot of the changes are just stuff moving around; easier to to grasp one commit at a time.