Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[windows] Test: Check for failed sandbox pod when testing for RunAsUserName #105943

Merged
merged 2 commits into from Oct 28, 2021
Merged

[windows] Test: Check for failed sandbox pod when testing for RunAsUserName #105943

merged 2 commits into from Oct 28, 2021

Conversation

jsturtevant
Copy link
Contributor

@jsturtevant jsturtevant commented Oct 27, 2021 •
edited

What type of PR is this?

/kind failing-test

What this PR does / why we need it:

With Hostprocess CRI updates the podsandbox was passed RunAsUserName to the security context in CRI and containerd (1.6+) was updated to use the CRI sandbox info: containerd/containerd#5865.

This puts the pod into pending state which is expected: #104635 (comment)

Which issue(s) this PR fixes:

Fixes #104635

Special notes for your reviewer:

It was decided to fix the test now, and create a new feature request to add additional information to pod sandbox failures to mark them in failed if the runtime knows it can: #104635 (comment)

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 27, 2021
@jsturtevant
Copy link
Contributor Author

/sig windows
/assign @marosset
/milestone 1.23

@k8s-ci-robot k8s-ci-robot added the sig/windows Categorizes an issue or PR as relevant to SIG Windows. label Oct 27, 2021
@k8s-ci-robot
Copy link
Contributor

@jsturtevant: The provided milestone is not valid for this repository. Milestones in this repository: [next-candidate, v1.16, v1.17, v1.18, v1.19, v1.20, v1.21, v1.22, v1.23, v1.24, v1.25, v1.26]

Use /milestone clear to clear the milestone.

In response to this:

/sig windows
/assign @marosset
/milestone 1.23

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.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Oct 27, 2021
@jsturtevant
Copy link
Contributor Author

/triage accepted
/priority important-soon

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 27, 2021
nick5616
nick5616 reviewed Oct 27, 2021
View reviewed changes
test/e2e/windows/security_context.go
"involvedObject.namespace": podInvalid.Namespace,
"reason": events.FailedCreatePodSandBox,
}.AsSelector().String()
hcsschimError := "The user name or password is incorrect."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a module to get this error string from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I searched hcsshim and did not find this error, It seems this is coming from the OS and hcsshim is wrapping it.

@marosset
Copy link
Contributor

/milestone v1.23

@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Oct 28, 2021
@marosset
Copy link
Contributor

Can we update the PR to also have a test case where only the RunAsUserName for a container is set to an invalid user?

@jsturtevant jsturtevant mentioned this pull request Oct 28, 2021
Closed
@jsturtevant
Copy link
Contributor Author

Can we update the PR to also have a test case where only the RunAsUserName for a container is set to an invalid user?

updated

@marosset
Copy link
Contributor

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Oct 28, 2021
marosset
marosset approved these changes Oct 28, 2021
View reviewed changes
Copy link
Contributor

@marosset marosset left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 28, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsturtevant, marosset

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit d6db275 into kubernetes:master Oct 28, 2021
@jsturtevant jsturtevant mentioned this pull request Dec 15, 2021
Merged
k8s-ci-robot added a commit that referenced this pull request Jan 4, 2022
@k8s-ci-robot
Merge pull request #107064 from jsturtevant/automated-cherry-pick-of-…
87eafcb 
…#105943-upstream-release-1.22

Automated cherry pick of #105943: Check for failed sandbox and failed workload containers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nick5616 nick5616 nick5616 left review comments

@marosset marosset marosset approved these changes

@jayunit100 jayunit100 Awaiting requested review from jayunit100

@ksubrmnn ksubrmnn Awaiting requested review from ksubrmnn

Assignees

@marosset marosset

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/testing Categorizes an issue or PR as relevant to SIG Testing. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.23
Development

Successfully merging this pull request may close these issues.

Pods that fail to create podsandbox due to errors don't mark the pods as failed
4 participants
@jsturtevant @k8s-ci-robot @marosset @nick5616