kubernetes: unit tests should not require root
What happened:
- Some of our unit tests fail when not run as root (locally), currently looking at
cluster/gce/*but I suspect there are more of these
What you expected to happen:
- Our unit tests should not fail as root, any test requiring this has gone well past being a “unit” test
- Our unit tests should probably not run as root in CI to prevent regressions
How to reproduce it (as minimally and precisely as possible):
make test
/sig testing
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 3
- Comments: 35 (33 by maintainers)
THANK YOU! I keep trying to get to this and something else comes up. That change sounds totally correct: Please do file the PR and I’ll try to make sure we get it reviewed 🙏
audit unit test fix in #102610
thanks to @PushkarJ it’s time to roll over the job config and close the gap on this permanently 🎉
In my local setup I was running as non-root user, but
make testdid not fail for me. Probably absence of user entry with a name is what (or one of the reasons that) causes the failure as we discussed here: https://github.com/kubernetes/kubernetes/pull/103127#discussion_r657505781CI job for unit tests, with non-root user is now green: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/103127/pull-kubernetes-unit-experimental/1407797774702874624
@BenTheElder I was able to reproduce this locally and got a much more readable result. It seems like
user.Current()returns error, when current user is non-root, but the code skips error checking and uses theuservalue returned (which is nil), which induces the panic in theifcondition forUID == 0check:After I added the error check, the tests passed. Will start working on PR to fix this, if there are no concerns
k8s.io/kubernetes/pkg/volume/csi: TestAttacherMountDevicestill seems to produce no logs when it fails.I noticed it has UID==0 specific codepath(s) but those seem to be to skip certain cases when UID is 0 https://github.com/kubernetes/kubernetes/blob/ea0764452222146c47ec826977f49d7001b0ea8c/pkg/volume/csi/csi_attacher_test.go#L1191
don’t add new unit tests that require root permission to run
sure.
I think we should remove the unit tests in question since they run configure-helper.sh. configure-helper.sh requires running as root by design.