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

Promote RunAsGroup e2e test to Conformance #98645

Merged
merged 1 commit into from Feb 23, 2021
Merged

Promote RunAsGroup e2e test to Conformance #98645

merged 1 commit into from Feb 23, 2021

Conversation

krmayankk
Copy link

/area conformance

@kubernetes/sig-architecture-pr-reviews @kubernetes/sig-foo-pr-reviews @kubernetes/cncf-conformance-wg

@k8s-ci-robot k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. area/conformance Issues or PRs related to kubernetes conformance tests size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/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 Feb 1, 2021
@k8s-ci-robot k8s-ci-robot added area/test sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Feb 1, 2021
@krmayankk krmayankk added this to Issues To Triage in conformance-definition Feb 1, 2021
@krmayankk krmayankk mentioned this pull request Feb 1, 2021
Merged
@ehashman
Copy link
Member

ehashman commented Feb 1, 2021

/release-note-none
/kind cleanup
/priority important-soon

Looks good from a triage perspective for node, but this is a testing PR so I won't apply the label.

/hold

because this e2e appears to be failing.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 1, 2021
@oomichi
Copy link
Member

oomichi commented Feb 1, 2021

Is RunAsGroup already a GA feature?
According to https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/ it seems a Beta feature since v1.14.
Is that changed recently?

This question comes because of one of conformance requirements it tests only GA, non-optional features or APIs (e.g., no alpha or beta endpoints, no feature flags required, no deprecated features) since https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md#conformance-test-requirements

/cc @oomichi

@ehashman
Copy link
Member

ehashman commented Feb 1, 2021

@oomichi see #94641

@oomichi
Copy link
Member

oomichi commented Feb 1, 2021

@oomichi see #94641

@ehashman Thanks for pointing it out.
#94641 should be before this as the requirement.

@SergeyKanzhelev SergeyKanzhelev moved this from Triage to PRs need attention from the author in SIG Node CI/Test Board Feb 1, 2021
@dims
Copy link
Member

dims commented Feb 13, 2021

looks like #94641 missed v1.21, we can revisit this next release

@krmayankk
Copy link
Author

@dims no, we are graduating RunAsGroup this release, see kubernetes/enhancements#213 (comment)

@krmayankk please rebase

done

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 19, 2021
@dims
Copy link
Member

dims commented Feb 19, 2021

@krmayankk @ehashman my apologies, i thought there were some API changes that needed to land as part of the other PR (and then the tests have to merge typically after that)

So i:

Since the PR just removes the feature flag and nothing else, i am happy to LGTM this change, please note that we will have to yank this IF the PR does not land in 1.21

thanks!

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 19, 2021
@dims
Copy link
Member

dims commented Feb 19, 2021

/retest

@ehashman
Copy link
Member

/retest
/triage accepted
(for node)

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 19, 2021
@ehashman ehashman moved this from PRs need attention from the author to PRs - Reviewer lgtm'd in SIG Node CI/Test Board Feb 19, 2021
@krmayankk
Copy link
Author

/test pull-kubernetes-conformance-kind-ga-only-parallel

@krmayankk
Copy link
Author

/assign @Random-Liu @smarterclayton

@spiffxp spiffxp moved this from Issues To Triage to Promotion PRs Needing Approval in conformance-definition Feb 23, 2021
@spiffxp spiffxp moved this from Promotion PRs Needing Approval to Promotion PRs Needing Two Weeks (flake free) in conformance-definition Feb 23, 2021
@spiffxp
Copy link
Member

spiffxp commented Feb 23, 2021

https://testgrid.k8s.io/sig-release-master-blocking#gce-cos-master-default&include-filter-by-regex=RunAsGroup&width=5 flakiness looks good

need to review that these tests adhere to https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md#conformance-test-requirements

@spiffxp
Copy link
Member

spiffxp commented Feb 23, 2021

it is non-privileged (e.g., does not require root on nodes, access to raw network interfaces, or cluster admin permissions)

Was the only thing that jumped out at me, but my read of these tests and kubernetes/enhancements#213 is that this meets the requirement

spiffxp
spiffxp approved these changes Feb 23, 2021
View reviewed changes
Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
/hold cancel
re: #98645 (comment) these tests are no longer failing

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, krmayankk, spiffxp

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

@spiffxp spiffxp moved this from Promotion PRs Needing Two Weeks (flake free) to Promotion PRs Needing Approval in conformance-definition Feb 23, 2021
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 23, 2021
@k8s-ci-robot k8s-ci-robot merged commit a5f9f25 into kubernetes:master Feb 23, 2021
conformance-definition automation moved this from Promotion PRs Needing Approval to Done Feb 23, 2021
SIG Node CI/Test Board automation moved this from PRs - Reviewer lgtm'd to Done Feb 23, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone Feb 23, 2021
@arunmk arunmk mentioned this pull request Mar 2, 2021
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spiffxp spiffxp spiffxp approved these changes

@sjenning sjenning Awaiting requested review from sjenning

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

@oomichi oomichi Awaiting requested review from oomichi

Assignees

@dims dims

@spiffxp spiffxp

@smarterclayton smarterclayton

@Random-Liu Random-Liu

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/conformance Issues or PRs related to kubernetes conformance tests area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. 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/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node CI/Test Board
Done
SIG Node CI/Test Board
Archived in project
conformance-definition
  
Done
Milestone
v1.21
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@krmayankk @ehashman @oomichi @dims @spiffxp @k8s-ci-robot @smarterclayton @Random-Liu