-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
pkg/kepval: enable PRR check for implemented KEPs #3007
Changes from all commits
c48b033
ed17414
761668f
105f034
dd3cb63
6411a4d
cf70bb1
a5a7071
3f4b245
a28ce6d
6f464ad
4b78b37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
#!/usr/bin/env python3 | ||
|
||
# Copyright 2021 The Kubernetes Authors. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
"""Edit KEPs en-masse by round-tripping them through ruamel.yaml | ||
|
||
This is not intended for general usage, because: | ||
- many keps have different formatting, and we're not at a point where | ||
we can enforce formatting standards, so this is almost guaranteed | ||
to introduce formatting change noise | ||
- the idea is to manually edit this file with the specific edit to be | ||
done, rather that developing a general purpose language to do this | ||
""" | ||
|
||
import argparse | ||
import glob | ||
|
||
from os import path | ||
|
||
import ruamel.yaml | ||
|
||
# Files that will be ignored | ||
EXCLUDED_FILES = [] | ||
# A hilariously large line length to ensure we never line-wrap | ||
MAX_WIDTH = 2000000000 | ||
|
||
def setup_yaml(): | ||
# Setup the ruamel.yaml parser | ||
yaml = ruamel.yaml.YAML(typ='rt') | ||
yaml.preserve_quotes = True | ||
# This is what's used in the template, currently ~36 KEPs have drifted | ||
yaml.indent(mapping=2, sequence=4, offset=2) | ||
yaml.width = MAX_WIDTH | ||
return yaml | ||
|
||
def edit_kep(yaml, file_name, force_rewrite=False): | ||
with open(file_name, "r") as fp: | ||
kep = yaml.load(fp) | ||
|
||
rewrite = force_rewrite | ||
|
||
stage = kep.get("stage", "unknown") | ||
status = kep.get("status", "unknown") | ||
latest_milestone = kep.get("latest-milestone", "unknown") | ||
last_updated = kep.get("last-updated", "unknown") | ||
milestone = kep.get("milestone", {}) | ||
|
||
if status == "implemented": | ||
if latest_milestone == "unknown": | ||
print(f'status: {status} stage: {stage} last-updated: {last_updated} file: {file_name}') | ||
kep["latest-milestone"] = "0.0" | ||
rewrite = True | ||
if stage == "unknown": | ||
if latest_milestone == "unknown": | ||
kep["stage"] = "stable" | ||
else: | ||
kep["stage"] = [s for s,v in milestone.items() if v == latest_milestone][0] | ||
rewrite = True | ||
|
||
# Dump KEP to file_name | ||
if rewrite: | ||
print(f' writing {file_name}') | ||
with open(file_name, "w") as fp: | ||
yaml.dump(kep, fp) | ||
fp.truncate() | ||
|
||
def main(keps_dir, force_rewrite): | ||
yaml = setup_yaml() | ||
for f in glob.glob(f'{keps_dir}/**/kep.yaml', recursive=True): | ||
if path.basename(f) not in EXCLUDED_FILES: | ||
try: | ||
print(f'processing file: {f}') | ||
edit_kep(yaml, f, force_rewrite) | ||
except Exception as e: # pylint: disable=broad-except | ||
print(f'ERROR: could not edit {f}: {e}') | ||
|
||
if __name__ == '__main__': | ||
PARSER = argparse.ArgumentParser( | ||
description='Does things to KEPs') | ||
PARSER.add_argument( | ||
'--keps-dir', | ||
default='../keps', | ||
help='Path to KEPs directoryProw Job Directory') | ||
PARSER.add_argument( | ||
'--force', | ||
default=False, | ||
help='Force rewrite of all KEPs') | ||
ARGS = PARSER.parse_args() | ||
|
||
main(ARGS.keps_dir, ARGS.force) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
kep-number: 1143 | ||
stable: | ||
approver: "@wojtek-t" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
kep-number: 2907 | ||
stable: | ||
approver: "@johnbelamaric" | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
kep-number: 2206 | ||
beta: | ||
approver: "@johnbelamaric" | ||
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #2758 is the PR that moved this KEP to beta |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
kep-number: 2539 | ||
beta: | ||
approver: "@johnbelamaric" | ||
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can speak a bit more to this one since I'm on the KEP. #2867 retroactively declared this beta since that's where this ultimately ended up during 1.21. This KEP was initially declared implementable at alpha, before PRR was required. The retroactive PR incorrectly called this "implemented" at beta, so it skipped PRR this cycle. Given that this KEP is related to prow, and not to anything that is contained in a kubernetes release, I think this is pretty uncontroversial There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree but cc: @johnbelamaric just for viz |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,3 +21,5 @@ replaces: | |
- n/a | ||
superseded-by: | ||
- n/a | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,3 +17,5 @@ approvers: | |
- "@liggitt" | ||
see-also: | ||
replaces: | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,3 +19,5 @@ replaces: | |
- n/a | ||
superseded-by: | ||
- n/a | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,3 +28,4 @@ approvers: | |
- "@johnbelamaric" | ||
see-also: | ||
replaces: | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,3 +22,4 @@ stage: stable | |
feature-gates: [] | ||
disable-supported: false | ||
metrics: [] | ||
latest-milestone: '0.0' |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,3 +16,5 @@ approvers: | |
creation-date: 2020-04-14 | ||
last-updated: 2021-01-06 | ||
status: implemented | ||
latest-milestone: '0.0' | ||
stage: stable | ||
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So like here's a concern of mine re: assuming everything is
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I believe this one is implemented as it was a KEP to create a subproject that was created: #1687 It was out of tree and untracked, the subproject was created, so I think this is done and implemented. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,3 +14,4 @@ approvers: | |
- "@ritazh" | ||
- "@mikedanese" | ||
status: implemented | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ approvers: | |
- "@pwittrock" | ||
creation-date: 2020-12-21 | ||
last-updated: 2020-01-15 | ||
status: implemented | ||
status: implementable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is in response to #2758 (comment) We have a KEP about leaving things at perma-beta, though maybe it only applies to kubernetes resources There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jrrickard wdyt? I tend to think implemented is the right choice given the past pr. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like there was an unanswered question in that PR, and the chair at the time was open to corrections down the line. I see how many other KEP's violate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The permabeta KEP is about served APIs - it implemented automatic disabling of apis that are beta for too many releases. But the general principle applies beyond APIs - we really shouldn't be leaving stuff in a perma-beta state. If we mark this as |
||
|
||
latest-milestone: "1.22" | ||
stage: "beta" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,3 +24,5 @@ see-also: | |
replaces: | ||
superseded-by: | ||
- n/a | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,3 +24,5 @@ replaces: | |
- "0008-kustomize.md" | ||
superseded-by: | ||
- n/a | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,3 +13,5 @@ creation-date: 2019-07-22 | |
last-updated: 2021-01-26 | ||
status: implemented | ||
|
||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,3 +20,5 @@ approvers: | |
creation-date: 2020-02-15 | ||
see-also: | ||
replaces: | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,3 +15,5 @@ status: implemented | |
see-also: | ||
replaces: | ||
superseded-by: | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,3 +16,5 @@ status: implemented | |
see-also: | ||
replaces: | ||
superseded-by: | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,3 +7,5 @@ approvers: | |
- thockin | ||
owning-sig: sig-network | ||
status: implemented | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,3 +23,5 @@ status: implemented | |
see-also: | ||
replaces: | ||
superseded-by: | ||
latest-milestone: '0.0' | ||
stage: stable |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,3 +21,5 @@ status: implemented | |
see-also: | ||
replaces: | ||
superseded-by: | ||
latest-milestone: '0.0' | ||
stage: stable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#3007 moved this to implemented but missed PRR files
I think this is out of tree so maybe not explicitly covered by PRR