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.

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

OCPBUGS-50587: Reject new NodeStatus with non-zero revision set #2208

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

OCPBUGS-50587: Reject new NodeStatus with non-zero revision set #2208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46fc4d5
operators/v1: reject new node statuses with current or target revisio…
everettraven Feb 20, 2025
ce61d0a
fixup! don't allow setting revisions
everettraven Feb 20, 2025
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: 3 additions & 4 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.

10 changes: 4 additions & 6 deletions openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30736,15 +30736,13 @@
"description": "NodeStatus provides information about the current state of a particular node managed by this operator.",
"type": "object",
"required": [
"nodeName",
"currentRevision"
"nodeName"
],
"properties": {
"currentRevision": {
"description": "currentRevision is the generation of the most recently successful deployment",
"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.",
"type": "integer",
"format": "int32",
"default": 0
"format": "int32"
},
"lastFailedCount": {
"description": "lastFailedCount is how often the installer pod of the last failed revision failed.",
Expand Down Expand Up @@ -30784,7 +30782,7 @@
"default": ""
},
"targetRevision": {
"description": "targetRevision is the generation of the deployment we're trying to apply",
"description": "targetRevision is the generation of the deployment we're trying to apply. Can not be set on creation of a nodeStatus.",
"type": "integer",
"format": "int32"
}
Expand Down
121 changes: 120 additions & 1 deletion operator/v1/tests/kubeapiservers.operator.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ tests:
operatorLogLevel: Normal
onUpdate:
- name: Should reject multiple nodes with nonzero target revisions
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/status/properties/nodeStatuses/items/x-kubernetes-validations/2
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
targetRevision: 1
targetRevision: 0
- nodeName: b
targetRevision: 0
- nodeName: c
Expand All @@ -43,6 +46,9 @@ tests:
- nodeName: d
expectedStatusError: "status.nodeStatuses: Invalid value: \"array\": no more than 1 node status may have a nonzero targetRevision"
- name: Should reject decreasing currentRevision
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/status/properties/nodeStatuses/items/x-kubernetes-validations/1
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
Expand All @@ -61,6 +67,9 @@ tests:
currentRevision: 2
expectedStatusError: "status.nodeStatuses[0].currentRevision: Invalid value: \"integer\": must only increase"
- name: Should reject clearing currentRevision
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/status/properties/nodeStatuses/items/x-kubernetes-validations/1
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
Expand All @@ -77,3 +86,113 @@ tests:
nodeStatuses:
- nodeName: a
expectedStatusError: "status.nodeStatuses[0].currentRevision: Invalid value: \"object\": cannot be unset once set"
- name: Should reject setting new nodeStatus with a currentRevision
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses: []
updated: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
currentRevision: 0
expectedStatusError: "status.nodeStatuses[0]: Invalid value: \"object\": currentRevision can not be set on creation of a nodeStatus"
- name: Should reject setting new nodeStatus with a targetRevision
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses: []
updated: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
targetRevision: 0
expectedStatusError: "status.nodeStatuses[0]: Invalid value: \"object\": targetRevision can not be set on creation of a nodeStatus"
- name: Should allow adding nodes with name only
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
updated: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
- nodeName: b
expected: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec:
logLevel: Normal
operatorLogLevel: Normal
status:
nodeStatuses:
- nodeName: a
- nodeName: b
- name: Should allow updating currentRevision of existing nodeStatus
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
updated: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
currentRevision: 1
expected: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec:
logLevel: Normal
operatorLogLevel: Normal
status:
nodeStatuses:
- nodeName: a
currentRevision: 1
- name: Should allow updating targetRevision of existing nodeStatus
initial: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
updated: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec: {} # No spec is required for a KubeAPIServer
status:
nodeStatuses:
- nodeName: a
targetRevision: 1
expected: |
apiVersion: operator.openshift.io/v1
kind: KubeAPIServer
spec:
logLevel: Normal
operatorLogLevel: Normal
status:
nodeStatuses:
- nodeName: a
targetRevision: 1
12 changes: 9 additions & 3 deletions operator/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,21 @@ type StaticPodOperatorStatus struct {

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

// currentRevision is the generation of the most recently successful deployment
// 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.
// +kubebuilder:validation:XValidation:rule="self >= oldSelf",message="must only increase"
CurrentRevision int32 `json:"currentRevision"`
// targetRevision is the generation of the deployment we're trying to apply
// +optional
CurrentRevision int32 `json:"currentRevision,omitempty"`
// targetRevision is the generation of the deployment we're trying to apply.
// Can not be set on creation of a nodeStatus.
// +optional
TargetRevision int32 `json:"targetRevision,omitempty"`

// lastFailedRevision is the generation of the deployment we tried and failed to deploy.
Expand Down
16 changes: 12 additions & 4 deletions operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,9 @@ spec:
of a particular node managed by this operator.
properties:
currentRevision:
description: currentRevision is the generation of the most recently
successful deployment
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.
format: int32
type: integer
x-kubernetes-validations:
Expand Down Expand Up @@ -292,8 +293,9 @@ spec:
description: nodeName is the name of the node
type: string
targetRevision:
description: targetRevision is the generation of the deployment
we're trying to apply
description: |-
targetRevision is the generation of the deployment we're trying to apply.
Can not be set on creation of a nodeStatus.
format: int32
type: integer
required:
Expand All @@ -303,6 +305,12 @@ spec:
- fieldPath: .currentRevision
message: cannot be unset once set
rule: has(self.currentRevision) || !has(oldSelf.currentRevision)
- message: currentRevision can not be set on creation of a nodeStatus
optionalOldSelf: true
rule: oldSelf.hasValue() || !has(self.currentRevision)
- message: targetRevision can not be set on creation of a nodeStatus
optionalOldSelf: true
rule: oldSelf.hasValue() || !has(self.targetRevision)
type: array
x-kubernetes-list-map-keys:
- nodeName
Expand Down
16 changes: 12 additions & 4 deletions operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-Default.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,9 @@ spec:
of a particular node managed by this operator.
properties:
currentRevision:
description: currentRevision is the generation of the most recently
successful deployment
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.
format: int32
type: integer
x-kubernetes-validations:
Expand Down Expand Up @@ -279,8 +280,9 @@ spec:
description: nodeName is the name of the node
type: string
targetRevision:
description: targetRevision is the generation of the deployment
we're trying to apply
description: |-
targetRevision is the generation of the deployment we're trying to apply.
Can not be set on creation of a nodeStatus.
format: int32
type: integer
required:
Expand All @@ -290,6 +292,12 @@ spec:
- fieldPath: .currentRevision
message: cannot be unset once set
rule: has(self.currentRevision) || !has(oldSelf.currentRevision)
- message: currentRevision can not be set on creation of a nodeStatus
optionalOldSelf: true
rule: oldSelf.hasValue() || !has(self.currentRevision)
- message: targetRevision can not be set on creation of a nodeStatus
optionalOldSelf: true
rule: oldSelf.hasValue() || !has(self.targetRevision)
type: array
x-kubernetes-list-map-keys:
- nodeName
Expand Down
16 changes: 12 additions & 4 deletions operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,9 @@ spec:
of a particular node managed by this operator.
properties:
currentRevision:
description: currentRevision is the generation of the most recently
successful deployment
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.
format: int32
type: integer
x-kubernetes-validations:
Expand Down Expand Up @@ -292,8 +293,9 @@ spec:
description: nodeName is the name of the node
type: string
targetRevision:
description: targetRevision is the generation of the deployment
we're trying to apply
description: |-
targetRevision is the generation of the deployment we're trying to apply.
Can not be set on creation of a nodeStatus.
format: int32
type: integer
required:
Expand All @@ -303,6 +305,12 @@ spec:
- fieldPath: .currentRevision
message: cannot be unset once set
rule: has(self.currentRevision) || !has(oldSelf.currentRevision)
- message: currentRevision can not be set on creation of a nodeStatus
optionalOldSelf: true
rule: oldSelf.hasValue() || !has(self.currentRevision)
- message: targetRevision can not be set on creation of a nodeStatus
optionalOldSelf: true
rule: oldSelf.hasValue() || !has(self.targetRevision)
type: array
x-kubernetes-list-map-keys:
- nodeName
Expand Down
16 changes: 12 additions & 4 deletions operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-TechPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,9 @@ spec:
of a particular node managed by this operator.
properties:
currentRevision:
description: currentRevision is the generation of the most recently
successful deployment
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.
format: int32
type: integer
x-kubernetes-validations:
Expand Down Expand Up @@ -292,8 +293,9 @@ spec:
description: nodeName is the name of the node
type: string
targetRevision:
description: targetRevision is the generation of the deployment
we're trying to apply
description: |-
targetRevision is the generation of the deployment we're trying to apply.
Can not be set on creation of a nodeStatus.
format: int32
type: integer
required:
Expand All @@ -303,6 +305,12 @@ spec:
- fieldPath: .currentRevision
message: cannot be unset once set
rule: has(self.currentRevision) || !has(oldSelf.currentRevision)
- message: currentRevision can not be set on creation of a nodeStatus
optionalOldSelf: true
rule: oldSelf.hasValue() || !has(self.currentRevision)
- message: targetRevision can not be set on creation of a nodeStatus
optionalOldSelf: true
rule: oldSelf.hasValue() || !has(self.targetRevision)
type: array
x-kubernetes-list-map-keys:
- nodeName
Expand Down
Loading