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.

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

KEP-2887: OpenAPI Enum Types to Beta #3184

Merged
merged 7 commits into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions keps/prod-readiness/sig-api-machinery/2887.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
kep-number: 2887
alpha:
approver: "@deads2k"
beta:
approver: "@deads2k"
59 changes: 39 additions & 20 deletions keps/sig-api-machinery/2887-openapi-enum-types/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ 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
Copy link
Member

Choose a reason for hiding this comment

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

What is "the Go parser"?

Copy link
Member Author

Choose a reason for hiding this comment

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

changed to "The parser of Go compiler"

Copy link
Member

Choose a reason for hiding this comment

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

is this really what we use for gengo to parse the types.go files?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. the imported package was go/parser

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.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not super clear on what this means, what's the exact behavior that you think we can have? Maybe include the bug too where a lot of the discussion has been going on.

Copy link
Member

Choose a reason for hiding this comment

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

Or can we try to be less hand-wavy on what we'll do here? I'm not sure I understand what you're proposing

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 agree. Externalizing this to an issue in kubernetes/gengo

Copy link
Member

Choose a reason for hiding this comment

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

I'd still be happy if we could discuss here what we can expect as a solution.

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 just realized that this issue should affect other markers too. I will keep it short in the KEP and describe it in a separate issue


### Risks and Mitigations

<!--
Expand Down Expand Up @@ -522,12 +532,6 @@ Pick one of these and delete the rest.
- [X] Feature gate (also fill in values in `kep.yaml`)
- Feature gate name: `OpenAPIEnum`
- Components depending on the feature gate: `kube-apiserver`
- [ ] Other
- Describe the mechanism:
- Will enabling / disabling the feature require downtime of the control
plane?
- Will enabling / disabling the feature require downtime or reprovisioning
of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled).

###### Does enabling the feature change any default behavior?

Expand Down Expand Up @@ -580,13 +584,18 @@ feature flags will be enabled on some API servers and not others during the
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 handle the request
which could result in inconsistent responses for the same request.

###### What specific metrics should inform a rollback?

<!--
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?

Expand All @@ -596,12 +605,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.?

<!--
Even if applying deprecation policies, they may still surprise some users.
-->

No.

### Monitoring Requirements

<!--
Expand All @@ -615,7 +627,7 @@ Ideally, this should be a metric. Operations against the Kubernetes API (e.g.,
checking if there are objects with field X set) may be a last resort. Avoid
logs or events for this purpose.
-->

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?

<!--
Expand All @@ -627,13 +639,8 @@ and operation of this feature.
Recall that end users cannot usually observe component logs or access metrics.
-->

- [ ] 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?

Expand All @@ -651,19 +658,17 @@ 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?

<!--
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?

Expand Down Expand Up @@ -695,6 +700,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-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.

### Scalability

<!--
Expand Down Expand Up @@ -784,6 +794,7 @@ details). For now, we leave it here.
-->

###### 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?

Expand All @@ -799,9 +810,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

<!--
Expand All @@ -815,6 +829,11 @@ Major milestones might include:
- when the KEP was retired or superseded
-->

- 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

<!--
Expand Down