tests: Fixes tests for Windows (containerd, RunAsUserName)#83058
tests: Fixes tests for Windows (containerd, RunAsUserName)#83058k8s-ci-robot merged 1 commit intokubernetes:masterfrom
Conversation
cf2e8d2 to
da2eca6
Compare
|
/test pull-kubernetes-node-e2e |
|
/assign @jingxu97 |
test/e2e/common/util.go
Outdated
There was a problem hiding this comment.
now we can only set userId as 1000 for linux. For current tests, it is fine. Will it become a concern in the future?
There was a problem hiding this comment.
Nothing changed, userid 1000 was also used before this change. Other userids can still be used.
There was a problem hiding this comment.
Here it is kind of hardcoded to set user id to 1000. My point is all tests will have to share the exact same value which is defined here.
There was a problem hiding this comment.
I don't see the problem.
There was a problem hiding this comment.
@jingxu97 - Before this value was split across many places, and this change defines it once. Are you saying you'd prefer the old way where 1000 was individually hardcoded in many test cases?
da2eca6 to
c7bb0b2
Compare
|
/test pull-kubernetes-e2e-gce-csi-serial |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bclau, msau42 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
test/e2e/common/util.go
Outdated
There was a problem hiding this comment.
maybe make it as constant, so you don't need to define it multiple places?
There was a problem hiding this comment.
Can't, they have to be vars. This is what happens when they're constants:
test/e2e/common/util.go:218:94: cannot take the address of nonAdminTestUserName
test/e2e/common/util.go:220:40: cannot take the address of nonRootTestUserID
Since we've added support for RunAsUserName, we can now run some new tests. However, the [LinuxOnly] tag will have to remain until the WindowsRunAsUserName feature becomes enabled by default. Additionally, Containerd supports file mounting on Windows, and some tests will be able to pass on Windows with Containerd instead of Docker.
a682790 to
f0e6d8e
Compare
|
/test pull-kubernetes-conformance-kind-ipv6 |
|
/hold |
|
LGTM from a conformance point of view |
|
/lgtm |
|
/hold cancel |
We've added the WindowsRunAsUserName feature some time ago, and it was promoted to Beta for v1.17. We can now remove the [LinuxOnly] tag for a few tests. Depends On: kubernetes#83058 Depends On: kubernetes#84882
What type of PR is this?
/kind failing-test
/sig windows
/sig testing
/area conformance
What this PR does / why we need it:
Since we've added support for
RunAsUserName, we can now run some new tests. However, the[LinuxOnly]tag will have to remain until theWindowsRunAsUserNamefeature becomes enabled by default.Additionally, Containerd supports file mounting on Windows, and some tests will be able to pass on Windows with Containerd instead of Docker.
Which issue(s) this PR fixes:
Related: #70189
Related: #68791
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: