From f7ac22ad5d693f631d29625c49f5d2fa39a38d88 Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Fri, 21 Jan 2022 13:00:00 -0800 Subject: [PATCH 1/7] KEP-2887: PRR for beta. --- keps/prod-readiness/sig-api-machinery/2887.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/keps/prod-readiness/sig-api-machinery/2887.yaml b/keps/prod-readiness/sig-api-machinery/2887.yaml index 7955aae9bad..5d44b0bdd38 100644 --- a/keps/prod-readiness/sig-api-machinery/2887.yaml +++ b/keps/prod-readiness/sig-api-machinery/2887.yaml @@ -1,3 +1,5 @@ kep-number: 2887 alpha: approver: "@deads2k" +beta: + approver: "@deads2k" From de991a99017c21a4f448ddd21ce6d00afddc46eb Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Tue, 25 Jan 2022 14:12:03 -0800 Subject: [PATCH 2/7] revise for Beta. --- .../2887-openapi-enum-types/README.md | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/keps/sig-api-machinery/2887-openapi-enum-types/README.md b/keps/sig-api-machinery/2887-openapi-enum-types/README.md index e9c4a3b0501..824160f942c 100644 --- a/keps/sig-api-machinery/2887-openapi-enum-types/README.md +++ b/keps/sig-api-machinery/2887-openapi-enum-types/README.md @@ -351,6 +351,13 @@ const ( Here, `StorageMedium` can have infinite number of possible values, which disqualify it as an enum type. +The Go parser has a limitation on type alias, for example, +```go +type Foo string +type FooAlias = Foo +``` +would result in the parser to treat FooAlias as a duplicated type. A workaround will be implemented during alpha-to-beta graduation. + ### Risks and Mitigations +If the API-server has multiple replicas, with some instances not yet enabling this feature, +whether the returned OpenAPI Spec contains enum types will depend on which instance holds the leader lock. ###### What specific metrics should inform a rollback? @@ -587,6 +590,8 @@ will rollout across nodes. What signals should users be paying attention to when the feature is young that might indicate a serious problem? --> +Standard API Server metrics apply to this feature. For example, when +`apiserver_request_duration_seconds` is too high for `/openapi` endpoints. ###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested? @@ -596,12 +601,15 @@ Longer term, we may want to require automated upgrade/rollback tests, but we are missing a bunch of machinery and tooling and can't do that now. --> +Enabling, disabling, and re-enabling the feature gate yields expected outcomes. ###### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.? +No. + ### Monitoring Requirements - +N/A. Workloads that queries the OpenAPI specs automatically use this feature if enabled. ###### How can someone using this feature know that it is working for their instance? -- [ ] Events - - Event Reason: -- [ ] API .status - - Condition name: - - Other field: -- [ ] Other (treat as last resort) - - Details: +- [X] Other (treat as last resort) + - Details: the returned OpenAPI Spec contains enum types. ###### What are the reasonable SLOs (Service Level Objectives) for the enhancement? @@ -651,6 +654,7 @@ high level (needs more precise definitions) those may be things like: These goals will help you determine what you need to measure (SLIs) in the next question. --> +This feature does not affect the SLO of API Server or any other components. ###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service? @@ -658,12 +662,9 @@ question. Pick one more of these and delete the rest. --> -- [ ] Metrics - - Metric name: - - [Optional] Aggregation method: - - Components exposing the metric: -- [ ] Other (treat as last resort) - - Details: +- [X] Metrics + - Metric name: apiserver_request_duration_seconds + - Components exposing the metric: kube-api-server ###### Are there any missing metrics that would be useful to have to improve observability of this feature? @@ -695,6 +696,11 @@ and creating new ones, as well as about cluster-level services (e.g. DNS): - Impact of its degraded performance or high-error rates on the feature: --> +- kube-aapi-server + - Usage description: + - Impact of its outage on the feature: The `/openapi` endpoint is unavailable + - Impact of its degraded performance or high-error rates on the feature: The `/openapi` endpoint is degraded. + ### Scalability ###### How does this feature react if the API server and/or etcd is unavailable? +This feature is part of API server. The feature is unavailable if API server is unavailable. ###### What are other known failure modes? @@ -799,9 +806,12 @@ For each of them, fill in the following information by copying the below templat Not required until feature graduated to beta. - Testing: Are there any tests for failure mode? If not, describe why. --> +N/A. This feature fails if and only if the API Server becomes unavailable. ###### What steps should be taken if SLOs are not being met to determine the problem? +N/A. This feature fails if and only if the API Server becomes unavailable. + ## Implementation History +- 09-09-2021 `Summary` and `Motivation` sections being merged, signaling SIG acceptance +- 09-09-2021 `Proposal` section being merged, signaling agreement on a proposed design +- 09-14-2021 Enum type generator merged as `kubernetes/kube-openapi#242` +- 11-16-2021 Enum type support for OpenAPI v2 merged as #105057 + ## Drawbacks -N/A. Workloads that queries the OpenAPI specs automatically use this feature if enabled. +N/A. Workloads that query the OpenAPI specs automatically use this feature if enabled. ###### How can someone using this feature know that it is working for their instance? -- kube-aapi-server +- `kube-apiserver` - Usage description: - Impact of its outage on the feature: The `/openapi` endpoint is unavailable - Impact of its degraded performance or high-error rates on the feature: The `/openapi` endpoint is degraded. From cf3137eb692e95380524885dd7ae425744accda3 Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Wed, 2 Feb 2022 14:44:27 -0800 Subject: [PATCH 5/7] fix failure mode of replicated API server. --- keps/sig-api-machinery/2887-openapi-enum-types/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/keps/sig-api-machinery/2887-openapi-enum-types/README.md b/keps/sig-api-machinery/2887-openapi-enum-types/README.md index c4c72fb2520..683c7b6d24e 100644 --- a/keps/sig-api-machinery/2887-openapi-enum-types/README.md +++ b/keps/sig-api-machinery/2887-openapi-enum-types/README.md @@ -585,7 +585,8 @@ rollout. Similarly, consider large clusters and how enablement/disablement will rollout across nodes. --> If the API-server has multiple replicas, with some instances not yet enabling this feature, -whether the returned OpenAPI Spec contains enum types will depend on which instance holds the leader lock. +whether the returned OpenAPI Spec contains enum types will depend on which instance handle the request +which could result in inconsistent responses for the same request. ###### What specific metrics should inform a rollback? From daebe1444b54178d1d3dc3fc90b14f11862797cf Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Wed, 2 Feb 2022 16:19:47 -0800 Subject: [PATCH 6/7] externalize gengo alias issue. --- .../2887-openapi-enum-types/README.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/keps/sig-api-machinery/2887-openapi-enum-types/README.md b/keps/sig-api-machinery/2887-openapi-enum-types/README.md index 683c7b6d24e..95240008a8b 100644 --- a/keps/sig-api-machinery/2887-openapi-enum-types/README.md +++ b/keps/sig-api-machinery/2887-openapi-enum-types/README.md @@ -351,15 +351,14 @@ const ( Here, `StorageMedium` can have infinite number of possible values, which disqualify it as an enum type. -The Go parser has a limitation on parsing type aliases. The parser cannot distinguish -between the original definition and its aliases. For example, -```go -type Foo string -type FooAlias = Foo -``` -would result in the parser to treat FooAlias and Foo as the same type. -As a result, `gengo` produce either `Foo` or `FooAlias` but not both. -As a workaround, during beta graduation, the enum parser will be updated to accept any name of the type. +The parser of Go compiler and, as a result, `gengo`, does not support type aliases. See https://github.com/kubernetes/gengo/issues/224 for details. +As of Kubernetes 1.23, we do not have any type aliases in kubernetes/kubernetes repo. Any attempt to add aliases would break the code generation. +Example: https://github.com/kubernetes/kubernetes/pull/106300 + +However, as of 1.23, the enum marker is the only marker to be added to a type declaration, and would be the first marker to be affected. +Until there is a fix to `gengo`, the enum generator has the following limitations: +- the enum marker must not be added to aliases +- an aliased enum type or value SHOULD NOT have comments. Otherwise, the comments will be squashed with these of the original with undefined ordering. ### Risks and Mitigations From 80eb0fb979224ef0198fe72263d859e2cdd655a2 Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Thu, 3 Feb 2022 09:30:51 -0800 Subject: [PATCH 7/7] mention enforcement of aliases rules. --- keps/sig-api-machinery/2887-openapi-enum-types/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/keps/sig-api-machinery/2887-openapi-enum-types/README.md b/keps/sig-api-machinery/2887-openapi-enum-types/README.md index 95240008a8b..d785a60dbdf 100644 --- a/keps/sig-api-machinery/2887-openapi-enum-types/README.md +++ b/keps/sig-api-machinery/2887-openapi-enum-types/README.md @@ -358,7 +358,9 @@ Example: https://github.com/kubernetes/kubernetes/pull/106300 However, as of 1.23, the enum marker is the only marker to be added to a type declaration, and would be the first marker to be affected. Until there is a fix to `gengo`, the enum generator has the following limitations: - the enum marker must not be added to aliases -- an aliased enum type or value SHOULD NOT have comments. Otherwise, the comments will be squashed with these of the original with undefined ordering. +- an aliased enum type or value SHOULD NOT have comments. Otherwise, the comments will be squashed with these of the original with undefined ordering. + +If the fix to `gengo` does not arrive in this release cycle, detection and enforcement the rules above will be added to the `verify-*.sh` scripts. ### Risks and Mitigations