deploy fixes #281
deploy fixes #281
Conversation
Bumping the external-provisioner broke the distributed deployment due to a breaking change from alpha to beta (POD_NAMESPACE -> NAMESPACE).
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pohly 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 |
k8s-ci-robot
added
cncf-cla: yes
|
/assign @chrishenzie |
deploy/util/deploy-hostpath.sh
Outdated
| (set +e; set -x; kubectl describe all,role,clusterrole,rolebinding,clusterrolebinding,serviceaccount,storageclass,csidriver --all-namespaces -l app.kubernetes.io/instance=hostpath.csi.k8s.io) | ||
| echo | ||
| echo "Pod logs:" | ||
| kubectl get pods -l app.kubernetes.io/instance=hostpath.csi.k8s.io --all-namespaces -o=jsonpath='{range .items[*]}{"\n"}{.metadata.name}{" "}{range .spec.containers[*]}{.name}{" "}{end}{end}' | while read -r pod containers; do |
There was a problem hiding this comment.
I don't see logs of the driver pod. kubectl get pods ... returns everything:
csi-hostpath-attacher-0 csi-attacher
csi-hostpath-provisioner-0 csi-provisioner
csi-hostpath-resizer-0 csi-resizer
csi-hostpath-snapshotter-0 csi-snapshotter
csi-hostpath-socat-0 socat
csi-hostpathplugin-0 csi-external-health-monitor-agent csi-external-health-monitor-controller node-driver-registrar hostpath liveness-probe
But the loop skips the last line?
There was a problem hiding this comment.
The last line returned by kubectl get pods does not have \n and read does not like it?
There was a problem hiding this comment.
Good catch. I checked that logs were printed, but missed the missing ones. Moving \n after the output for each pod fixes this.
Even better would be to print "pod name + container name" for each container because then we can avoid one for loop, but I don't know how to do that with jsonpath. If you have a suggestion, please share. In the meantime I'll update the PR with the fix.
There was a problem hiding this comment.
I did not know you can have loops in jsonpath until today, so no, I don't have a better one.
With the recently introduced labels it becomes possible to describe all objects that make up the driver deployment. This might be a lot of output, but as this typically runs in a CI environment, it is better to print too much than to little information because retrieving more information later isn't possible. Now also the logs of all containers are printed.
The manual editing as part of 03204df caused the two deployments to diverge (different label selectors, service defined with old "app" label in 1.20). Now "diff -r deploy/kubernetes-1.18 deploy/kubernetes-1.20" shows only intentional changes around the snapshotter beta API.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/retest Test flake in https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-csi_csi-driver-host-path/281/pull-kubernetes-csi-csi-driver-host-path-1-19-on-kubernetes-1-19/1389506592101109760: "External Storage [Driver: hostpath.csi.k8s.io] [Testpattern: Dynamic PV (filesystem volmode)] multiVolume [Slow] should concurrently access the single volume from pods on the same node" |
|
@pohly: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
https://k8s-testgrid.appspot.com/sig-storage-csi-ci#distributed-on-master is still failing, the external-provisioner update was incomplete. This showed again that we want the container logs to be visible in CI runs, so this gets added.
Which issue(s) this PR fixes:
Fixes #94
Does this PR introduce a user-facing change?: