Skip to content

CNTRLPLANE-1576: add event-ttl configuration to kube-apiserver #2520

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

CNTRLPLANE-1576: add event-ttl configuration to kube-apiserver #2520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions openapi/generated_openapi/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30290,6 +30290,11 @@
"forceRedeploymentReason"
],
"properties": {
"eventTTLMinutes": {
"description": "eventTTLMinutes specifies the amount of time that the events are stored before being deleted. The TTL is allowed between 5 minutes minimum up to a maximum of 180 minutes (3 hours).\n\nLowering this value will reduce the storage required in etcd. Note that this setting will only apply to new events being created and will not update existing events.\n\nWhen omitted this means no opinion, and the platform is left to choose a reasonable default, which is subject to change over time. The current default value is 3h (180 minutes).",
"type": "integer",
"format": "int32"
},
"failedRevisionLimit": {
"description": "failedRevisionLimit is the number of failed static pod installer revisions to keep on disk and in the api -1 = unlimited, 0 or unset = 5 (default)",
"type": "integer",
Expand Down
47 changes: 47 additions & 0 deletions operator/v1/tests/kubeapiservers.operator.openshift.io/EventTTL.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "KubeAPIServer"
crdName: kubeapiservers.operator.openshift.io
tests:
onCreate:
- name: Should be able to create default omitted event ttl
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
expected: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec:
logLevel: Normal
operatorLogLevel: Normal
- name: Should be able to create a normal hour event ttl
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec:
eventTTLMinutes: 60
expected: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec:
logLevel: Normal
operatorLogLevel: Normal
eventTTLMinutes: 60
- name: Should not be able to create with less than 5 minutes
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
metadata:
name: gg1
spec:
eventTTLMinutes: 4
expectedError: "Invalid value: 4: spec.eventTTLMinutes in body should be greater than or equal to 5"
- name: Should not be able to create more than 360 minutes
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
metadata:
name: gg1
spec:
eventTTLMinutes: 181
expectedError: "Invalid value: 181: spec.eventTTLMinutes in body should be less than or equal to 180"
14 changes: 14 additions & 0 deletions operator/v1/types_kubeapiserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,20 @@ type KubeAPIServer struct {

type KubeAPIServerSpec struct {
StaticPodOperatorSpec `json:",inline"`

// eventTTLMinutes specifies the amount of time that the events are stored before being deleted.
// The TTL is allowed between 5 minutes minimum up to a maximum of 180 minutes (3 hours).
//
// Lowering this value will reduce the storage required in etcd. Note that this setting will only apply
// to new events being created and will not update existing events.
//
// When omitted this means no opinion, and the platform is left to choose a reasonable default, which is subject to change over time.
// The current default value is 3h (180 minutes).
//
// +kubebuilder:validation:Minimum=5
// +kubebuilder:validation:Maximum=180
// +optional
EventTTLMinutes int32 `json:"eventTTLMinutes,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

mhm, what happens if an admin sets that field, but there is a controller that is using an old generated client that doesn’t know about it and that controller updates the spec?

is this something we should/usually take into account?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a general issue with adding new fields to any API. In this case, depending on the type of request they make (update vs patch), they would likely clear the field and then the behaviour would be reverted, it's not a catastrophic change of the behaviour so isn't going to burn the cluster to the ground.

If you wanted to, you could make this field un-removable, but I don't think that's really the right use case for this field

Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity, how could one make a field unremovable?

Copy link
Contributor

Choose a reason for hiding this comment

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

You need a chain of "un-removability" from the nearest ancestor that is always required

// +kubebuilder:validation:XValidation:rule="!oldSelf.?child.hasValue() || self.?child.hasValue()",optionalOldSelf=true,message="child may not be removed once set"

This would go on the struct definition of each ancestor til you find the nearest always required ancestor

}

type KubeAPIServerStatus struct {
Expand Down
14 changes: 14 additions & 0 deletions operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ spec:
description: spec is the specification of the desired behavior of the
Kubernetes API Server
properties:
eventTTLMinutes:
description: |-
eventTTLMinutes specifies the amount of time that the events are stored before being deleted.
The TTL is allowed between 5 minutes minimum up to a maximum of 180 minutes (3 hours).

Lowering this value will reduce the storage required in etcd. Note that this setting will only apply
to new events being created and will not update existing events.

When omitted this means no opinion, and the platform is left to choose a reasonable default, which is subject to change over time.
The current default value is 3h (180 minutes).
format: int32
maximum: 180
minimum: 5
type: integer
failedRevisionLimit:
description: |-
failedRevisionLimit is the number of failed static pod installer revisions to keep on disk and in the api
Expand Down
14 changes: 14 additions & 0 deletions ...enerated.featuregated-crd-manifests/kubeapiservers.operator.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ spec:
description: spec is the specification of the desired behavior of the
Kubernetes API Server
properties:
eventTTLMinutes:
description: |-
eventTTLMinutes specifies the amount of time that the events are stored before being deleted.
The TTL is allowed between 5 minutes minimum up to a maximum of 180 minutes (3 hours).
Lowering this value will reduce the storage required in etcd. Note that this setting will only apply
to new events being created and will not update existing events.
When omitted this means no opinion, and the platform is left to choose a reasonable default, which is subject to change over time.
The current default value is 3h (180 minutes).
format: int32
maximum: 180
minimum: 5
type: integer
failedRevisionLimit:
description: |-
failedRevisionLimit is the number of failed static pod installer revisions to keep on disk and in the api
Expand Down
8 changes: 8 additions & 0 deletions operator/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions payload-manifests/crds/0000_20_kube-apiserver_01_kubeapiservers.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ spec:
description: spec is the specification of the desired behavior of the
Kubernetes API Server
properties:
eventTTLMinutes:
description: |-
eventTTLMinutes specifies the amount of time that the events are stored before being deleted.
The TTL is allowed between 5 minutes minimum up to a maximum of 180 minutes (3 hours).

Lowering this value will reduce the storage required in etcd. Note that this setting will only apply
to new events being created and will not update existing events.

When omitted this means no opinion, and the platform is left to choose a reasonable default, which is subject to change over time.
The current default value is 3h (180 minutes).
format: int32
maximum: 180
minimum: 5
type: integer
failedRevisionLimit:
description: |-
failedRevisionLimit is the number of failed static pod installer revisions to keep on disk and in the api
Expand Down