Skip to content

add .status.relatedObjects to clusterversion to get CVO references #2339

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 6 commits into
base: master
Choose a base branch
from
Open

add .status.relatedObjects to clusterversion to get CVO references #2339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1cd8717
add .status.relatedObjects to clusterversion to get CVO references
deads2k May 22, 2025
aaf5b96
squash to original
deads2k May 22, 2025
0ce563c
tests
deads2k May 28, 2025
7943458
tests
deads2k May 28, 2025
23e47a4
add default value for ObjectReference to allow its use as an SSA key
deads2k May 28, 2025
5933f49
test fix
deads2k May 30, 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
74 changes: 74 additions & 0 deletions config/v1/tests/clusterversions.config.openshift.io/CVORelatedObjects.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "ClusterVersion"
crdName: clusterversions.config.openshift.io
featureGates:
- CVORelatedObjects
tests:
onCreate:
- name: Should be able to create a minimal ClusterVersion
initial: |
apiVersion: config.openshift.io/v1
kind: ClusterVersion
spec:
clusterID: foo
expected: |
apiVersion: config.openshift.io/v1
kind: ClusterVersion
spec:
clusterID: foo
onUpdate:
- name: .status.relatedObjects namespace default should be empty
initial: |
apiVersion: config.openshift.io/v1
kind: ClusterVersion
spec:
clusterID: foo
status:
desired:
version: foo
image: foo
architecture: Multi
observedGeneration: 1
versionHash: foo
availableUpdates:
- version: foo
image: foo
updated: |
apiVersion: config.openshift.io/v1
kind: ClusterVersion
spec:
clusterID: foo
status:
desired:
version: foo
image: foo
architecture: Multi
observedGeneration: 1
versionHash: foo
availableUpdates:
- version: foo
image: foo
relatedObjects:
- group: g1
resource: r1
name: n1
expected: |
apiVersion: config.openshift.io/v1
kind: ClusterVersion
spec:
clusterID: foo
status:
desired:
version: foo
image: foo
architecture: Multi
observedGeneration: 1
versionHash: foo
availableUpdates:
- version: foo
image: foo
relatedObjects:
- group: g1
resource: r1
namespace: ns1
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be the empty string?

Suggested change
namespace: ns1
namespace: ""

name: n1
1 change: 1 addition & 0 deletions config/v1/types_cluster_operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ type ObjectReference struct {
// +required
Resource string `json:"resource"`
// namespace of the referent.
// +kubebuilder:default=""
// +optional
Namespace string `json:"namespace,omitempty"`
// name of the referent.
Expand Down
15 changes: 15 additions & 0 deletions config/v1/types_cluster_version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,21 @@ type ClusterVersionStatus struct {
// +listType=atomic
// +optional
ConditionalUpdates []ConditionalUpdate `json:"conditionalUpdates,omitempty"`

// relatedObjects is a list of objects that are "interesting" or related to this operator.
// `oc adm inspect` honors this field in any type to navigate and collect related data.
// Common uses are:
// 1. operator namespaces
// 2. operand namespaces
Copy link
Member

Choose a reason for hiding this comment

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

We obviously want to include the openshift-cluster-version namespace for our operator, but "operand" gets a bit sticky with the CVO. Do we do just our own namespace on the thin end, and build out CVO logging as needed to help debug things where we don't capture enough to debug a resource we're failing to reconcile? Or all ~1k resources release-manifests are asking us to manage on the thick end? Or pick some middle ground like "asking for all ClusterOperators is easy, and likely covers reporting on most of the resources we care about"? Or...?

// +optional
// +openshift:enable:FeatureGate=CVORelatedObjects
// +kubebuilder:validation:MaxItems=100
// +listType=map
// +listMapKey=group
// +listMapKey=resource
// +listMapKey=namespace
// +listMapKey=name
RelatedObjects []ObjectReference `json:"relatedObjects,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

+listType=map and appropriate listMapKey missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+listType=map and appropriate listMapKey missing

Will do. Consideration for our future, will we fix the existing usages too?

Copy link
Contributor

Choose a reason for hiding this comment

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

If they're in status, I would expect we are already doing the right thing and so should be fine to fix it

I also assume these would ratchet 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh very neat. I think you can only make map keys on required fields and not all the fields are logically required. I might go with atomic (single writer of this thing anyway)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can only make map keys on required fields and not all the fields are logically required

Yes I agree, the required ones are group, name and resource, so potential overlap on namespace?

If you don't have the listType enforcing the uniqueness, how about CEL?

Copy link
Contributor

Choose a reason for hiding this comment

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

For reference, the incantation is

+kubebuilder:validation:XValiadtion:rule="self.all(x, self.exists_one(y, x == y))",message="items in relatedObjects should be unique"

Copy link
Contributor

Choose a reason for hiding this comment

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

List should set a sensible maximum length, given intended use case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

List should set a sensible maximum length, given intended use case

I'm so glad I insisted on these linters :)

}

// UpdateState is a constant representing whether an update was successfully
Expand Down
1 change: 1 addition & 0 deletions .../zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusteroperators.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ spec:
description: name of the referent.
type: string
namespace:
default: ""
description: namespace of the referent.
type: string
resource:
Expand Down
37 changes: 37 additions & 0 deletions ...rd-manifests/0000_00_cluster-version-operator_01_clusterversions-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,43 @@ spec:
and conditions fields may represent a previous version.
format: int64
type: integer
relatedObjects:
description: |-
relatedObjects is a list of objects that are "interesting" or related to this operator.
`oc adm inspect` honors this field in any type to navigate and collect related data.
Common uses are:
1. operator namespaces
2. operand namespaces
items:
description: ObjectReference contains enough information to let
you inspect or modify the referred object.
properties:
group:
description: group of the referent.
type: string
name:
description: name of the referent.
type: string
namespace:
default: ""
description: namespace of the referent.
type: string
resource:
description: resource of the referent.
type: string
required:
- group
- name
- resource
type: object
maxItems: 100
type: array
x-kubernetes-list-map-keys:
- group
- resource
- namespace
- name
x-kubernetes-list-type: map
versionHash:
description: |-
versionHash is a fingerprint of the content that the cluster will be
Expand Down
37 changes: 37 additions & 0 deletions ...nifests/0000_00_cluster-version-operator_01_clusterversions-TechPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,43 @@ spec:
and conditions fields may represent a previous version.
format: int64
type: integer
relatedObjects:
description: |-
relatedObjects is a list of objects that are "interesting" or related to this operator.
`oc adm inspect` honors this field in any type to navigate and collect related data.
Common uses are:
1. operator namespaces
2. operand namespaces
items:
description: ObjectReference contains enough information to let
you inspect or modify the referred object.
properties:
group:
description: group of the referent.
type: string
name:
description: name of the referent.
type: string
namespace:
default: ""
description: namespace of the referent.
type: string
resource:
description: resource of the referent.
type: string
required:
- group
- name
- resource
type: object
maxItems: 100
type: array
x-kubernetes-list-map-keys:
- group
- resource
- namespace
- name
x-kubernetes-list-type: map
versionHash:
description: |-
versionHash is a fingerprint of the content that the cluster will be
Expand Down
1 change: 1 addition & 0 deletions config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ spec:
description: name of the referent.
type: string
namespace:
default: ""
description: namespace of the referent.
type: string
resource:
Expand Down
5 changes: 5 additions & 0 deletions config/v1/zz_generated.deepcopy.go
View file
Open in desktop

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

1 change: 1 addition & 0 deletions config/v1/zz_generated.featuregated-crd-manifests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ clusterversions.config.openshift.io:
Capability: ""
Category: ""
FeatureGates:
- CVORelatedObjects
- ImageStreamImportMode
- SignatureStores
FilenameOperatorName: cluster-version-operator
Expand Down
1 change: 1 addition & 0 deletions ...enerated.featuregated-crd-manifests/clusteroperators.config.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ spec:
description: name of the referent.
type: string
namespace:
default: ""
description: namespace of the referent.
type: string
resource:
Expand Down
Loading