Skip to content

Commit 6b76c18

Browse files
everettraveneverettraven
committed
operators/v1: reject new node statuses with current or target revision set
Entries of static pod operators' node statuses are co-managed: - A node controller, responsible for ensuring a node status entry for per control plane node - An installer controller, responsible for managing operand revisions for each entry New node status entries with a non-zero revision populated signals that a single client is managing both aspects and is not intentional. This validation should serve as a pragmatic backstop against multi-writer field errors. Signed-off-by: Bryce Palmer <[email protected]>
1 parent da6bf56 commit 6b76c18

25 files changed

+418
-96
lines changed

‎openapi/generated_openapi/zz_generated.openapi.go

Lines changed: 3 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎openapi/openapi.json

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30736,15 +30736,13 @@
3073630736
"description": "NodeStatus provides information about the current state of a particular node managed by this operator.",
3073730737
"type": "object",
3073830738
"required": [
30739-
"nodeName",
30740-
"currentRevision"
30739+
"nodeName"
3074130740
],
3074230741
"properties": {
3074330742
"currentRevision": {
30744-
"description": "currentRevision is the generation of the most recently successful deployment",
30743+
"description": "currentRevision is the generation of the most recently successful deployment. Can not be set on creation of a nodeStatus. Updates must only increase the value.",
3074530744
"type": "integer",
30746-
"format": "int32",
30747-
"default": 0
30745+
"format": "int32"
3074830746
},
3074930747
"lastFailedCount": {
3075030748
"description": "lastFailedCount is how often the installer pod of the last failed revision failed.",
@@ -30784,7 +30782,7 @@
3078430782
"default": ""
3078530783
},
3078630784
"targetRevision": {
30787-
"description": "targetRevision is the generation of the deployment we're trying to apply",
30785+
"description": "targetRevision is the generation of the deployment we're trying to apply. Can not be set on creation of a nodeStatus.",
3078830786
"type": "integer",
3078930787
"format": "int32"
3079030788
}

‎operator/v1/tests/kubeapiservers.operator.openshift.io/AAA_ungated.yaml

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,17 @@ tests:
1616
operatorLogLevel: Normal
1717
onUpdate:
1818
- name: Should reject multiple nodes with nonzero target revisions
19+
initialCRDPatches:
20+
- op: remove
21+
path: /spec/versions/0/schema/openAPIV3Schema/properties/status/properties/nodeStatuses/items/x-kubernetes-validations/2
1922
initial: |
2023
apiVersion: operator.openshift.io/v1
2124
kind: KubeAPIServer
2225
spec: {} # No spec is required for a KubeAPIServer
2326
status:
2427
nodeStatuses:
2528
- nodeName: a
26-
targetRevision: 1
29+
targetRevision: 0
2730
- nodeName: b
2831
targetRevision: 0
2932
- nodeName: c
@@ -43,6 +46,9 @@ tests:
4346
- nodeName: d
4447
expectedStatusError: "status.nodeStatuses: Invalid value: \"array\": no more than 1 node status may have a nonzero targetRevision"
4548
- name: Should reject decreasing currentRevision
49+
initialCRDPatches:
50+
- op: remove
51+
path: /spec/versions/0/schema/openAPIV3Schema/properties/status/properties/nodeStatuses/items/x-kubernetes-validations/1
4652
initial: |
4753
apiVersion: operator.openshift.io/v1
4854
kind: KubeAPIServer
@@ -61,6 +67,9 @@ tests:
6167
currentRevision: 2
6268
expectedStatusError: "status.nodeStatuses[0].currentRevision: Invalid value: \"integer\": must only increase"
6369
- name: Should reject clearing currentRevision
70+
initialCRDPatches:
71+
- op: remove
72+
path: /spec/versions/0/schema/openAPIV3Schema/properties/status/properties/nodeStatuses/items/x-kubernetes-validations/1
6473
initial: |
6574
apiVersion: operator.openshift.io/v1
6675
kind: KubeAPIServer
@@ -77,3 +86,113 @@ tests:
7786
nodeStatuses:
7887
- nodeName: a
7988
expectedStatusError: "status.nodeStatuses[0].currentRevision: Invalid value: \"object\": cannot be unset once set"
89+
- name: Should reject setting new nodeStatus with a currentRevision
90+
initial: |
91+
apiVersion: operator.openshift.io/v1
92+
kind: KubeAPIServer
93+
spec: {} # No spec is required for a KubeAPIServer
94+
status:
95+
nodeStatuses: []
96+
updated: |
97+
apiVersion: operator.openshift.io/v1
98+
kind: KubeAPIServer
99+
spec: {} # No spec is required for a KubeAPIServer
100+
status:
101+
nodeStatuses:
102+
- nodeName: a
103+
currentRevision: 0
104+
expectedStatusError: "status.nodeStatuses[0].currentRevision: Invalid value: \"object\": currentRevision can not be set on creation of a nodeStatus"
105+
- name: Should reject setting new nodeStatus with a targetRevision
106+
initial: |
107+
apiVersion: operator.openshift.io/v1
108+
kind: KubeAPIServer
109+
spec: {} # No spec is required for a KubeAPIServer
110+
status:
111+
nodeStatuses: []
112+
updated: |
113+
apiVersion: operator.openshift.io/v1
114+
kind: KubeAPIServer
115+
spec: {} # No spec is required for a KubeAPIServer
116+
status:
117+
nodeStatuses:
118+
- nodeName: a
119+
targetRevision: 0
120+
expectedStatusError: "status.nodeStatuses[0].targetRevision: Invalid value: \"object\": targetRevision can not be set on creation of a nodeStatus"
121+
- name: Should allow adding nodes with name only
122+
initial: |
123+
apiVersion: operator.openshift.io/v1
124+
kind: KubeAPIServer
125+
spec: {} # No spec is required for a KubeAPIServer
126+
status:
127+
nodeStatuses:
128+
- nodeName: a
129+
updated: |
130+
apiVersion: operator.openshift.io/v1
131+
kind: KubeAPIServer
132+
spec: {} # No spec is required for a KubeAPIServer
133+
status:
134+
nodeStatuses:
135+
- nodeName: a
136+
- nodeName: b
137+
expected: |
138+
apiVersion: operator.openshift.io/v1
139+
kind: KubeAPIServer
140+
spec:
141+
logLevel: Normal
142+
operatorLogLevel: Normal
143+
status:
144+
nodeStatuses:
145+
- nodeName: a
146+
- nodeName: b
147+
- name: Should allow updating currentRevision of existing nodeStatus
148+
initial: |
149+
apiVersion: operator.openshift.io/v1
150+
kind: KubeAPIServer
151+
spec: {} # No spec is required for a KubeAPIServer
152+
status:
153+
nodeStatuses:
154+
- nodeName: a
155+
updated: |
156+
apiVersion: operator.openshift.io/v1
157+
kind: KubeAPIServer
158+
spec: {} # No spec is required for a KubeAPIServer
159+
status:
160+
nodeStatuses:
161+
- nodeName: a
162+
currentRevision: 1
163+
expected: |
164+
apiVersion: operator.openshift.io/v1
165+
kind: KubeAPIServer
166+
spec:
167+
logLevel: Normal
168+
operatorLogLevel: Normal
169+
status:
170+
nodeStatuses:
171+
- nodeName: a
172+
currentRevision: 1
173+
- name: Should allow updating targetRevision of existing nodeStatus
174+
initial: |
175+
apiVersion: operator.openshift.io/v1
176+
kind: KubeAPIServer
177+
spec: {} # No spec is required for a KubeAPIServer
178+
status:
179+
nodeStatuses:
180+
- nodeName: a
181+
updated: |
182+
apiVersion: operator.openshift.io/v1
183+
kind: KubeAPIServer
184+
spec: {} # No spec is required for a KubeAPIServer
185+
status:
186+
nodeStatuses:
187+
- nodeName: a
188+
targetRevision: 1
189+
expected: |
190+
apiVersion: operator.openshift.io/v1
191+
kind: KubeAPIServer
192+
spec:
193+
logLevel: Normal
194+
operatorLogLevel: Normal
195+
status:
196+
nodeStatuses:
197+
- nodeName: a
198+
targetRevision: 1

‎operator/v1/types.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,15 +258,21 @@ type StaticPodOperatorStatus struct {
258258

259259
// NodeStatus provides information about the current state of a particular node managed by this operator.
260260
// +kubebuilder:validation:XValidation:rule="has(self.currentRevision) || !has(oldSelf.currentRevision)",message="cannot be unset once set",fieldPath=".currentRevision"
261+
// +kubebuilder:validation:XValidation:rule="oldSelf.hasValue() || !has(self.currentRevision)",message="currentRevision can not be set on creation of a nodeStatus",optionalOldSelf=true,fieldPath=.currentRevision
262+
// +kubebuilder:validation:XValidation:rule="oldSelf.hasValue() || !has(self.targetRevision)",message="targetRevision can not be set on creation of a nodeStatus",optionalOldSelf=true,fieldPath=.targetRevision
261263
type NodeStatus struct {
262264
// nodeName is the name of the node
263265
// +required
264266
NodeName string `json:"nodeName"`
265267

266-
// currentRevision is the generation of the most recently successful deployment
268+
// currentRevision is the generation of the most recently successful deployment.
269+
// Can not be set on creation of a nodeStatus. Updates must only increase the value.
267270
// +kubebuilder:validation:XValidation:rule="self >= oldSelf",message="must only increase"
268-
CurrentRevision int32 `json:"currentRevision"`
269-
// targetRevision is the generation of the deployment we're trying to apply
271+
// +optional
272+
CurrentRevision int32 `json:"currentRevision,omitempty"`
273+
// targetRevision is the generation of the deployment we're trying to apply.
274+
// Can not be set on creation of a nodeStatus.
275+
// +optional
270276
TargetRevision int32 `json:"targetRevision,omitempty"`
271277

272278
// lastFailedRevision is the generation of the deployment we tried and failed to deploy.

‎operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-CustomNoUpgrade.crd.yaml

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,9 @@ spec:
252252
of a particular node managed by this operator.
253253
properties:
254254
currentRevision:
255-
description: currentRevision is the generation of the most recently
256-
successful deployment
255+
description: |-
256+
currentRevision is the generation of the most recently successful deployment.
257+
Can not be set on creation of a nodeStatus. Updates must only increase the value.
257258
format: int32
258259
type: integer
259260
x-kubernetes-validations:
@@ -292,8 +293,9 @@ spec:
292293
description: nodeName is the name of the node
293294
type: string
294295
targetRevision:
295-
description: targetRevision is the generation of the deployment
296-
we're trying to apply
296+
description: |-
297+
targetRevision is the generation of the deployment we're trying to apply.
298+
Can not be set on creation of a nodeStatus.
297299
format: int32
298300
type: integer
299301
required:
@@ -303,6 +305,14 @@ spec:
303305
- fieldPath: .currentRevision
304306
message: cannot be unset once set
305307
rule: has(self.currentRevision) || !has(oldSelf.currentRevision)
308+
- fieldPath: .currentRevision
309+
message: currentRevision can not be set on creation of a nodeStatus
310+
optionalOldSelf: true
311+
rule: oldSelf.hasValue() || !has(self.currentRevision)
312+
- fieldPath: .targetRevision
313+
message: targetRevision can not be set on creation of a nodeStatus
314+
optionalOldSelf: true
315+
rule: oldSelf.hasValue() || !has(self.targetRevision)
306316
type: array
307317
x-kubernetes-list-map-keys:
308318
- nodeName

‎operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-Default.crd.yaml

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,9 @@ spec:
239239
of a particular node managed by this operator.
240240
properties:
241241
currentRevision:
242-
description: currentRevision is the generation of the most recently
243-
successful deployment
242+
description: |-
243+
currentRevision is the generation of the most recently successful deployment.
244+
Can not be set on creation of a nodeStatus. Updates must only increase the value.
244245
format: int32
245246
type: integer
246247
x-kubernetes-validations:
@@ -279,8 +280,9 @@ spec:
279280
description: nodeName is the name of the node
280281
type: string
281282
targetRevision:
282-
description: targetRevision is the generation of the deployment
283-
we're trying to apply
283+
description: |-
284+
targetRevision is the generation of the deployment we're trying to apply.
285+
Can not be set on creation of a nodeStatus.
284286
format: int32
285287
type: integer
286288
required:
@@ -290,6 +292,14 @@ spec:
290292
- fieldPath: .currentRevision
291293
message: cannot be unset once set
292294
rule: has(self.currentRevision) || !has(oldSelf.currentRevision)
295+
- fieldPath: .currentRevision
296+
message: currentRevision can not be set on creation of a nodeStatus
297+
optionalOldSelf: true
298+
rule: oldSelf.hasValue() || !has(self.currentRevision)
299+
- fieldPath: .targetRevision
300+
message: targetRevision can not be set on creation of a nodeStatus
301+
optionalOldSelf: true
302+
rule: oldSelf.hasValue() || !has(self.targetRevision)
293303
type: array
294304
x-kubernetes-list-map-keys:
295305
- nodeName

‎operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-DevPreviewNoUpgrade.crd.yaml

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,9 @@ spec:
252252
of a particular node managed by this operator.
253253
properties:
254254
currentRevision:
255-
description: currentRevision is the generation of the most recently
256-
successful deployment
255+
description: |-
256+
currentRevision is the generation of the most recently successful deployment.
257+
Can not be set on creation of a nodeStatus. Updates must only increase the value.
257258
format: int32
258259
type: integer
259260
x-kubernetes-validations:
@@ -292,8 +293,9 @@ spec:
292293
description: nodeName is the name of the node
293294
type: string
294295
targetRevision:
295-
description: targetRevision is the generation of the deployment
296-
we're trying to apply
296+
description: |-
297+
targetRevision is the generation of the deployment we're trying to apply.
298+
Can not be set on creation of a nodeStatus.
297299
format: int32
298300
type: integer
299301
required:
@@ -303,6 +305,14 @@ spec:
303305
- fieldPath: .currentRevision
304306
message: cannot be unset once set
305307
rule: has(self.currentRevision) || !has(oldSelf.currentRevision)
308+
- fieldPath: .currentRevision
309+
message: currentRevision can not be set on creation of a nodeStatus
310+
optionalOldSelf: true
311+
rule: oldSelf.hasValue() || !has(self.currentRevision)
312+
- fieldPath: .targetRevision
313+
message: targetRevision can not be set on creation of a nodeStatus
314+
optionalOldSelf: true
315+
rule: oldSelf.hasValue() || !has(self.targetRevision)
306316
type: array
307317
x-kubernetes-list-map-keys:
308318
- nodeName

‎operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-TechPreviewNoUpgrade.crd.yaml

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,9 @@ spec:
252252
of a particular node managed by this operator.
253253
properties:
254254
currentRevision:
255-
description: currentRevision is the generation of the most recently
256-
successful deployment
255+
description: |-
256+
currentRevision is the generation of the most recently successful deployment.
257+
Can not be set on creation of a nodeStatus. Updates must only increase the value.
257258
format: int32
258259
type: integer
259260
x-kubernetes-validations:
@@ -292,8 +293,9 @@ spec:
292293
description: nodeName is the name of the node
293294
type: string
294295
targetRevision:
295-
description: targetRevision is the generation of the deployment
296-
we're trying to apply
296+
description: |-
297+
targetRevision is the generation of the deployment we're trying to apply.
298+
Can not be set on creation of a nodeStatus.
297299
format: int32
298300
type: integer
299301
required:
@@ -303,6 +305,14 @@ spec:
303305
- fieldPath: .currentRevision
304306
message: cannot be unset once set
305307
rule: has(self.currentRevision) || !has(oldSelf.currentRevision)
308+
- fieldPath: .currentRevision
309+
message: currentRevision can not be set on creation of a nodeStatus
310+
optionalOldSelf: true
311+
rule: oldSelf.hasValue() || !has(self.currentRevision)
312+
- fieldPath: .targetRevision
313+
message: targetRevision can not be set on creation of a nodeStatus
314+
optionalOldSelf: true
315+
rule: oldSelf.hasValue() || !has(self.targetRevision)
306316
type: array
307317
x-kubernetes-list-map-keys:
308318
- nodeName

0 commit comments

Comments
 (0)