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

Add Conformance Tests to KEP requirements #2146

Merged
merged 1 commit into from Apr 26, 2021
Merged

Add Conformance Tests to KEP requirements #2146

merged 1 commit into from Apr 26, 2021

Conversation

hh
Copy link
Member

@hh hh commented Nov 12, 2020

No description provided.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 12, 2020
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 12, 2020
justaugustus
justaugustus requested changes Nov 24, 2020
View reviewed changes
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

@hh -- a few nits

keps/NNNN-kep-template/README.md Outdated
- [ ] (R) Graduation criteria is in place
- [ ] (R) [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) for new GA Endpoints prior to Release
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- [ ] (R) [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) for new GA Endpoints prior to Release
- [ ] (R) [Conformance Tests](https://git.k8s.io/community/contributors/devel/sig-architecture/conformance-tests.md) for new GA Endpoints prior to Release

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

keps/NNNN-kep-template/README.md Outdated
@@ -128,7 +128,9 @@ Items marked with (R) are required *prior to targeting to a milestone / release*
- [ ] (R) KEP approvers have approved the KEP status as `implementable`
- [ ] (R) Design details are appropriately documented
- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
- [ ] e2e Tests for all new Endpoints
Copy link
Member

Choose a reason for hiding this comment

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

Can we be more explicit about what we mean by Endpoints and what specifically is required here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I expanded this out a bit, and also updated the underlying metadata to be explicit about API endpoints that are part of the KEP:

https://github.com/kubernetes/enhancements/pull/2146/files#diff-56771f8d4989d3c6f35d326519113795505560c5e277b61b667c96ab09e4067dR35-R45

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/kubernetes/enhancements/pull/2146/files#r535576335

# The openAPI operations to expose in api/open-apispec/swagger.json
 apis:
   - stage: alpha
     operations:
       - useThisFeatureAlphaV1
   - stage: beta
     operations:
       - useThisFeatureBetaV1
   - stage: stable
     operations:
       - useThisFeatureV1

Copy link
Member

Choose a reason for hiding this comment

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

I am concerned that listing endpoints will be relatively burdensome for KEP authors, without providing a huge value to anyone outside of the conformance team.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed ii@20b93a5

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 3, 2020
hh
hh commented Dec 3, 2020
View reviewed changes
keps/NNNN-kep-template/kep.yaml Outdated
@@ -31,6 +31,19 @@ stage: alpha|beta|stable
# worked on.
latest-milestone: "v1.19"


Copy link
Member Author

Choose a reason for hiding this comment

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

@justaugustus @LappleApple

I think this might be a good point to update the metadata so we know velocity of API and features.
Might be a better indicator of what's coming next in a release and ensure we have testing in place.... and probably a lot of other good could come of it.

@justaugustus justaugustus added this to In progress in Enhancements Subproject via automation Dec 4, 2020
@justaugustus
Copy link
Member

I think this might be a good point to update the metadata so we know velocity of API and features.
Might be a better indicator of what's coming next in a release and ensure we have testing in place.... and probably a lot of other good could come of it.

@hh -- Great suggestion from yesterday's SIG Arch meeting!

/hold @kubernetes/enhancements -- let's discuss/review this in next week's meeting.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 4, 2020
@justaugustus justaugustus added this to the keps-beta milestone Dec 4, 2020
@LappleApple LappleApple moved this from In progress to For Review in Enhancements Subproject Feb 4, 2021
@LappleApple LappleApple moved this from For Review to PRs - Blocked, or Author Inactivity in +30 Days in Enhancements Subproject Mar 16, 2021
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 6, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 7, 2021
@LappleApple LappleApple moved this from PRs - Blocked, or Author Inactivity in +30 Days to For Review in Enhancements Subproject Apr 8, 2021
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 22, 2021
johnbelamaric
johnbelamaric reviewed Apr 22, 2021
View reviewed changes
keps/NNNN-kep-template/README.md Outdated
@@ -127,8 +127,13 @@ Items marked with (R) are required *prior to targeting to a milestone / release*
- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
- [ ] (R) KEP approvers have approved the KEP status as `implementable`
- [ ] (R) Design details are appropriately documented
- [ ] (R) Include API Operation exposed per stage (alpha/beta/GA) in kep.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, can you pull this line too

@johnbelamaric
Copy link
Member

/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 Apr 26, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hh, johnbelamaric

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 26, 2021
@kikisdeliveryservice
Copy link
Member

/lgtm
/hold cancel

@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 Apr 26, 2021
@k8s-ci-robot k8s-ci-robot merged commit 75bd21d into kubernetes:master Apr 26, 2021
Enhancements Subproject automation moved this from For Review to Done 1.22 Apr 26, 2021
@k8s-ci-robot k8s-ci-robot modified the milestones: keps-beta, v1.22 Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnbelamaric johnbelamaric johnbelamaric left review comments

@justaugustus justaugustus justaugustus requested changes

@jeremyrickard jeremyrickard Awaiting requested review from jeremyrickard

@LappleApple LappleApple Awaiting requested review from LappleApple

Assignees

@kikisdeliveryservice kikisdeliveryservice

@johnbelamaric johnbelamaric

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
Enhancements Subproject
  
Done
Milestone
v1.22
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@hh @justaugustus @johnbelamaric @k8s-ci-robot @kikisdeliveryservice