diff --git a/Makefile b/Makefile index 5371d5b48d96..475e0ade018c 100644 --- a/Makefile +++ b/Makefile @@ -305,6 +305,10 @@ generate-manifests-core: $(CONTROLLER_GEN) $(KUSTOMIZE) ## Generate manifests e. crd:crdVersions=v1 \ output:crd:dir=./cmd/clusterctl/config/crd/bases $(KUSTOMIZE) build $(CLUSTERCTL_MANIFEST_DIR)/crd > $(CLUSTERCTL_MANIFEST_DIR)/manifest/clusterctl-api.yaml + $(CONTROLLER_GEN) \ + paths=./internal/test/builder/... \ + crd:crdVersions=v1 \ + output:crd:dir=./internal/test/builder/crd .PHONY: generate-manifests-kubeadm-bootstrap generate-manifests-kubeadm-bootstrap: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc. for kubeadm bootstrap diff --git a/internal/test/builder/crd/test.cluster.x-k8s.io_phase0obj.yaml b/internal/test/builder/crd/test.cluster.x-k8s.io_phase0obj.yaml new file mode 100644 index 000000000000..1be851749ef3 --- /dev/null +++ b/internal/test/builder/crd/test.cluster.x-k8s.io_phase0obj.yaml @@ -0,0 +1,103 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.1 + name: phase0obj.test.cluster.x-k8s.io +spec: + group: test.cluster.x-k8s.io + names: + categories: + - cluster-api + kind: Phase0Obj + listKind: Phase0ObjList + plural: phase0obj + singular: phase0obj + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: Phase0Obj defines an object with clusterv1.Conditions. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: Phase0ObjSpec defines the spec of a Phase0Obj. + properties: + foo: + type: string + type: object + status: + description: Phase0ObjStatus defines the status of a Phase0Obj. + properties: + bar: + type: string + conditions: + description: Conditions provide observations of the operational state + of a Cluster API resource. + items: + description: Condition defines an observation of a Cluster API resource + operational state. + properties: + lastTransitionTime: + description: |- + Last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when + the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + A human readable message indicating details about the transition. + This field may be empty. + type: string + reason: + description: |- + The reason for the condition's last transition in CamelCase. + The specific API may choose whether or not this field is considered a guaranteed API. + This field may be empty. + type: string + severity: + description: |- + Severity provides an explicit classification of Reason code, so the users or machines can immediately + understand the current situation and act accordingly. + The Severity field MUST be set only when Status=False. + type: string + status: + description: Status of the condition, one of True, False, Unknown. + type: string + type: + description: |- + Type of condition in CamelCase or in foo.example.com/CamelCase. + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions + can be useful (see .node.status.conditions), the ability to deconflict is important. + type: string + required: + - lastTransitionTime + - status + - type + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/internal/test/builder/crd/test.cluster.x-k8s.io_phase1obj.yaml b/internal/test/builder/crd/test.cluster.x-k8s.io_phase1obj.yaml new file mode 100644 index 000000000000..cc5f8a29dc77 --- /dev/null +++ b/internal/test/builder/crd/test.cluster.x-k8s.io_phase1obj.yaml @@ -0,0 +1,169 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.1 + name: phase1obj.test.cluster.x-k8s.io +spec: + group: test.cluster.x-k8s.io + names: + categories: + - cluster-api + kind: Phase1Obj + listKind: Phase1ObjList + plural: phase1obj + singular: phase1obj + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: Phase1Obj defines an object with conditions and experimental + conditions. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: Phase1ObjSpec defines the spec of a Phase1Obj. + properties: + foo: + type: string + type: object + status: + description: Phase1ObjStatus defines the status of a Phase1Obj. + properties: + bar: + type: string + conditions: + description: Conditions provide observations of the operational state + of a Cluster API resource. + items: + description: Condition defines an observation of a Cluster API resource + operational state. + properties: + lastTransitionTime: + description: |- + Last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when + the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + A human readable message indicating details about the transition. + This field may be empty. + type: string + reason: + description: |- + The reason for the condition's last transition in CamelCase. + The specific API may choose whether or not this field is considered a guaranteed API. + This field may be empty. + type: string + severity: + description: |- + Severity provides an explicit classification of Reason code, so the users or machines can immediately + understand the current situation and act accordingly. + The Severity field MUST be set only when Status=False. + type: string + status: + description: Status of the condition, one of True, False, Unknown. + type: string + type: + description: |- + Type of condition in CamelCase or in foo.example.com/CamelCase. + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions + can be useful (see .node.status.conditions), the ability to deconflict is important. + type: string + required: + - lastTransitionTime + - status + - type + type: object + type: array + v1beta2: + description: Phase1ObjStatusV1beta2 defines the status.V1Beta1 of + a Phase1Obj. + properties: + conditions: + items: + description: Condition contains details for one aspect of the + current state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/internal/test/builder/crd/test.cluster.x-k8s.io_phase2obj.yaml b/internal/test/builder/crd/test.cluster.x-k8s.io_phase2obj.yaml new file mode 100644 index 000000000000..d1e115c87a96 --- /dev/null +++ b/internal/test/builder/crd/test.cluster.x-k8s.io_phase2obj.yaml @@ -0,0 +1,174 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.1 + name: phase2obj.test.cluster.x-k8s.io +spec: + group: test.cluster.x-k8s.io + names: + categories: + - cluster-api + kind: Phase2Obj + listKind: Phase2ObjList + plural: phase2obj + singular: phase2obj + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: Phase2Obj defines an object with conditions and back compatibility + conditions. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: Phase2ObjSpec defines the spec of a Phase2Obj. + properties: + foo: + type: string + type: object + status: + description: Phase2ObjStatus defines the status of a Phase2Obj. + properties: + bar: + type: string + conditions: + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + deprecated: + description: Phase2ObjStatusDeprecated defines the status.Deprecated + of a Phase2Obj. + properties: + v1beta1: + description: Phase2ObjStatusDeprecatedV1Beta2 defines the status.Deprecated.V1Beta2 + of a Phase2Obj. + properties: + conditions: + description: Conditions provide observations of the operational + state of a Cluster API resource. + items: + description: Condition defines an observation of a Cluster + API resource operational state. + properties: + lastTransitionTime: + description: |- + Last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when + the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + A human readable message indicating details about the transition. + This field may be empty. + type: string + reason: + description: |- + The reason for the condition's last transition in CamelCase. + The specific API may choose whether or not this field is considered a guaranteed API. + This field may be empty. + type: string + severity: + description: |- + Severity provides an explicit classification of Reason code, so the users or machines can immediately + understand the current situation and act accordingly. + The Severity field MUST be set only when Status=False. + type: string + status: + description: Status of the condition, one of True, False, + Unknown. + type: string + type: + description: |- + Type of condition in CamelCase or in foo.example.com/CamelCase. + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions + can be useful (see .node.status.conditions), the ability to deconflict is important. + type: string + required: + - lastTransitionTime + - status + - type + type: object + type: array + type: object + type: object + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/internal/test/builder/crd/test.cluster.x-k8s.io_phase3obj.yaml b/internal/test/builder/crd/test.cluster.x-k8s.io_phase3obj.yaml new file mode 100644 index 000000000000..ee6029a64026 --- /dev/null +++ b/internal/test/builder/crd/test.cluster.x-k8s.io_phase3obj.yaml @@ -0,0 +1,116 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.1 + name: phase3obj.test.cluster.x-k8s.io +spec: + group: test.cluster.x-k8s.io + names: + categories: + - cluster-api + kind: Phase3Obj + listKind: Phase3ObjList + plural: phase3obj + singular: phase3obj + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: Phase3Obj defines an object with metav1.conditions. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: Phase3ObjSpec defines the spec of a Phase3Obj. + properties: + foo: + type: string + type: object + status: + description: Phase3ObjStatus defines the status of a Phase3Obj. + properties: + bar: + type: string + conditions: + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/internal/test/builder/doc.go b/internal/test/builder/doc.go index 6c346d1ef4da..0c6988cbd434 100644 --- a/internal/test/builder/doc.go +++ b/internal/test/builder/doc.go @@ -16,4 +16,6 @@ limitations under the License. // Package builder implements builder and CRDs for creating API objects for testing. // +kubebuilder:object:generate=true +// +groupName=test.cluster.x-k8s.io +// +versionName=v1alpha1 package builder diff --git a/internal/test/builder/v1beta2_transition.go b/internal/test/builder/v1beta2_transition.go index 8cc90b9e1f6d..26820b30c6e1 100644 --- a/internal/test/builder/v1beta2_transition.go +++ b/internal/test/builder/v1beta2_transition.go @@ -17,7 +17,6 @@ limitations under the License. package builder import ( - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -25,29 +24,12 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) -var ( - // TransitionV1beta2GroupVersion is group version used for test CRDs used for validating the v1beta2 transition. - TransitionV1beta2GroupVersion = schema.GroupVersion{Group: "transition.v1beta2.cluster.x-k8s.io", Version: "v1beta1"} - - // Phase0ObjKind is the kind for the Phase0Obj type. - Phase0ObjKind = "Phase0Obj" - // Phase0ObjCRD is a Phase0Obj CRD. - Phase0ObjCRD = phase0ObjCRD(TransitionV1beta2GroupVersion.WithKind(Phase0ObjKind)) - - // Phase1ObjKind is the kind for the Phase1Obj type. - Phase1ObjKind = "Phase1Obj" - // Phase1ObjCRD is a Phase1Obj CRD. - Phase1ObjCRD = phase1ObjCRD(TransitionV1beta2GroupVersion.WithKind(Phase1ObjKind)) - - // Phase2ObjKind is the kind for the Phase2Obj type. - Phase2ObjKind = "Phase2Obj" - // Phase2ObjCRD is a Phase2Obj CRD. - Phase2ObjCRD = phase2ObjCRD(TransitionV1beta2GroupVersion.WithKind(Phase2ObjKind)) +// This files provide types to validate transition from clusterv1.Conditions in v1Beta1 API to the metav1.Conditions in the v1Beta2 API. +// Please refer to util/conditions/v1beta2/doc.go for more context. - // Phase3ObjKind is the kind for the Phase3Obj type. - Phase3ObjKind = "Phase3Obj" - // Phase3ObjCRD is a Phase3Obj CRD. - Phase3ObjCRD = phase3ObjCRD(TransitionV1beta2GroupVersion.WithKind(Phase3ObjKind)) +var ( + // TestGroupVersion is group version used for test CRDs used for validating the v1beta2 transition. + TestGroupVersion = schema.GroupVersion{Group: "test.cluster.x-k8s.io", Version: "v1alpha1"} // schemeBuilder is used to add go types to the GroupVersionKind scheme. schemeBuilder = runtime.NewSchemeBuilder(addTransitionV1beta2Types) @@ -57,13 +39,13 @@ var ( ) func addTransitionV1beta2Types(scheme *runtime.Scheme) error { - scheme.AddKnownTypes(TransitionV1beta2GroupVersion, + scheme.AddKnownTypes(TestGroupVersion, &Phase0Obj{}, &Phase0ObjList{}, &Phase1Obj{}, &Phase1ObjList{}, &Phase2Obj{}, &Phase2ObjList{}, &Phase3Obj{}, &Phase3ObjList{}, ) - metav1.AddToGroupVersion(scheme, TransitionV1beta2GroupVersion) + metav1.AddToGroupVersion(scheme, TestGroupVersion) return nil } @@ -77,6 +59,9 @@ type Phase0ObjList struct { // Phase0Obj defines an object with clusterv1.Conditions. // +kubebuilder:object:root=true +// +kubebuilder:resource:path=phase0obj,scope=Namespaced,categories=cluster-api +// +kubebuilder:subresource:status +// +kubebuilder:storageversion type Phase0Obj struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -91,7 +76,9 @@ type Phase0ObjSpec struct { // Phase0ObjStatus defines the status of a Phase0Obj. type Phase0ObjStatus struct { - Bar string `json:"bar,omitempty"` + Bar string `json:"bar,omitempty"` + + // +optional Conditions clusterv1.Conditions `json:"conditions,omitempty"` } @@ -105,45 +92,6 @@ func (o *Phase0Obj) SetConditions(conditions clusterv1.Conditions) { o.Status.Conditions = conditions } -func phase0ObjCRD(gvk schema.GroupVersionKind) *apiextensionsv1.CustomResourceDefinition { - return generateCRD(gvk, map[string]apiextensionsv1.JSONSchemaProps{ - "metadata": { - // NOTE: in CRD there is only a partial definition of metadata schema. - // Ref https://github.com/kubernetes-sigs/controller-tools/blob/59485af1c1f6a664655dad49543c474bb4a0d2a2/pkg/crd/gen.go#L185 - Type: "object", - }, - "spec": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "foo": {Type: "string"}, - }, - }, - "status": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "bar": {Type: "string"}, - "conditions": { - Type: "array", - Items: &apiextensionsv1.JSONSchemaPropsOrArray{ - Schema: &apiextensionsv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "type": {Type: "string"}, - "status": {Type: "string"}, - "severity": {Type: "string"}, - "reason": {Type: "string"}, - "message": {Type: "string"}, - "lastTransitionTime": {Type: "string"}, - }, - Required: []string{"type", "status"}, - }, - }, - }, - }, - }, - }) -} - // Phase1ObjList is a list of Phase1Obj. // +kubebuilder:object:root=true type Phase1ObjList struct { @@ -154,6 +102,9 @@ type Phase1ObjList struct { // Phase1Obj defines an object with conditions and experimental conditions. // +kubebuilder:object:root=true +// +kubebuilder:resource:path=phase1obj,scope=Namespaced,categories=cluster-api +// +kubebuilder:subresource:status +// +kubebuilder:storageversion type Phase1Obj struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -168,9 +119,22 @@ type Phase1ObjSpec struct { // Phase1ObjStatus defines the status of a Phase1Obj. type Phase1ObjStatus struct { - Bar string `json:"bar,omitempty"` - Conditions clusterv1.Conditions `json:"conditions,omitempty"` - ExperimentalConditions []metav1.Condition `json:"experimentalConditions,omitempty"` + Bar string `json:"bar,omitempty"` + + // +optional + Conditions clusterv1.Conditions `json:"conditions,omitempty"` + + // +optional + V1Beta2 Phase1ObjStatusV1beta2 `json:"v1beta2,omitempty"` +} + +// Phase1ObjStatusV1beta2 defines the status.V1Beta1 of a Phase1Obj. +type Phase1ObjStatusV1beta2 struct { + + // +optional + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty"` } // GetConditions returns the set of conditions for this object. @@ -183,62 +147,6 @@ func (o *Phase1Obj) SetConditions(conditions clusterv1.Conditions) { o.Status.Conditions = conditions } -func phase1ObjCRD(gvk schema.GroupVersionKind) *apiextensionsv1.CustomResourceDefinition { - return generateCRD(gvk, map[string]apiextensionsv1.JSONSchemaProps{ - "metadata": { - // NOTE: in CRD there is only a partial definition of metadata schema. - // Ref https://github.com/kubernetes-sigs/controller-tools/blob/59485af1c1f6a664655dad49543c474bb4a0d2a2/pkg/crd/gen.go#L185 - Type: "object", - }, - "spec": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "foo": {Type: "string"}, - }, - }, - "status": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "bar": {Type: "string"}, - "conditions": { - Type: "array", - Items: &apiextensionsv1.JSONSchemaPropsOrArray{ - Schema: &apiextensionsv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "type": {Type: "string"}, - "status": {Type: "string"}, - "severity": {Type: "string"}, - "reason": {Type: "string"}, - "message": {Type: "string"}, - "lastTransitionTime": {Type: "string"}, - }, - Required: []string{"type", "status"}, - }, - }, - }, - "experimentalConditions": { - Type: "array", - Items: &apiextensionsv1.JSONSchemaPropsOrArray{ - Schema: &apiextensionsv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "type": {Type: "string"}, - "status": {Type: "string"}, - "observedGeneration": {Type: "integer"}, - "reason": {Type: "string"}, - "message": {Type: "string"}, - "lastTransitionTime": {Type: "string"}, - }, - Required: []string{"type", "status"}, - }, - }, - }, - }, - }, - }) -} - // Phase2ObjList is a list of Phase2Obj. // +kubebuilder:object:root=true type Phase2ObjList struct { @@ -249,6 +157,9 @@ type Phase2ObjList struct { // Phase2Obj defines an object with conditions and back compatibility conditions. // +kubebuilder:object:root=true +// +kubebuilder:resource:path=phase2obj,scope=Namespaced,categories=cluster-api +// +kubebuilder:subresource:status +// +kubebuilder:storageversion type Phase2Obj struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -263,85 +174,39 @@ type Phase2ObjSpec struct { // Phase2ObjStatus defines the status of a Phase2Obj. type Phase2ObjStatus struct { - Bar string `json:"bar,omitempty"` - Conditions []metav1.Condition `json:"conditions,omitempty"` - BackCompatibility Phase2ObjStatusBackCompatibility `json:"backCompatibility,omitempty"` + Bar string `json:"bar,omitempty"` + + // +optional + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty"` + + // +optional + Deprecated Phase2ObjStatusDeprecated `json:"deprecated,omitempty"` +} + +// Phase2ObjStatusDeprecated defines the status.Deprecated of a Phase2Obj. +type Phase2ObjStatusDeprecated struct { + + // +optional + V1Beta1 Phase2ObjStatusDeprecatedV1Beta2 `json:"v1beta1,omitempty"` } -// Phase2ObjStatusBackCompatibility defines the status.BackCompatibility of a Phase2Obj. -type Phase2ObjStatusBackCompatibility struct { +// Phase2ObjStatusDeprecatedV1Beta2 defines the status.Deprecated.V1Beta2 of a Phase2Obj. +type Phase2ObjStatusDeprecatedV1Beta2 struct { + + // +optional Conditions clusterv1.Conditions `json:"conditions,omitempty"` } // GetConditions returns the set of conditions for this object. func (o *Phase2Obj) GetConditions() clusterv1.Conditions { - return o.Status.BackCompatibility.Conditions + return o.Status.Deprecated.V1Beta1.Conditions } // SetConditions sets the conditions on this object. func (o *Phase2Obj) SetConditions(conditions clusterv1.Conditions) { - o.Status.BackCompatibility.Conditions = conditions -} - -func phase2ObjCRD(gvk schema.GroupVersionKind) *apiextensionsv1.CustomResourceDefinition { - return generateCRD(gvk, map[string]apiextensionsv1.JSONSchemaProps{ - "metadata": { - // NOTE: in CRD there is only a partial definition of metadata schema. - // Ref https://github.com/kubernetes-sigs/controller-tools/blob/59485af1c1f6a664655dad49543c474bb4a0d2a2/pkg/crd/gen.go#L185 - Type: "object", - }, - "spec": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "foo": {Type: "string"}, - }, - }, - "status": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "bar": {Type: "string"}, - "conditions": { - Type: "array", - Items: &apiextensionsv1.JSONSchemaPropsOrArray{ - Schema: &apiextensionsv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "type": {Type: "string"}, - "status": {Type: "string"}, - "observedGeneration": {Type: "integer"}, - "reason": {Type: "string"}, - "message": {Type: "string"}, - "lastTransitionTime": {Type: "string"}, - }, - Required: []string{"type", "status"}, - }, - }, - }, - "backCompatibility": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "conditions": { - Type: "array", - Items: &apiextensionsv1.JSONSchemaPropsOrArray{ - Schema: &apiextensionsv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "type": {Type: "string"}, - "status": {Type: "string"}, - "severity": {Type: "string"}, - "reason": {Type: "string"}, - "message": {Type: "string"}, - "lastTransitionTime": {Type: "string"}, - }, - Required: []string{"type", "status"}, - }, - }, - }, - }, - }, - }, - }, - }) + o.Status.Deprecated.V1Beta1.Conditions = conditions } // Phase3ObjList is a list of Phase3Obj. @@ -354,6 +219,9 @@ type Phase3ObjList struct { // Phase3Obj defines an object with metav1.conditions. // +kubebuilder:object:root=true +// +kubebuilder:resource:path=phase3obj,scope=Namespaced,categories=cluster-api +// +kubebuilder:subresource:status +// +kubebuilder:storageversion type Phase3Obj struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -368,45 +236,10 @@ type Phase3ObjSpec struct { // Phase3ObjStatus defines the status of a Phase3Obj. type Phase3ObjStatus struct { - Bar string `json:"bar,omitempty"` - Conditions []metav1.Condition `json:"conditions,omitempty"` -} + Bar string `json:"bar,omitempty"` -func phase3ObjCRD(gvk schema.GroupVersionKind) *apiextensionsv1.CustomResourceDefinition { - return generateCRD(gvk, map[string]apiextensionsv1.JSONSchemaProps{ - "metadata": { - // NOTE: in CRD there is only a partial definition of metadata schema. - // Ref https://github.com/kubernetes-sigs/controller-tools/blob/59485af1c1f6a664655dad49543c474bb4a0d2a2/pkg/crd/gen.go#L185 - Type: "object", - }, - "spec": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "foo": {Type: "string"}, - }, - }, - "status": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "bar": {Type: "string"}, - "conditions": { - Type: "array", - Items: &apiextensionsv1.JSONSchemaPropsOrArray{ - Schema: &apiextensionsv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "type": {Type: "string"}, - "status": {Type: "string"}, - "observedGeneration": {Type: "integer"}, - "reason": {Type: "string"}, - "message": {Type: "string"}, - "lastTransitionTime": {Type: "string"}, - }, - Required: []string{"type", "status"}, - }, - }, - }, - }, - }, - }) + // +optional + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty"` } diff --git a/internal/test/builder/zz_generated.deepcopy.go b/internal/test/builder/zz_generated.deepcopy.go index bf87beff060a..68e8506cacd2 100644 --- a/internal/test/builder/zz_generated.deepcopy.go +++ b/internal/test/builder/zz_generated.deepcopy.go @@ -1025,8 +1025,24 @@ func (in *Phase1ObjStatus) DeepCopyInto(out *Phase1ObjStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ExperimentalConditions != nil { - in, out := &in.ExperimentalConditions, &out.ExperimentalConditions + in.V1Beta2.DeepCopyInto(&out.V1Beta2) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Phase1ObjStatus. +func (in *Phase1ObjStatus) DeepCopy() *Phase1ObjStatus { + if in == nil { + return nil + } + out := new(Phase1ObjStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Phase1ObjStatusV1beta2) DeepCopyInto(out *Phase1ObjStatusV1beta2) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) @@ -1034,12 +1050,12 @@ func (in *Phase1ObjStatus) DeepCopyInto(out *Phase1ObjStatus) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Phase1ObjStatus. -func (in *Phase1ObjStatus) DeepCopy() *Phase1ObjStatus { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Phase1ObjStatusV1beta2. +func (in *Phase1ObjStatusV1beta2) DeepCopy() *Phase1ObjStatusV1beta2 { if in == nil { return nil } - out := new(Phase1ObjStatus) + out := new(Phase1ObjStatusV1beta2) in.DeepCopyInto(out) return out } @@ -1128,7 +1144,7 @@ func (in *Phase2ObjStatus) DeepCopyInto(out *Phase2ObjStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - in.BackCompatibility.DeepCopyInto(&out.BackCompatibility) + in.Deprecated.DeepCopyInto(&out.Deprecated) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Phase2ObjStatus. @@ -1142,7 +1158,23 @@ func (in *Phase2ObjStatus) DeepCopy() *Phase2ObjStatus { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Phase2ObjStatusBackCompatibility) DeepCopyInto(out *Phase2ObjStatusBackCompatibility) { +func (in *Phase2ObjStatusDeprecated) DeepCopyInto(out *Phase2ObjStatusDeprecated) { + *out = *in + in.V1Beta1.DeepCopyInto(&out.V1Beta1) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Phase2ObjStatusDeprecated. +func (in *Phase2ObjStatusDeprecated) DeepCopy() *Phase2ObjStatusDeprecated { + if in == nil { + return nil + } + out := new(Phase2ObjStatusDeprecated) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Phase2ObjStatusDeprecatedV1Beta2) DeepCopyInto(out *Phase2ObjStatusDeprecatedV1Beta2) { *out = *in if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions @@ -1153,12 +1185,12 @@ func (in *Phase2ObjStatusBackCompatibility) DeepCopyInto(out *Phase2ObjStatusBac } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Phase2ObjStatusBackCompatibility. -func (in *Phase2ObjStatusBackCompatibility) DeepCopy() *Phase2ObjStatusBackCompatibility { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Phase2ObjStatusDeprecatedV1Beta2. +func (in *Phase2ObjStatusDeprecatedV1Beta2) DeepCopy() *Phase2ObjStatusDeprecatedV1Beta2 { if in == nil { return nil } - out := new(Phase2ObjStatusBackCompatibility) + out := new(Phase2ObjStatusDeprecatedV1Beta2) in.DeepCopyInto(out) return out } diff --git a/internal/test/envtest/environment.go b/internal/test/envtest/environment.go index 04bf1808504d..4ac5a7f7fa7b 100644 --- a/internal/test/envtest/environment.go +++ b/internal/test/envtest/environment.go @@ -207,6 +207,7 @@ func newEnvironment(uncachedObjs ...client.Object) *Environment { filepath.Join(root, "config", "crd", "bases"), filepath.Join(root, "controlplane", "kubeadm", "config", "crd", "bases"), filepath.Join(root, "bootstrap", "kubeadm", "config", "crd", "bases"), + filepath.Join(root, "internal", "test", "builder", "crd"), }, CRDs: []*apiextensionsv1.CustomResourceDefinition{ builder.GenericBootstrapConfigCRD.DeepCopy(), @@ -231,10 +232,6 @@ func newEnvironment(uncachedObjs ...client.Object) *Environment { builder.TestBootstrapConfigCRD.DeepCopy(), builder.TestControlPlaneTemplateCRD.DeepCopy(), builder.TestControlPlaneCRD.DeepCopy(), - builder.Phase0ObjCRD.DeepCopy(), - builder.Phase1ObjCRD.DeepCopy(), - builder.Phase2ObjCRD.DeepCopy(), - builder.Phase3ObjCRD.DeepCopy(), }, // initialize webhook here to be able to test the envtest install via webhookOptions // This should set LocalServingCertDir and LocalServingPort that are used below. diff --git a/util/conditions/experimental/doc.go b/util/conditions/experimental/doc.go deleted file mode 100644 index 3e9771f8febf..000000000000 --- a/util/conditions/experimental/doc.go +++ /dev/null @@ -1,18 +0,0 @@ -/* -Copyright 2024 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package experimental implements experimental condition utilities. -package experimental diff --git a/util/conditions/experimental/getter.go b/util/conditions/experimental/getter.go deleted file mode 100644 index e1a2257956a8..000000000000 --- a/util/conditions/experimental/getter.go +++ /dev/null @@ -1,205 +0,0 @@ -/* -Copyright 2024 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package experimental - -import ( - "reflect" - - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" -) - -// Get returns a conditions from the object. -// -// Get support retrieving conditions from objects at different stages of the transition to metav1.condition type: -// - Objects with metav1.condition in status.experimental conditions -// - Objects with metav1.condition in status.conditions -// -// In case the object does not have metav1.conditions, Get tries to read clusterv1.condition from status.conditions -// and convert them to metav1.conditions. -func Get(obj runtime.Object, conditionType string) (*metav1.Condition, error) { - conditions, err := GetAll(obj) - if err != nil { - return nil, err - } - return meta.FindStatusCondition(conditions, conditionType), nil -} - -// GetAll returns all the conditions from the object. -// -// GetAll support retrieving conditions from objects at different stages of the transition to metav1.condition type: -// - Objects with metav1.condition in status.experimental conditions -// - Objects with metav1.condition in status.conditions -// -// In case the object does not have metav1.conditions, GetAll tries to read clusterv1.condition from status.conditions -// and convert them to metav1.conditions. -func GetAll(obj runtime.Object) ([]metav1.Condition, error) { - if obj == nil { - return nil, errors.New("obj cannot be nil") - } - - switch obj.(type) { - case runtime.Unstructured: - return getFromUnstructuredObject(obj) - default: - return getFromTypedObject(obj) - } -} - -func getFromUnstructuredObject(obj runtime.Object) ([]metav1.Condition, error) { - u, ok := obj.(runtime.Unstructured) - if !ok { - // NOTE: this should not happen due to the type assertion before calling this fun - return nil, errors.New("obj cannot be converted to runtime.Unstructured") - } - - if !reflect.ValueOf(u).Elem().IsValid() { - return nil, errors.New("obj cannot be nil") - } - - value, _, _ := unstructured.NestedFieldNoCopy(u.UnstructuredContent(), "status", "experimentalConditions") - if conditions, ok := value.([]interface{}); ok { - return convertUnstructuredConditions(conditions), nil - } - - value, _, _ = unstructured.NestedFieldNoCopy(u.UnstructuredContent(), "status", "conditions") - if conditions, ok := value.([]interface{}); ok { - return convertUnstructuredConditions(conditions), nil - } - - return nil, errors.New("obj must have Status with one of Conditions or ExperimentalConditions") -} - -func convertUnstructuredConditions(conditions []interface{}) []metav1.Condition { - if conditions == nil { - return nil - } - - convertedConditions := make([]metav1.Condition, 0, len(conditions)) - for _, c := range conditions { - cMap, ok := c.(map[string]interface{}) - if !ok || cMap == nil { - // TODO: think about returning an error in this case and when type of status are not set (as a signal it is a condition type) - continue - } - - var conditionType string - if v, ok := cMap["type"]; ok { - conditionType = v.(string) - } - - var status string - if v, ok := cMap["status"]; ok { - status = v.(string) - } - - var observedGeneration int64 - if v, ok := cMap["observedGeneration"]; ok { - observedGeneration = v.(int64) - } - - var lastTransitionTime metav1.Time - if v, ok := cMap["lastTransitionTime"]; ok { - _ = lastTransitionTime.UnmarshalQueryParameter(v.(string)) - } - - var reason string - if v, ok := cMap["reason"]; ok { - reason = v.(string) - } - - var message string - if v, ok := cMap["message"]; ok { - message = v.(string) - } - - convertedConditions = append(convertedConditions, metav1.Condition{ - Type: conditionType, - Status: metav1.ConditionStatus(status), - ObservedGeneration: observedGeneration, - LastTransitionTime: lastTransitionTime, - Reason: reason, - Message: message, - }) - } - return convertedConditions -} - -func getFromTypedObject(obj runtime.Object) ([]metav1.Condition, error) { - ptr := reflect.ValueOf(obj) - if ptr.Kind() != reflect.Pointer { - return nil, errors.New("obj must be a pointer") - } - - elem := ptr.Elem() - if !elem.IsValid() { - return nil, errors.New("obj must be a valid value (non zero value of its type)") - } - - statusField := elem.FieldByName("Status") - if statusField == (reflect.Value{}) { - return nil, errors.New("obj must have a Status field") - } - - // Get conditions. - // NOTE: Given that we allow providers to migrate at different speed, it is required to support objects at the different stage of the transition from legacy conditions to metav1.conditions. - // In order to handle this, first try to read Status.ExperimentalConditions, then Status.Conditions; for Status.Conditions, also support conversion from legacy conditions. - // The ExperimentalConditions branch and the conversion from legacy conditions should be dropped when v1beta1 API are removed. - - if conditionField := statusField.FieldByName("ExperimentalConditions"); conditionField != (reflect.Value{}) { - v1beta2Conditions, ok := conditionField.Interface().([]metav1.Condition) - if !ok { - return nil, errors.New("obj.Status.ExperimentalConditions must be of type []metav1.conditions") - } - return v1beta2Conditions, nil - } - - if conditionField := statusField.FieldByName("Conditions"); conditionField != (reflect.Value{}) { - conditions, ok := conditionField.Interface().([]metav1.Condition) - if ok { - return conditions, nil - } - - return convertFromV1Beta1Conditions(conditionField) - } - - return nil, errors.New("obj.Status must have one of Conditions or ExperimentalConditions") -} - -func convertFromV1Beta1Conditions(conditionField reflect.Value) ([]metav1.Condition, error) { - v1betaConditions, ok := conditionField.Interface().(clusterv1.Conditions) - if !ok { - return nil, errors.New("obj.Status.Conditions must be of type []metav1.conditions or []clusterv1.Condition") - } - - convertedConditions := make([]metav1.Condition, len(v1betaConditions)) - for i, c := range v1betaConditions { - convertedConditions[i] = metav1.Condition{ - Type: string(c.Type), - Status: metav1.ConditionStatus(c.Status), - LastTransitionTime: c.LastTransitionTime, - Reason: c.Reason, - Message: c.Message, - } - } - return convertedConditions, nil -} diff --git a/util/conditions/experimental/setter.go b/util/conditions/experimental/setter.go deleted file mode 100644 index b8334b5317e8..000000000000 --- a/util/conditions/experimental/setter.go +++ /dev/null @@ -1,202 +0,0 @@ -/* -Copyright 2024 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package experimental - -import ( - "reflect" - "sort" - - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" -) - -// SetOption is some configuration that modifies options for a Set request. -type SetOption interface { - // ApplyToSet applies this configuration to the given Set options. - ApplyToSet(option *SetOptions) -} - -// SetOptions allows to define options for the set operation. -type SetOptions struct { - conditionsFields []string - less Less -} - -// ApplyOptions applies the given list options on these options, -// and then returns itself (for convenient chaining). -func (o *SetOptions) ApplyOptions(opts []SetOption) *SetOptions { - for _, opt := range opts { - opt.ApplyToSet(o) - } - return o -} - -// Set a condition on the given object. -// -// Set support adding conditions to objects at different stages of the transition to metav1.condition type: -// - Objects with metav1.condition in status.experimental conditions -// - Objects with metav1.condition in status.conditions -// -// In case the object is unstructured, it is required to provide the path where conditions are defined using -// the ConditionFields option (because it is not possible to infer where conditions are by looking at the UnstructuredContent only). -// -// Additionally, Set enforce the lexicographic condition order (Available and Ready fist, everything else in alphabetical order), -// but this can be changed by using the ConditionSortFunc option. -func Set(obj runtime.Object, condition metav1.Condition, opts ...SetOption) error { - conditions, err := GetAll(obj) - if err != nil { - return err - } - - if changed := meta.SetStatusCondition(&conditions, condition); !changed { - return nil - } - - return SetAll(obj, conditions, opts...) -} - -// SetAll the conditions on the given object. -// -// SetAll support adding conditions to objects at different stages of the transition to metav1.condition type: -// - Objects with metav1.condition in status.experimental conditions -// - Objects with metav1.condition in status.conditions -// -// In case the object is unstructured, it is required to provide the path where conditions are defined using -// the ConditionFields option (because it is not possible to infer where conditions are by looking at the UnstructuredContent only). -// -// Additionally, SetAll enforce the lexicographic condition order (Available and Ready fist, everything else in alphabetical order), -// but this can be changed by using the ConditionSortFunc option. -func SetAll(obj runtime.Object, conditions []metav1.Condition, opts ...SetOption) error { - setOpt := &SetOptions{ - // By default sort condition by lexicographicLess order (first available, then ready, then the other conditions if alphabetical order. - less: lexicographicLess, - } - setOpt.ApplyOptions(opts) - - // TODO: think about setting ObservedGeneration from obj - // TODO: think about setting LastTransition Time with reconcile time --> All the conditions will flit at the same time, but we loose correlation with logs - - if setOpt.less != nil { - sort.SliceStable(conditions, func(i, j int) bool { - return setOpt.less(conditions[i], conditions[j]) - }) - } - - switch obj.(type) { - case runtime.Unstructured: - return setToUnstructuredObject(obj, conditions, setOpt.conditionsFields) - default: - return setToTypedObject(obj, conditions) - } -} - -func setToUnstructuredObject(obj runtime.Object, conditions []metav1.Condition, conditionsFields []string) error { - if obj == nil { - return errors.New("cannot set conditions on a nil object") - } - - // NOTE: Given that we allow providers to migrate at different speed, it is required to support objects at the different stage of the transition from legacy conditions to metav1.conditions. - // In order to handle this with Unstructured, it is required to ask the path for the conditions field in the given object (it cannot be inferred). - // conditionsFields should be dropped when v1beta1 API are removed because new conditions will always be in Status.Conditions. - - if len(conditionsFields) == 0 { - return errors.New("while transition from legacy conditions types to metav1.conditions types is in progress, cannot set conditions on Unstructured object without conditionFields path being set") - } - - u, ok := obj.(runtime.Unstructured) - if !ok { - return errors.New("cannot call setToUnstructuredObject on a object which is not Unstructured") - } - - if !reflect.ValueOf(u).Elem().IsValid() { - return errors.New("obj cannot be nil") - } - - v := make([]interface{}, 0, len(conditions)) - for i := range conditions { - c, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&conditions[i]) - if err != nil { - return errors.Wrapf(err, "failed to convert conditions %s to Unstructured", conditions[i].Type) - } - v = append(v, c) - } - - if err := unstructured.SetNestedField(u.UnstructuredContent(), v, conditionsFields...); err != nil { - return errors.Wrap(err, "failed to set conditions into Unstructured") - } - return nil -} - -var metav1ConditionsType = reflect.TypeOf([]metav1.Condition{}) - -func setToTypedObject(obj runtime.Object, conditions []metav1.Condition) error { - if obj == nil { - return errors.New("cannot set conditions on a nil object") - } - - ptr := reflect.ValueOf(obj) - if ptr.Kind() != reflect.Pointer { - return errors.New("cannot set conditions on a object that is not a pointer") - } - - elem := ptr.Elem() - if !elem.IsValid() { - return errors.New("obj must be a valid value (non zero value of its type)") - } - - statusField := elem.FieldByName("Status") - if statusField == (reflect.Value{}) { - return errors.New("cannot set conditions on a object without a status field") - } - - // Set conditions. - // NOTE: Given that we allow providers to migrate at different speed, it is required to support objects at the different stage of the transition from legacy conditions to metav1.conditions. - // In order to handle this, first try to set Status.ExperimentalConditions, then Status.Conditions. - // The ExperimentalConditions branch should be dropped when v1beta1 API are removed. - - if conditionField := statusField.FieldByName("ExperimentalConditions"); conditionField != (reflect.Value{}) { - if conditionField.Type() != metav1ConditionsType { - return errors.Errorf("cannot set conditions on Status.ExperimentalConditions field if it isn't %s: %s type detected", metav1ConditionsType.String(), reflect.TypeOf(conditionField.Interface()).String()) - } - - setToTypedField(conditionField, conditions) - return nil - } - - if conditionField := statusField.FieldByName("Conditions"); conditionField != (reflect.Value{}) { - if conditionField.Type() != metav1ConditionsType { - return errors.Errorf("cannot set conditions on Status.Conditions field if it isn't %s: %s type detected", metav1ConditionsType.String(), reflect.TypeOf(conditionField.Interface()).String()) - } - - setToTypedField(conditionField, conditions) - return nil - } - - return errors.New("cannot set conditions on a object without Status.ExperimentalConditions or Status.Conditions") -} - -func setToTypedField(conditionField reflect.Value, conditions []metav1.Condition) { - n := len(conditions) - conditionField.Set(reflect.MakeSlice(conditionField.Type(), n, n)) - for i := range n { - itemField := conditionField.Index(i) - itemField.Set(reflect.ValueOf(conditions[i])) - } -} diff --git a/util/conditions/experimental/setter_test.go b/util/conditions/experimental/setter_test.go deleted file mode 100644 index 0793c5192239..000000000000 --- a/util/conditions/experimental/setter_test.go +++ /dev/null @@ -1,284 +0,0 @@ -/* -Copyright 2024 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package experimental - -import ( - "testing" - - "github.com/google/go-cmp/cmp" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/internal/test/builder" -) - -func TestSetAll(t *testing.T) { - now := metav1.Now().Rfc3339Copy() - - conditions := []metav1.Condition{ - { - Type: "fooCondition", - Status: metav1.ConditionTrue, - ObservedGeneration: 10, - LastTransitionTime: now, - Reason: "FooReason", - Message: "FooMessage", - }, - } - - cloneConditions := func() []metav1.Condition { - ret := make([]metav1.Condition, len(conditions)) - copy(ret, conditions) - return ret - } - - t.Run("fails with nil", func(t *testing.T) { - g := NewWithT(t) - - conditions := cloneConditions() - err := SetAll(nil, conditions) - g.Expect(err).To(HaveOccurred()) - }) - - t.Run("fails for nil object", func(t *testing.T) { - g := NewWithT(t) - var foo *builder.Phase0Obj - - conditions := cloneConditions() - err := SetAll(foo, conditions) - g.Expect(err).To(HaveOccurred()) - - var fooUnstructured *unstructured.Unstructured - - err = SetAll(fooUnstructured, conditions, ConditionFields{"status", "experimentalConditions"}) - g.Expect(err).To(HaveOccurred()) - }) - - t.Run("fails for object without status", func(t *testing.T) { - g := NewWithT(t) - foo := &ObjWithoutStatus{} - - conditions := cloneConditions() - err := SetAll(foo, conditions) - g.Expect(err).To(HaveOccurred()) - - // TODO: think about how unstructured can detect status without statue (if we ever need it, because for unstructured the users explicitly define the ConditionFields) - }) - - t.Run("fails for object with status without conditions or experimental conditions", func(t *testing.T) { - g := NewWithT(t) - foo := &ObjWithStatusWithoutConditions{} - - _, err := GetAll(foo) - g.Expect(err).To(HaveOccurred()) - - // TODO: think about how unstructured can detect status without conditions or experimental condition type (if we ever need it, because for unstructured the users explicitly define the ConditionFields) - }) - - t.Run("fails for object with wrong condition type", func(t *testing.T) { - g := NewWithT(t) - foo := &ObjWithWrongConditionType{} - - _, err := GetAll(foo) - g.Expect(err).To(HaveOccurred()) - - // TODO: think about how unstructured can detect status with wrong conditions type (if we ever need it, because for unstructured the users explicitly define the ConditionFields) - }) - - t.Run("fails for object with wrong experimental condition type", func(t *testing.T) { - g := NewWithT(t) - foo := &ObjWithWrongExperimentalConditionType{} - - _, err := GetAll(foo) - g.Expect(err).To(HaveOccurred()) - - // TODO: think about how unstructured can detect status with wrong experimental conditions type (if we ever need it, because for unstructured the users explicitly define the ConditionFields) - }) - - t.Run("fails for Unstructured when ConditionFields are not provided", func(t *testing.T) { - g := NewWithT(t) - fooUnstructured := &unstructured.Unstructured{} - - conditions := cloneConditions() - err := SetAll(fooUnstructured, conditions) - g.Expect(err).To(HaveOccurred()) - }) - - t.Run("v1beta object with legacy conditions", func(t *testing.T) { - g := NewWithT(t) - foo := &builder.Phase0Obj{ - Status: builder.Phase0ObjStatus{Conditions: nil}, - } - - conditions := cloneConditions() - err := SetAll(foo, conditions) - g.Expect(err).To(HaveOccurred()) // Can't set legacy conditions. - }) - - t.Run("v1beta1 object with both legacy and experimental conditions", func(t *testing.T) { - g := NewWithT(t) - foo := &builder.Phase1Obj{ - Status: builder.Phase1ObjStatus{ - Conditions: clusterv1.Conditions{ - { - Type: "barCondition", - Status: corev1.ConditionFalse, - LastTransitionTime: now, - }, - }, - ExperimentalConditions: nil, - }, - } - - conditions := cloneConditions() - err := SetAll(foo, conditions) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(foo.Status.ExperimentalConditions).To(Equal(conditions), cmp.Diff(foo.Status.ExperimentalConditions, conditions)) - }) - - t.Run("v1beta1 object with both legacy and experimental conditions / Unstructured", func(t *testing.T) { - g := NewWithT(t) - foo := &builder.Phase1Obj{ - Status: builder.Phase1ObjStatus{ - Conditions: clusterv1.Conditions{ - { - Type: "barCondition", - Status: corev1.ConditionFalse, - LastTransitionTime: now, - }, - }, - ExperimentalConditions: nil, - }, - } - - conditions := cloneConditions() - fooUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(foo) - g.Expect(err).NotTo(HaveOccurred()) - - u := &unstructured.Unstructured{Object: fooUnstructured} - err = SetAll(u, conditions, ConditionFields{"status", "experimentalConditions"}) - g.Expect(err).NotTo(HaveOccurred()) - - fooFromUnstructured := &builder.Phase1Obj{} - err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), fooFromUnstructured) - g.Expect(err).NotTo(HaveOccurred()) - - got, err := GetAll(fooFromUnstructured) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(got).To(MatchConditions(conditions), cmp.Diff(got, conditions)) - }) - - t.Run("v1beta2 object with conditions and backward compatible conditions", func(t *testing.T) { - g := NewWithT(t) - foo := &builder.Phase2Obj{ - Status: builder.Phase2ObjStatus{ - Conditions: nil, - BackCompatibility: builder.Phase2ObjStatusBackCompatibility{ - Conditions: clusterv1.Conditions{ - { - Type: "barCondition", - Status: corev1.ConditionFalse, - LastTransitionTime: now, - }, - }, - }, - }, - } - - conditions := cloneConditions() - err := SetAll(foo, conditions) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(foo.Status.Conditions).To(MatchConditions(conditions), cmp.Diff(foo.Status.Conditions, conditions)) - }) - - t.Run("v1beta2 object with conditions and backward compatible conditions / Unstructured", func(t *testing.T) { - g := NewWithT(t) - foo := &builder.Phase2Obj{ - Status: builder.Phase2ObjStatus{ - Conditions: nil, - BackCompatibility: builder.Phase2ObjStatusBackCompatibility{ - Conditions: clusterv1.Conditions{ - { - Type: "barCondition", - Status: corev1.ConditionFalse, - LastTransitionTime: now, - }, - }, - }, - }, - } - - conditions := cloneConditions() - fooUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(foo) - g.Expect(err).NotTo(HaveOccurred()) - - u := &unstructured.Unstructured{Object: fooUnstructured} - err = SetAll(u, conditions, ConditionFields{"status", "conditions"}) - g.Expect(err).NotTo(HaveOccurred()) - - fooFromUnstructured := &builder.Phase2Obj{} - err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), fooFromUnstructured) - g.Expect(err).NotTo(HaveOccurred()) - - got, err := GetAll(fooFromUnstructured) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(got).To(MatchConditions(conditions), cmp.Diff(got, conditions)) - }) - - t.Run("v1beta2 object with conditions (end state)", func(t *testing.T) { - g := NewWithT(t) - foo := &builder.Phase3Obj{ - Status: builder.Phase3ObjStatus{ - Conditions: nil, - }, - } - - conditions := cloneConditions() - err := SetAll(foo, conditions) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(foo.Status.Conditions).To(Equal(conditions), cmp.Diff(foo.Status.Conditions, conditions)) - }) - - t.Run("v1beta2 object with conditions (end state) / Unstructured", func(t *testing.T) { - g := NewWithT(t) - foo := &builder.Phase3Obj{ - Status: builder.Phase3ObjStatus{ - Conditions: nil, - }, - } - - conditions := cloneConditions() - fooUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(foo) - g.Expect(err).NotTo(HaveOccurred()) - - u := &unstructured.Unstructured{Object: fooUnstructured} - err = SetAll(u, conditions, ConditionFields{"status", "conditions"}) - g.Expect(err).NotTo(HaveOccurred()) - - fooFromUnstructured := &builder.Phase3Obj{} - err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), fooFromUnstructured) - g.Expect(err).NotTo(HaveOccurred()) - - got, err := GetAll(fooFromUnstructured) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(got).To(MatchConditions(conditions), cmp.Diff(got, conditions)) - }) -} diff --git a/util/conditions/experimental/aggregate.go b/util/conditions/v1beta2/aggregate.go similarity index 65% rename from util/conditions/experimental/aggregate.go rename to util/conditions/v1beta2/aggregate.go index 0be2f93fa2af..b8c6fc41249f 100644 --- a/util/conditions/experimental/aggregate.go +++ b/util/conditions/v1beta2/aggregate.go @@ -14,16 +14,17 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "fmt" + "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) -// AggregateOption is some configuration that modifies options for a aggregate request. +// AggregateOption is some configuration that modifies options for a aggregate call. type AggregateOption interface { // ApplyToAggregate applies this configuration to the given aggregate options. ApplyToAggregate(option *AggregateOptions) @@ -31,8 +32,8 @@ type AggregateOption interface { // AggregateOptions allows to set options for the aggregate operation. type AggregateOptions struct { - mergeStrategy MergeStrategy - overrideType string + mergeStrategy MergeStrategy + targetConditionType string } // ApplyOptions applies the given list options on these options, @@ -44,35 +45,42 @@ func (o *AggregateOptions) ApplyOptions(opts []AggregateOption) *AggregateOption return o } -// NewAggregateCondition aggregates a condition from a list of objects; if the given condition does not exist in the source object, -// missing conditions are considered Unknown, reason NotYetReported. +// NewAggregateCondition aggregates a condition from a list of objects; the given condition must have positive polarity; +// if the given condition does not exist in one of the source objects, missing conditions are considered Unknown, reason NotYetReported. // // By default, the Aggregate condition has the same type of the source condition, but this can be changed by using -// the OverrideType option. +// the TargetConditionType option. // // Additionally, it is possible to inject custom merge strategies using the WithMergeStrategy option. -func NewAggregateCondition(objects []runtime.Object, conditionType string, opts ...AggregateOption) (*metav1.Condition, error) { +func NewAggregateCondition(sourceObjs []runtime.Object, sourceConditionType string, opts ...AggregateOption) (*metav1.Condition, error) { + if len(sourceObjs) == 0 { + return nil, errors.New("sourceObjs can't be empty") + } + aggregateOpt := &AggregateOptions{ - mergeStrategy: newDefaultMergeStrategy(), + mergeStrategy: newDefaultMergeStrategy(), + targetConditionType: sourceConditionType, } aggregateOpt.ApplyOptions(opts) - conditionsInScope := make([]ConditionWithOwnerInfo, 0, len(objects)) - for _, obj := range objects { - // TODO: consider if we want to aggregate all errors before returning + conditionsInScope := make([]ConditionWithOwnerInfo, 0, len(sourceObjs)) + for _, obj := range sourceObjs { conditions, err := getConditionsWithOwnerInfo(obj) if err != nil { + // Note: considering all sourceObjs are usually of the same type (and thus getConditionsWithOwnerInfo will either pass or fail for all sourceObjs), we are returning at the first error. + // This also avoid to implement fancy error aggregation, which is required to manage a potentially high number of sourceObjs/errors. return nil, err } // Drops all the conditions not in scope for the merge operation hasConditionType := false for _, condition := range conditions { - if condition.Type != conditionType { + if condition.Type != sourceConditionType { continue } conditionsInScope = append(conditionsInScope, condition) hasConditionType = true + break } // Add the expected conditions if it does not exist, so we are compliant with K8s guidelines @@ -83,10 +91,11 @@ func NewAggregateCondition(objects []runtime.Object, conditionType string, opts conditionsInScope = append(conditionsInScope, ConditionWithOwnerInfo{ OwnerResource: conditionOwner, Condition: metav1.Condition{ - Type: conditionType, + Type: aggregateOpt.targetConditionType, Status: metav1.ConditionUnknown, Reason: NotYetReportedReason, - Message: fmt.Sprintf("Condition %s not yet reported from %s", conditionType, conditionOwner.String()), + Message: fmt.Sprintf("Condition %s not yet reported from %s", sourceConditionType, conditionOwner.Kind), + // NOTE: LastTransitionTime and ObservedGeneration are not relevant for merge. }, }) } @@ -94,7 +103,7 @@ func NewAggregateCondition(objects []runtime.Object, conditionType string, opts status, reason, message, err := aggregateOpt.mergeStrategy.Merge( conditionsInScope, - []string{conditionType}, + []string{sourceConditionType}, nil, // negative conditions false, // step counter ) @@ -103,14 +112,11 @@ func NewAggregateCondition(objects []runtime.Object, conditionType string, opts } c := &metav1.Condition{ - Type: conditionType, + Type: aggregateOpt.targetConditionType, Status: status, Reason: reason, Message: message, - } - - if aggregateOpt.overrideType != "" { - c.Type = aggregateOpt.overrideType + // NOTE: LastTransitionTime and ObservedGeneration will be set when this condition is added to an object by calling Set. } return c, err @@ -119,9 +125,9 @@ func NewAggregateCondition(objects []runtime.Object, conditionType string, opts // SetAggregateCondition is a convenience method that calls NewAggregateCondition to create an aggregate condition from the source objects, // and then calls Set to add the new condition to the target object. func SetAggregateCondition(sourceObjs []runtime.Object, targetObj runtime.Object, conditionType string, opts ...AggregateOption) error { - mirrorCondition, err := NewAggregateCondition(sourceObjs, conditionType, opts...) + aggregateCondition, err := NewAggregateCondition(sourceObjs, conditionType, opts...) if err != nil { return err } - return Set(targetObj, *mirrorCondition) + return Set(targetObj, *aggregateCondition) } diff --git a/util/conditions/experimental/aggregate_test.go b/util/conditions/v1beta2/aggregate_test.go similarity index 71% rename from util/conditions/experimental/aggregate_test.go rename to util/conditions/v1beta2/aggregate_test.go index 908d118f48e4..eb63b0779b6e 100644 --- a/util/conditions/experimental/aggregate_test.go +++ b/util/conditions/v1beta2/aggregate_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "fmt" @@ -45,9 +45,24 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionFalse, // False because there is one issue - Reason: "Reason-1", // Picking the reason from the only existing issue - Message: "(False): Message-1 from default/obj0", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionFalse, // False because there is one issue + Reason: "Reason-1", // Picking the reason from the only existing issue + Message: "Message-1 from Phase3Obj obj0", // messages from all the issues & unknown conditions (info dropped) + }, + }, + { + name: "One issue with target type", + conditions: [][]metav1.Condition{ + {{Type: AvailableCondition, Status: metav1.ConditionFalse, Reason: "Reason-1", Message: "Message-1"}}, // obj1 + {{Type: AvailableCondition, Status: metav1.ConditionTrue, Reason: "Reason-99", Message: "Message-99"}}, // obj2 + }, + conditionType: AvailableCondition, + options: []AggregateOption{TargetConditionType("SomethingAvailable")}, + want: &metav1.Condition{ + Type: "SomethingAvailable", + Status: metav1.ConditionFalse, // False because there is one issue + Reason: "Reason-1", // Picking the reason from the only existing issue + Message: "Message-1 from Phase3Obj obj0", // messages from all the issues & unknown conditions (info dropped) }, }, { @@ -62,9 +77,9 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionFalse, // False because there is one issue - Reason: ManyIssuesReason, // Using a generic reason - Message: "(False): Message-1 from default/obj0, default/obj1, default/obj2", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionFalse, // False because there is one issue + Reason: MultipleIssuesReason, // Using a generic reason + Message: "Message-1 from Phase3Objs obj0, obj1, obj2", // messages from all the issues & unknown conditions (info dropped) }, }, { @@ -81,9 +96,9 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionFalse, // False because there is one issue - Reason: ManyIssuesReason, // Using a generic reason - Message: "(False): Message-1 from default/obj0, default/obj1, default/obj2 and 2 other Phase3Objs", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionFalse, // False because there is one issue + Reason: MultipleIssuesReason, // Using a generic reason + Message: "Message-1 from Phase3Objs obj0, obj1, obj2 and 2 more", // messages from all the issues & unknown conditions (info dropped) }, }, { @@ -101,9 +116,9 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionFalse, // False because there is one issue - Reason: ManyIssuesReason, // Using a generic reason - Message: "(False): Message-1 from default/obj0, default/obj3, default/obj4; (False): Message-2 from default/obj1, default/obj2; (False): Message-3 from default/obj5", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionFalse, // False because there is one issue + Reason: MultipleIssuesReason, // Using a generic reason + Message: "Message-1 from Phase3Objs obj0, obj3, obj4; Message-2 from Phase3Objs obj1, obj2; Message-3 from Phase3Obj obj5", // messages from all the issues & unknown conditions (info dropped) }, }, { @@ -121,9 +136,9 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionFalse, // False because there is one issue - Reason: ManyIssuesReason, // Using a generic reason - Message: "(False): Message-1 from default/obj0, default/obj4; (False): Message-2 from default/obj1; (False): Message-3 from default/obj5; other 2 Phase3Objs with issues", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionFalse, // False because there is one issue + Reason: MultipleIssuesReason, // Using a generic reason + Message: "Message-1 from Phase3Objs obj0, obj4; Message-2 from Phase3Obj obj1; Message-3 from Phase3Obj obj5; 2 Phase3Objs with other issues", // messages from all the issues & unknown conditions (info dropped) }, }, { @@ -138,9 +153,9 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionFalse, // False because there is one issue - Reason: ManyIssuesReason, // Using a generic reason - Message: "(False): Message-1 from default/obj0; (False): Message-2 from default/obj1; (Unknown): Message-3 from default/obj2", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionFalse, // False because there is one issue + Reason: MultipleIssuesReason, // Using a generic reason + Message: "Message-1 from Phase3Obj obj0; Message-2 from Phase3Obj obj1; Message-3 from Phase3Obj obj2", // messages from all the issues & unknown conditions (info dropped) }, }, { @@ -156,9 +171,9 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionFalse, // False because there is one issue - Reason: ManyIssuesReason, // Using a generic reason - Message: "(False): Message-1 from default/obj0; (False): Message-2 from default/obj1; (False): Message-4 from default/obj3; other 1 Phase3Objs unknown", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionFalse, // False because there is one issue + Reason: MultipleIssuesReason, // Using a generic reason + Message: "Message-1 from Phase3Obj obj0; Message-2 from Phase3Obj obj1; Message-4 from Phase3Obj obj3; 1 Phase3Obj with status unknown", // messages from all the issues & unknown conditions (info dropped) }, }, { @@ -176,9 +191,9 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionUnknown, // False because there is one issue - Reason: ManyUnknownsReason, // Using a generic reason - Message: "(Unknown): Message-1 from default/obj0, default/obj4; (Unknown): Message-2 from default/obj1; (Unknown): Message-3 from default/obj5; other 2 Phase3Objs unknown", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionUnknown, // False because there is one issue + Reason: MultipleUnknownReported, // Using a generic reason + Message: "Message-1 from Phase3Objs obj0, obj4; Message-2 from Phase3Obj obj1; Message-3 from Phase3Obj obj5; 2 Phase3Objs with status unknown", // messages from all the issues & unknown conditions (info dropped) }, }, { @@ -195,9 +210,9 @@ func TestAggregate(t *testing.T) { options: []AggregateOption{}, want: &metav1.Condition{ Type: AvailableCondition, - Status: metav1.ConditionTrue, // False because there is one issue - Reason: ManyInfoReason, // Using a generic reason - Message: "(True): Message-1 from default/obj0, default/obj4; (True): Message-2 from default/obj1; (True): Message-3 from default/obj5; other 1 Phase3Objs with info messages", // messages from all the issues & unknown conditions (info dropped) + Status: metav1.ConditionTrue, // False because there is one issue + Reason: MultipleInfoReason, // Using a generic reason + Message: "Message-1 from Phase3Objs obj0, obj4; Message-2 from Phase3Obj obj1; Message-3 from Phase3Obj obj5; 1 Phase3Obj with additional info", // messages from all the issues & unknown conditions (info dropped) }, }, } @@ -225,4 +240,10 @@ func TestAggregate(t *testing.T) { g.Expect(got).To(Equal(tt.want)) }) } + + t.Run("Fails if conditions in scope are empty", func(t *testing.T) { + g := NewWithT(t) + _, err := NewAggregateCondition(nil, AvailableCondition) // no source objs --> Condition in scope will be empty + g.Expect(err).To(HaveOccurred()) + }) } diff --git a/util/conditions/v1beta2/doc.go b/util/conditions/v1beta2/doc.go new file mode 100644 index 000000000000..95ec14a193c1 --- /dev/null +++ b/util/conditions/v1beta2/doc.go @@ -0,0 +1,32 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package v1beta2 implements utils for metav1.Conditions that will be used starting with the v1beta2 API. +// +// Please note that in order to make this change while respecting API deprecation rules, it is required +// to go through a phased approach: +// - Phase 1. metav1.Conditions will be added into v1beta1 API types under the Status.V1beta2.Conditions struct (clusterv1.Conditions will remain in Status.Condition) +// - Phase 2. when introducing v1beta2 API types: +// - clusterv1.Conditions will be moved from Status.Condition to Status.Deprecated.V1Beta1.Conditions +// - metav1.Conditions will be moved from Status.V1beta2.Conditions from Status.Condition +// +// - Phase 3. when removing v1beta1 API types, Status.Deprecated will be dropped. +// +// Please see the proposal ... for more details TODO add link +// +// In order to make this transition easier both for CAPI and other projects using this package, +// utils automatically adapt to handle objects at different stage of the transition. +package v1beta2 diff --git a/util/conditions/v1beta2/getter.go b/util/conditions/v1beta2/getter.go new file mode 100644 index 000000000000..fbd0fdce5007 --- /dev/null +++ b/util/conditions/v1beta2/getter.go @@ -0,0 +1,284 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta2 + +import ( + "reflect" + + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" +) + +// TODO: Move to the API package. +const ( + // NoReasonReported identifies a clusterv1.Condition that reports no reason. + NoReasonReported = "NoReasonReported" +) + +// Get returns a conditions from the object. +// +// Get support retrieving conditions from objects at different stages of the transition to metav1.condition type: +// - Objects with clusterv1.condition in status.Conditions; in this case a best effort conversion +// to metav1.condition is performed, just enough to allow surfacing a condition from a providers object with Mirror +// - Objects with metav1.condition in status.V1Beta2.Conditions conditions +// - Objects with metav1.condition in status.conditions +// +// Please note that Get also supports reading conditions from unstructured objects; in this case, best effort +// conversion from a map is performed, just enough to allow surfacing a condition from a providers object with Mirror +// +// In case the object does not have metav1.conditions, Get tries to read clusterv1.condition from status.conditions +// and convert them to metav1.conditions. +func Get(sourceObj runtime.Object, sourceConditionType string) (*metav1.Condition, error) { + conditions, err := GetAll(sourceObj) + if err != nil { + return nil, err + } + return meta.FindStatusCondition(conditions, sourceConditionType), nil +} + +// GetAll returns all the conditions from the object. +// +// Get support retrieving conditions from objects at different stages of the transition to metav1.condition type: +// - Objects with clusterv1.condition in status.Conditions; in this case a best effort conversion +// to metav1.condition is performed, just enough to allow surfacing a condition from a providers object with Mirror +// - Objects with metav1.condition in status.V1Beta2.Conditions conditions +// - Objects with metav1.condition in status.conditions +// +// Please note that Get also supports reading conditions from unstructured objects; in this case, best effort +// conversion from a map is performed, just enough to allow surfacing a condition from a providers object with Mirror +// +// In case the object does not have metav1.conditions, GetAll tries to read clusterv1.condition from status.conditions +// and convert them to metav1.conditions. +func GetAll(sourceObj runtime.Object) ([]metav1.Condition, error) { + if sourceObj == nil { + return nil, errors.New("sourceObj cannot be nil") + } + + switch sourceObj.(type) { + case runtime.Unstructured: + return getFromUnstructuredObject(sourceObj) + default: + return getFromTypedObject(sourceObj) + } +} + +func getFromUnstructuredObject(obj runtime.Object) ([]metav1.Condition, error) { + u, ok := obj.(runtime.Unstructured) + if !ok { + // NOTE: this should not happen due to the type assertion before calling this func + return nil, errors.New("obj cannot be converted to runtime.Unstructured") + } + + if !reflect.ValueOf(u).Elem().IsValid() { + return nil, errors.New("obj cannot be nil") + } + + ownerInfo := getConditionOwnerInfo(obj) + + value, exists, err := unstructured.NestedFieldNoCopy(u.UnstructuredContent(), "status", "v1beta2", "conditions") + if exists && err == nil { + if conditions, ok := value.([]interface{}); ok { + r, err := convertFromUnstructuredConditions(conditions) + if err != nil { + return nil, errors.Wrapf(err, "failied to convert %s.status.v1beta2.conditions to []metav1.Condition", ownerInfo) + } + return r, nil + } + } + + value, exists, err = unstructured.NestedFieldNoCopy(u.UnstructuredContent(), "status", "conditions") + if exists && err == nil { + if conditions, ok := value.([]interface{}); ok { + r, err := convertFromUnstructuredConditions(conditions) + if err != nil { + return nil, errors.Wrapf(err, "failied to convert %s.status.conditions to []metav1.Condition", ownerInfo) + } + return r, nil + } + } + + return nil, errors.Errorf("%s must have status with one of conditions or v1beta2.conditions", ownerInfo) +} + +// convertFromUnstructuredConditions converts []interface{} to metav1.Condition; this operation must account for +// objects which are not transitioning to metav1.Condition, or not yet fully transitioned, and thus a best +// effort conversion of values to metav1.Condition is performed. +func convertFromUnstructuredConditions(conditions []interface{}) ([]metav1.Condition, error) { + if conditions == nil { + return nil, nil + } + + convertedConditions := make([]metav1.Condition, 0, len(conditions)) + for _, c := range conditions { + cMap, ok := c.(map[string]interface{}) + if !ok || cMap == nil { + continue + } + + var conditionType string + if v, ok := cMap["type"]; ok { + conditionType = v.(string) + } + + var status string + if v, ok := cMap["status"]; ok { + status = v.(string) + } + + var observedGeneration int64 + if v, ok := cMap["observedGeneration"]; ok { + observedGeneration = v.(int64) + } + + var lastTransitionTime metav1.Time + if v, ok := cMap["lastTransitionTime"]; ok && v != nil && v.(string) != "" { + if err := lastTransitionTime.UnmarshalQueryParameter(v.(string)); err != nil { + return nil, errors.Wrapf(err, "failed to unmarshal last transition time value: %s", v) + } + } + + var reason string + if v, ok := cMap["reason"]; ok { + reason = v.(string) + } + + var message string + if v, ok := cMap["message"]; ok { + message = v.(string) + } + + c := metav1.Condition{ + Type: conditionType, + Status: metav1.ConditionStatus(status), + ObservedGeneration: observedGeneration, + LastTransitionTime: lastTransitionTime, + Reason: reason, + Message: message, + } + if err := validateAndFixConvertedCondition(&c); err != nil { + return nil, err + } + + convertedConditions = append(convertedConditions, c) + } + return convertedConditions, nil +} + +func getFromTypedObject(obj runtime.Object) ([]metav1.Condition, error) { + ptr := reflect.ValueOf(obj) + if ptr.Kind() != reflect.Pointer { + return nil, errors.New("obj must be a pointer") + } + + elem := ptr.Elem() + if !elem.IsValid() { + return nil, errors.New("obj must be a valid value (non zero value of its type)") + } + + ownerInfo := getConditionOwnerInfo(obj) + + statusField := elem.FieldByName("Status") + if statusField == (reflect.Value{}) { + return nil, errors.Errorf("%s must have a status field", ownerInfo) + } + + // Get conditions. + // NOTE: Given that we allow providers to migrate at different speed, it is required to support objects at the different stage of the transition from legacy conditions to metav1.conditions. + // In order to handle this, first try to read Status.V1Beta2.Conditions, then Status.Conditions; for Status.Conditions, also support conversion from legacy conditions. + // The V1Beta2 branch and the conversion from legacy conditions should be dropped when v1beta1 API are removed. + + if v1beta2Field := statusField.FieldByName("V1Beta2"); v1beta2Field != (reflect.Value{}) { + if conditionField := v1beta2Field.FieldByName("Conditions"); conditionField != (reflect.Value{}) { + v1beta2Conditions, ok := conditionField.Interface().([]metav1.Condition) + if !ok { + return nil, errors.Errorf("%s.status.v1beta2.conditions must be of type []metav1.Condition", ownerInfo) + } + return v1beta2Conditions, nil + } + } + + if conditionField := statusField.FieldByName("Conditions"); conditionField != (reflect.Value{}) { + conditions, ok := conditionField.Interface().([]metav1.Condition) + if ok { + return conditions, nil + } + + v1betaConditions, ok := conditionField.Interface().(clusterv1.Conditions) + if !ok { + return nil, errors.Errorf("%s.status.conditions must be of type []metav1.Condition or clusterv1.Conditions", ownerInfo) + } + r, err := convertFromV1Beta1Conditions(v1betaConditions) + if err != nil { + return nil, errors.Wrapf(err, "failied to convert %s.status.conditions to []metav1.Condition", ownerInfo) + } + return r, nil + } + + return nil, errors.Errorf("%s.status must have one of conditions or v1Beta2.conditions", ownerInfo) +} + +// convertFromV1Beta1Conditions converts a clusterv1.Condition to metav1.Condition. +// NOTE: this operation is performed at best effort and assuming conditions have been set using Cluster API condition utils. +func convertFromV1Beta1Conditions(v1betaConditions clusterv1.Conditions) ([]metav1.Condition, error) { + convertedConditions := make([]metav1.Condition, len(v1betaConditions)) + for i, c := range v1betaConditions { + convertedConditions[i] = metav1.Condition{ + Type: string(c.Type), + Status: metav1.ConditionStatus(c.Status), + LastTransitionTime: c.LastTransitionTime, + Reason: c.Reason, + Message: c.Message, + } + + if err := validateAndFixConvertedCondition(&convertedConditions[i]); err != nil { + return nil, err + } + } + return convertedConditions, nil +} + +// validateAndFixConvertedCondition validates and fixes a clusterv1.Condition converted to a metav1.Condition. +// this operation assumes conditions have been set using Cluster API condition utils; +// also, only a few, minimal rules are enforced, just enough to allow surfacing a condition from a providers object with Mirror. +func validateAndFixConvertedCondition(c *metav1.Condition) error { + if c.Type == "" { + return errors.New("condition type must be set") + } + if c.Status == "" { + return errors.New("condition status must be set") + } + if c.Reason == "" { + switch c.Status { + case metav1.ConditionFalse: // When using old Cluster API condition utils, for conditions with Status false, Reason can be empty only when a condition has negative polarity (means "good") + c.Reason = NoReasonReported + case metav1.ConditionTrue: // When using old Cluster API condition utils, for conditions with Status true, Reason can be empty only when a condition has positive polarity (means "good"). + c.Reason = NoReasonReported + case metav1.ConditionUnknown: + return errors.New("condition must be set when a condition is unknown") + } + } + + // NOTE: Empty LastTransitionTime is tolerated because it will be set when assigning the newly generated mirror condition to an object. + // NOTE: Others metav1.Condition validations rules, e.g. regex, are not enforced at this stage; they will be enforced by the API server at a later stage. + + return nil +} diff --git a/util/conditions/experimental/getter_test.go b/util/conditions/v1beta2/getter_test.go similarity index 64% rename from util/conditions/experimental/getter_test.go rename to util/conditions/v1beta2/getter_test.go index dac4680a91dd..3853c46c62c4 100644 --- a/util/conditions/experimental/getter_test.go +++ b/util/conditions/v1beta2/getter_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "testing" @@ -62,16 +62,18 @@ func (f *ObjWithWrongConditionType) DeepCopyObject() runtime.Object { panic("implement me") } -type ObjWithWrongExperimentalConditionType struct { +type ObjWithWrongV1beta2ConditionType struct { metav1.TypeMeta metav1.ObjectMeta Status struct { - Conditions clusterv1.Conditions - ExperimentalConditions []string + Conditions clusterv1.Conditions + V1Beta2 struct { + Conditions []string + } } } -func (f *ObjWithWrongExperimentalConditionType) DeepCopyObject() runtime.Object { +func (f *ObjWithWrongV1beta2ConditionType) DeepCopyObject() runtime.Object { panic("implement me") } @@ -112,7 +114,7 @@ func TestGetAll(t *testing.T) { g.Expect(err).To(HaveOccurred()) }) - t.Run("fails for object with status without conditions or experimental conditions", func(t *testing.T) { + t.Run("fails for object with status without conditions or v1beta2 conditions", func(t *testing.T) { g := NewWithT(t) foo := &ObjWithStatusWithoutConditions{} @@ -126,26 +128,6 @@ func TestGetAll(t *testing.T) { g.Expect(err).To(HaveOccurred()) }) - t.Run("fails for object with wrong condition type", func(t *testing.T) { - g := NewWithT(t) - foo := &ObjWithWrongConditionType{} - - _, err := GetAll(foo) - g.Expect(err).To(HaveOccurred()) - - // TODO: think about how unstructured can detect wrong type - }) - - t.Run("fails for object with wrong experimental condition type", func(t *testing.T) { - g := NewWithT(t) - foo := &ObjWithWrongExperimentalConditionType{} - - _, err := GetAll(foo) - g.Expect(err).To(HaveOccurred()) - - // TODO: think about how unstructured can detect wrong type - }) - t.Run("v1beta object with legacy conditions", func(t *testing.T) { g := NewWithT(t) foo := &builder.Phase0Obj{ @@ -172,6 +154,7 @@ func TestGetAll(t *testing.T) { Type: string(foo.Status.Conditions[0].Type), Status: metav1.ConditionStatus(foo.Status.Conditions[0].Status), LastTransitionTime: foo.Status.Conditions[0].LastTransitionTime, + Reason: NoReasonReported, }, { Type: string(foo.Status.Conditions[1].Type), @@ -194,7 +177,7 @@ func TestGetAll(t *testing.T) { g.Expect(got).To(MatchConditions(expect), cmp.Diff(got, expect)) }) - t.Run("v1beta1 object with both legacy and experimental conditions", func(t *testing.T) { + t.Run("v1beta1 object with both legacy and v1beta2 conditions", func(t *testing.T) { g := NewWithT(t) foo := &builder.Phase1Obj{ Status: builder.Phase1ObjStatus{ @@ -205,14 +188,16 @@ func TestGetAll(t *testing.T) { LastTransitionTime: now, }, }, - ExperimentalConditions: []metav1.Condition{ - { - Type: "fooCondition", - Status: metav1.ConditionTrue, - ObservedGeneration: 10, - LastTransitionTime: now, - Reason: "FooReason", - Message: "FooMessage", + V1Beta2: builder.Phase1ObjStatusV1beta2{ + Conditions: []metav1.Condition{ + { + Type: "fooCondition", + Status: metav1.ConditionTrue, + ObservedGeneration: 10, + LastTransitionTime: now, + Reason: "FooReason", + Message: "FooMessage", + }, }, }, }, @@ -220,12 +205,12 @@ func TestGetAll(t *testing.T) { expect := []metav1.Condition{ { - Type: foo.Status.ExperimentalConditions[0].Type, - Status: foo.Status.ExperimentalConditions[0].Status, - LastTransitionTime: foo.Status.ExperimentalConditions[0].LastTransitionTime, - ObservedGeneration: foo.Status.ExperimentalConditions[0].ObservedGeneration, - Reason: foo.Status.ExperimentalConditions[0].Reason, - Message: foo.Status.ExperimentalConditions[0].Message, + Type: foo.Status.V1Beta2.Conditions[0].Type, + Status: foo.Status.V1Beta2.Conditions[0].Status, + LastTransitionTime: foo.Status.V1Beta2.Conditions[0].LastTransitionTime, + ObservedGeneration: foo.Status.V1Beta2.Conditions[0].ObservedGeneration, + Reason: foo.Status.V1Beta2.Conditions[0].Reason, + Message: foo.Status.V1Beta2.Conditions[0].Message, }, } @@ -250,14 +235,17 @@ func TestGetAll(t *testing.T) { Type: "fooCondition", Status: metav1.ConditionTrue, LastTransitionTime: now, + Reason: "fooReason", }, }, - BackCompatibility: builder.Phase2ObjStatusBackCompatibility{ - Conditions: clusterv1.Conditions{ - { - Type: "barCondition", - Status: corev1.ConditionFalse, - LastTransitionTime: now, + Deprecated: builder.Phase2ObjStatusDeprecated{ + V1Beta1: builder.Phase2ObjStatusDeprecatedV1Beta2{ + Conditions: clusterv1.Conditions{ + { + Type: "barCondition", + Status: corev1.ConditionFalse, + LastTransitionTime: now, + }, }, }, }, @@ -269,6 +257,7 @@ func TestGetAll(t *testing.T) { Type: foo.Status.Conditions[0].Type, Status: foo.Status.Conditions[0].Status, LastTransitionTime: foo.Status.Conditions[0].LastTransitionTime, + Reason: foo.Status.Conditions[0].Reason, }, } @@ -293,6 +282,7 @@ func TestGetAll(t *testing.T) { Type: "fooCondition", Status: metav1.ConditionTrue, LastTransitionTime: now, + Reason: "fooReason", }, }, }, @@ -303,6 +293,7 @@ func TestGetAll(t *testing.T) { Type: foo.Status.Conditions[0].Type, Status: foo.Status.Conditions[0].Status, LastTransitionTime: foo.Status.Conditions[0].LastTransitionTime, + Reason: foo.Status.Conditions[0].Reason, }, } @@ -318,3 +309,86 @@ func TestGetAll(t *testing.T) { g.Expect(got).To(MatchConditions(expect), cmp.Diff(got, expect)) }) } + +func TestConvertFromV1Beta1Conditions(t *testing.T) { + tests := []struct { + name string + conditions []clusterv1.Condition + want []metav1.Condition + wantError bool + }{ + { + name: "Fails if Type is missing", + conditions: clusterv1.Conditions{ + clusterv1.Condition{Status: corev1.ConditionTrue}, + }, + wantError: true, + }, + { + name: "Fails if Status is missing", + conditions: clusterv1.Conditions{ + clusterv1.Condition{Type: clusterv1.ConditionType("foo")}, + }, + wantError: true, + }, + { + name: "Defaults reason for positive polarity", + conditions: clusterv1.Conditions{ + clusterv1.Condition{Type: clusterv1.ConditionType("foo"), Status: corev1.ConditionTrue}, + }, + wantError: false, + want: []metav1.Condition{ + { + Type: "foo", + Status: metav1.ConditionTrue, + Reason: NoReasonReported, + }, + }, + }, + { + name: "Defaults reason for negative polarity", + conditions: clusterv1.Conditions{ + clusterv1.Condition{Type: clusterv1.ConditionType("foo"), Status: corev1.ConditionFalse}, + }, + wantError: false, + want: []metav1.Condition{ + { + Type: "foo", + Status: metav1.ConditionFalse, + Reason: NoReasonReported, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := convertFromV1Beta1Conditions(tt.conditions) + if tt.wantError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + g.Expect(got).To(Equal(tt.want), cmp.Diff(tt.want, got)) + }) + t.Run(tt.name+" - unstructured", func(t *testing.T) { + g := NewWithT(t) + + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&builder.Phase0Obj{Status: builder.Phase0ObjStatus{Conditions: tt.conditions}}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(unstructuredObj).To(HaveKey("status")) + unstructuredStatusObj := unstructuredObj["status"].(map[string]interface{}) + g.Expect(unstructuredStatusObj).To(HaveKey("conditions")) + + got, err := convertFromUnstructuredConditions(unstructuredStatusObj["conditions"].([]interface{})) + if tt.wantError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + g.Expect(got).To(Equal(tt.want), cmp.Diff(tt.want, got)) + }) + } +} diff --git a/util/conditions/experimental/matcher.go b/util/conditions/v1beta2/matcher.go similarity index 62% rename from util/conditions/experimental/matcher.go rename to util/conditions/v1beta2/matcher.go index 72a9dd5db883..12927a2c5c0b 100644 --- a/util/conditions/experimental/matcher.go +++ b/util/conditions/v1beta2/matcher.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "fmt" @@ -24,21 +24,51 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// MatchConditions returns a custom matcher to check equality of clusterv1.Conditions. -func MatchConditions(expected []metav1.Condition) types.GomegaMatcher { +// IgnoreLastTransitionTime instruct MatchConditions and MatchCondition to ignore the LastTransitionTime field. +type IgnoreLastTransitionTime bool + +// ApplyMatch applies this configuration to the given Match options. +func (f IgnoreLastTransitionTime) ApplyMatch(opts *MatchOptions) { + opts.ignoreLastTransitionTime = bool(f) +} + +// MatchOption is some configuration that modifies options for a match request. +type MatchOption interface { + // ApplyMatch applies this configuration to the given match options. + ApplyMatch(option *MatchOptions) +} + +// MatchOptions allows to set options for the match operation. +type MatchOptions struct { + ignoreLastTransitionTime bool +} + +// ApplyOptions applies the given list options on these options, +// and then returns itself (for convenient chaining). +func (o *MatchOptions) ApplyOptions(opts []MatchOption) *MatchOptions { + for _, opt := range opts { + opt.ApplyMatch(o) + } + return o +} + +// MatchConditions returns a custom matcher to check equality of []metav1.Condition. +func MatchConditions(expected []metav1.Condition, opts ...MatchOption) types.GomegaMatcher { return &matchConditions{ + opts: opts, expected: expected, } } type matchConditions struct { + opts []MatchOption expected []metav1.Condition } func (m matchConditions) Match(actual interface{}) (success bool, err error) { elems := []interface{}{} for _, condition := range m.expected { - elems = append(elems, MatchCondition(condition)) + elems = append(elems, MatchCondition(condition, m.opts...)) } return gomega.ConsistOf(elems...).Match(actual) @@ -52,18 +82,25 @@ func (m matchConditions) NegatedFailureMessage(actual interface{}) (message stri return fmt.Sprintf("expected\n\t%#v\nto not match\n\t%#v\n", actual, m.expected) } -// MatchCondition returns a custom matcher to check equality of clusterv1.Condition. -func MatchCondition(expected metav1.Condition) types.GomegaMatcher { +// MatchCondition returns a custom matcher to check equality of []metav1.Condition. +func MatchCondition(expected metav1.Condition, opts ...MatchOption) types.GomegaMatcher { return &matchCondition{ + opts: opts, expected: expected, } } type matchCondition struct { + opts []MatchOption expected metav1.Condition } func (m matchCondition) Match(actual interface{}) (success bool, err error) { + matchOpt := &MatchOptions{ + ignoreLastTransitionTime: false, + } + matchOpt.ApplyOptions(m.opts) + actualCondition, ok := actual.(metav1.Condition) if !ok { return false, fmt.Errorf("actual should be of type metav1.Condition") @@ -90,6 +127,13 @@ func (m matchCondition) Match(actual interface{}) (success bool, err error) { return ok, err } + if !matchOpt.ignoreLastTransitionTime { + ok, err = gomega.Equal(m.expected.LastTransitionTime).Match(actualCondition.LastTransitionTime) + if !ok { + return ok, err + } + } + return ok, err } diff --git a/util/conditions/experimental/matcher_test.go b/util/conditions/v1beta2/matcher_test.go similarity index 78% rename from util/conditions/experimental/matcher_test.go rename to util/conditions/v1beta2/matcher_test.go index cd45ecc0b4d5..d51496ae9300 100644 --- a/util/conditions/experimental/matcher_test.go +++ b/util/conditions/v1beta2/matcher_test.go @@ -14,16 +14,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "testing" + "time" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestMatchConditions(t *testing.T) { + t0 := metav1.Now() + testCases := []struct { name string actual interface{} @@ -42,7 +45,7 @@ func TestMatchConditions(t *testing.T) { { Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, @@ -51,7 +54,7 @@ func TestMatchConditions(t *testing.T) { { Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, @@ -64,14 +67,14 @@ func TestMatchConditions(t *testing.T) { { Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, { Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, @@ -80,14 +83,14 @@ func TestMatchConditions(t *testing.T) { { Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, { Type: "different", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "different", Message: "different", }, @@ -100,14 +103,14 @@ func TestMatchConditions(t *testing.T) { { Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, { Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, @@ -116,7 +119,7 @@ func TestMatchConditions(t *testing.T) { { Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, @@ -138,16 +141,21 @@ func TestMatchConditions(t *testing.T) { } func TestMatchCondition(t *testing.T) { + t0 := metav1.Now() + t1 := metav1.NewTime(t0.Add(1 * -time.Minute)) + testCases := []struct { name string actual interface{} expected metav1.Condition + options []MatchOption expectMatch bool }{ { name: "with an empty condition", actual: metav1.Condition{}, expected: metav1.Condition{}, + options: []MatchOption{}, expectMatch: true, }, { @@ -155,17 +163,18 @@ func TestMatchCondition(t *testing.T) { actual: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{}, + LastTransitionTime: t0, Reason: "reason", Message: "message", }, expected: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{}, + LastTransitionTime: t0, Reason: "reason", Message: "message", }, + options: []MatchOption{}, expectMatch: true, }, { @@ -173,17 +182,37 @@ func TestMatchCondition(t *testing.T) { actual: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, + Reason: "reason", + Message: "message", + }, + expected: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: t1, + Reason: "reason", + Message: "message", + }, + options: []MatchOption{}, + expectMatch: false, + }, + { + name: "with a different LastTransitionTime but with IgnoreLastTransitionTime", + actual: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: t0, Reason: "reason", Message: "message", }, expected: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{}, + LastTransitionTime: t1, Reason: "reason", Message: "message", }, + options: []MatchOption{IgnoreLastTransitionTime(true)}, expectMatch: true, }, { @@ -191,17 +220,18 @@ func TestMatchCondition(t *testing.T) { actual: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, expected: metav1.Condition{ Type: "different", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, + options: []MatchOption{}, expectMatch: false, }, { @@ -209,17 +239,18 @@ func TestMatchCondition(t *testing.T) { actual: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, expected: metav1.Condition{ Type: "type", Status: metav1.ConditionFalse, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, + options: []MatchOption{}, expectMatch: false, }, { @@ -227,7 +258,7 @@ func TestMatchCondition(t *testing.T) { actual: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", ObservedGeneration: 1, @@ -235,11 +266,12 @@ func TestMatchCondition(t *testing.T) { expected: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", ObservedGeneration: 2, }, + options: []MatchOption{}, expectMatch: false, }, { @@ -247,17 +279,18 @@ func TestMatchCondition(t *testing.T) { actual: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, expected: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "different", Message: "message", }, + options: []MatchOption{}, expectMatch: false, }, { @@ -265,17 +298,18 @@ func TestMatchCondition(t *testing.T) { actual: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "message", }, expected: metav1.Condition{ Type: "type", Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), + LastTransitionTime: t0, Reason: "reason", Message: "different", }, + options: []MatchOption{}, expectMatch: false, }, } @@ -284,9 +318,9 @@ func TestMatchCondition(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) if tc.expectMatch { - g.Expect(tc.actual).To(MatchCondition(tc.expected)) + g.Expect(tc.actual).To(MatchCondition(tc.expected, tc.options...)) } else { - g.Expect(tc.actual).ToNot(MatchCondition(tc.expected)) + g.Expect(tc.actual).ToNot(MatchCondition(tc.expected, tc.options...)) } }) } diff --git a/util/conditions/experimental/merge_strategies.go b/util/conditions/v1beta2/merge_strategies.go similarity index 66% rename from util/conditions/experimental/merge_strategies.go rename to util/conditions/v1beta2/merge_strategies.go index 96aee11720c1..f20e2b915fab 100644 --- a/util/conditions/experimental/merge_strategies.go +++ b/util/conditions/v1beta2/merge_strategies.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "fmt" @@ -23,21 +23,25 @@ import ( "strings" "github.com/gobuffalo/flect" + "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/klog/v2" ) +// TODO: Move to the API package. const ( - // ManyIssuesReason is set on conditions generated during aggregate or summary operations when many conditions/objects are reporting issues. - ManyIssuesReason = "MoreThanOneIssuesReported" + // MultipleIssuesReason is set on conditions generated during aggregate or summary operations when multiple conditions/objects are reporting issues. + // NOTE: If a custom merge strategy is used for the aggregate or summary operations, this might not be true anymore. + MultipleIssuesReason = "MultipleIssuesReported" - // ManyUnknownsReason is set on conditions generated during aggregate or summary operations when many conditions/objects are reporting unknown. - ManyUnknownsReason = "MoreThanOneUnknownReported" + // MultipleUnknownReported is set on conditions generated during aggregate or summary operations when multiple conditions/objects are reporting unknown. + // NOTE: If a custom merge strategy is used for the aggregate or summary operations, this might not be true anymore. + MultipleUnknownReported = "MultipleUnknownReported" - // ManyInfoReason is set on conditions generated during aggregate or summary operations when many conditions/objects are reporting info. - ManyInfoReason = "MoreThanOneInfoReported" + // MultipleInfoReason is set on conditions generated during aggregate or summary operations when multiple conditions/objects are reporting info. + // NOTE: If a custom merge strategy is used for the aggregate or summary operations, this might not be true anymore. + MultipleInfoReason = "MultipleInfoReported" ) // MergeStrategy defines a strategy used to merge conditions during the aggregate or summary operation. @@ -50,12 +54,12 @@ type MergeStrategy interface { // The list of conditionTypes has an implicit order; it is up to the implementation of merge to use this info or not. // If negativeConditionTypes are in scope, the implementation of merge should treat them accordingly. // - // It required, the implementation of merge must add a stepCounter to the output message. + // If stepCounter is true, the implementation of merge must add info about step progress to the output message. Merge(conditions []ConditionWithOwnerInfo, conditionTypes []string, negativeConditionTypes sets.Set[string], stepCounter bool) (status metav1.ConditionStatus, reason, message string, err error) } -// ConditionWithOwnerInfo is a wrapper metav1.Condition with addition ConditionOwnerInfo. -// Those info can be used when generating the message resulting from the merge operation. +// ConditionWithOwnerInfo is a wrapper around metav1.Condition with additional ConditionOwnerInfo. +// These infos can be used when generating the message resulting from the merge operation. type ConditionWithOwnerInfo struct { OwnerResource ConditionOwnerInfo metav1.Condition @@ -63,9 +67,13 @@ type ConditionWithOwnerInfo struct { // ConditionOwnerInfo contains info about the object that owns the condition. type ConditionOwnerInfo struct { - metav1.TypeMeta - Namespace string - Name string + Kind string + Name string +} + +// String returns a string representation of the ConditionOwnerInfo. +func (o *ConditionOwnerInfo) String() string { + return fmt.Sprintf("%s %s", o.Kind, o.Name) } // defaultMergeStrategy defines the default merge strategy for Cluster API conditions. @@ -83,18 +91,22 @@ func newDefaultMergeStrategy() MergeStrategy { return &defaultMergeStrategy{} } -// Merge all the condition in input based on a strategy that surfaces issue first, then unknown conditions, the info (if none of issues and unknown condition exists). +// Merge all conditions in input based on a strategy that surfaces issues first, then unknown conditions, then info (if none of issues and unknown condition exists). // - issues: conditions with positive polarity (normal True) and status False or conditions with negative polarity (normal False) and status True. // - unknown: conditions with status unknown. // - info: conditions with positive polarity (normal True) and status True or conditions with negative polarity (normal False) and status False. func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, conditionTypes []string, negativeConditionTypes sets.Set[string], stepCounter bool) (status metav1.ConditionStatus, reason, message string, err error) { + if len(conditions) == 0 { + return "", "", "", errors.New("can't merge an empty list of conditions") + } + // Infer which operation is calling this func, so it is possible to use different strategies for computing the message for the target condition. // - When merge should consider a single condition type, we can assume this func is called within an aggregate operation // (Aggregate should merge the same condition across many objects) isAggregateOperation := len(conditionTypes) == 1 // - Otherwise we can assume this func is called within an summary operation - // (Aggregate should merge the same condition across many objects) + // (Summary should merge different conditions from the same object) isSummaryOperation := !isAggregateOperation // sortConditions the relevance defined by the users (the order of condition types), LastTransition time (older first). @@ -104,9 +116,9 @@ func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, condit // Compute the status for the target condition: // Note: This function always return a condition with positive polarity. - // - If the top group are issues, use false - // - If the top group are unknown, use unknown - // - If the top group are info, use true + // - if there are issues, use false + // - else if there are unknown, use unknown + // - else if there are info, use true switch { case len(issueConditions) > 0: status = metav1.ConditionFalse @@ -114,6 +126,9 @@ func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, condit status = metav1.ConditionUnknown case len(infoConditions) > 0: status = metav1.ConditionTrue + default: + // NOTE: this is already handled above, but repeating also here for better readability. + return "", "", "", errors.New("can't merge an empty list of conditions") } // Compute the reason for the target condition: @@ -123,20 +138,23 @@ func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, condit case len(issueConditions) == 1: reason = issueConditions[0].Reason case len(issueConditions) > 1: - reason = ManyIssuesReason + reason = MultipleIssuesReason case len(unknownConditions) == 1: reason = unknownConditions[0].Reason case len(unknownConditions) > 1: - reason = ManyUnknownsReason + reason = MultipleUnknownReported case len(infoConditions) == 1: reason = infoConditions[0].Reason case len(infoConditions) > 1: - reason = ManyInfoReason + reason = MultipleInfoReason + default: + // NOTE: this is already handled above, but repeating also here for better readability. + return "", "", "", errors.New("can't merge an empty list of conditions") } // Compute the message for the target condition, which is optimized for the operation being performed. - // When performing the summary operation, usually we are merging a small set of conditions form the same object, + // When performing the summary operation, usually we are merging a small set of conditions from the same object, // Considering the small number of conditions, involved it is acceptable/preferred to provide as much detail // as possible about the messages from the conditions being merged. // @@ -152,7 +170,7 @@ func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, condit for _, condition := range append(issueConditions, append(unknownConditions, infoConditions...)...) { priority := getPriority(condition.Condition, negativeConditionTypes) if priority == infoMergePriority { - // Drop info messages when we are surfacing issues on unknown. + // Drop info messages when we are surfacing issues or unknown. if status != metav1.ConditionTrue { continue } @@ -191,53 +209,34 @@ func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, condit // For each message it is reported a list of max 3 objects reporting the message; if more objects are reporting the same // message, the number of those objects is surfaced. // - // e.g. (False): Message-1 from default/obj0, default/obj1, default/obj2 and 2 other Objects + // e.g. (False): Message-1 from obj0, obj1, obj2 and 2 more Objects // // If there are other objects - objects not included in the list above - reporting issues/unknown (or info there no issues/unknown), // the number of those objects is surfaced. // - // e.g. ...; other 2 Objects with issues; other 1 Objects unknown + // e.g. ...; 2 more Objects with issues; 1 more Objects with unknown status // if isAggregateOperation { - // Gets the kind to be used when composing messages. - // NOTE: This assume all the objects in an aggregate operation are of the same kind. - kind := "" - if len(conditions) > 0 { - kind = flect.Pluralize(conditions[0].OwnerResource.Kind) - } - n := 3 messages := []string{} // Get max n issue messages, decrement n, and track if there are other objects reporting issues not included in the messages. if len(issueConditions) > 0 { - issueMessages, otherIssues := aggregateMessages(issueConditions, &n, kind, false) - + issueMessages := aggregateMessages(issueConditions, &n, false, "with other issues") messages = append(messages, issueMessages...) - if otherIssues > 0 { - messages = append(messages, fmt.Sprintf("other %d %s with issues", otherIssues, kind)) - } } // Get max n unknown messages, decrement n, and track if there are other objects reporting unknown not included in the messages. if len(unknownConditions) > 0 { - unknownMessages, otherUnknown := aggregateMessages(unknownConditions, &n, kind, false) - + unknownMessages := aggregateMessages(unknownConditions, &n, false, "with status unknown") messages = append(messages, unknownMessages...) - if otherUnknown > 0 { - messages = append(messages, fmt.Sprintf("other %d %s unknown", otherUnknown, kind)) - } } // Only if there are no issue or unknown, // Get max n info messages, decrement n, and track if there are other objects reporting info not included in the messages. if len(issueConditions) == 0 && len(unknownConditions) == 0 && len(infoConditions) > 0 { - infoMessages, infoUnknown := aggregateMessages(infoConditions, &n, kind, true) - + infoMessages := aggregateMessages(infoConditions, &n, true, "with additional info") messages = append(messages, infoMessages...) - if infoUnknown > 0 { - messages = append(messages, fmt.Sprintf("other %d %s with info messages", infoUnknown, kind)) - } } message = strings.Join(messages, "; ") @@ -282,7 +281,7 @@ func splitConditionsByPriority(conditions []ConditionWithOwnerInfo, negativePola } // getPriority returns the merge priority for each condition. -// The default implementation identifies as: +// It assign to conditions the following priority values: // - issues: conditions with positive polarity (normal True) and status False or conditions with negative polarity (normal False) and status True. // - unknown: conditions with status unknown. // - info: conditions with positive polarity (normal True) and status True or conditions with negative polarity (normal False) and status False. @@ -307,60 +306,83 @@ func getPriority(condition metav1.Condition, negativePolarityConditionTypes sets } // aggregateMessages return messages for the aggregate operation. -func aggregateMessages(conditions []ConditionWithOwnerInfo, n *int, kind string, dropEmpty bool) (messages []string, other int) { +func aggregateMessages(conditions []ConditionWithOwnerInfo, n *int, dropEmpty bool, otherMessage string) (messages []string) { // create a map with all the messages and the list of objects reporting the same message. - messageObjMap := map[string][]string{} + messageObjMap := map[string]map[string][]string{} for _, condition := range conditions { if dropEmpty && condition.Message == "" { continue } - m := fmt.Sprintf("(%s)", condition.Status) - if condition.Message != "" { - m += fmt.Sprintf(": %s", condition.Message) - } - if _, ok := messageObjMap[m]; !ok { - messageObjMap[m] = []string{} + m := condition.Message + if _, ok := messageObjMap[condition.OwnerResource.Kind]; !ok { + messageObjMap[condition.OwnerResource.Kind] = map[string][]string{} } - messageObjMap[m] = append(messageObjMap[m], klog.KRef(condition.OwnerResource.Namespace, condition.OwnerResource.Name).String()) + messageObjMap[condition.OwnerResource.Kind][m] = append(messageObjMap[condition.OwnerResource.Kind][m], condition.OwnerResource.Name) } - // compute the order of messages according to the number of objects reporting the same message. - // Note: The message text is used as a secondary criteria to sort messages with the same number of objects. - messageIndex := make([]string, 0, len(messageObjMap)) - for m := range messageObjMap { - messageIndex = append(messageIndex, m) + // Gets the objects kind (with a stable order). + kinds := make([]string, 0, len(messageObjMap)) + for kind := range messageObjMap { + kinds = append(kinds, kind) } + sort.Strings(kinds) + + // Aggregate messages for each object kind. + for _, kind := range kinds { + kindPlural := flect.Pluralize(kind) + messageObjMapForKind := messageObjMap[kind] + + // compute the order of messages according to the number of objects reporting the same message. + // Note: The message text is used as a secondary criteria to sort messages with the same number of objects. + messageIndex := make([]string, 0, len(messageObjMapForKind)) + for m := range messageObjMapForKind { + messageIndex = append(messageIndex, m) + } - sort.SliceStable(messageIndex, func(i, j int) bool { - return len(messageObjMap[messageIndex[i]]) > len(messageObjMap[messageIndex[j]]) || - (len(messageObjMap[messageIndex[i]]) == len(messageObjMap[messageIndex[j]]) && messageIndex[i] < messageIndex[j]) - }) + sort.SliceStable(messageIndex, func(i, j int) bool { + return len(messageObjMapForKind[messageIndex[i]]) > len(messageObjMapForKind[messageIndex[j]]) || + (len(messageObjMapForKind[messageIndex[i]]) == len(messageObjMapForKind[messageIndex[j]]) && messageIndex[i] < messageIndex[j]) + }) - // Pick the first n messages, decrement n. - // For each message, add up to three objects; if more add the number of the remaining objects with the same message. - // Count the number of objects reporting messages not included in the above. - // Note: we are showing up to three objects because usually control plane has 3 machines, and we want to show all issues - // to control plane machines if any, - for i := range len(messageIndex) { - if *n == 0 { - other += len(messageObjMap[messageIndex[i]]) - continue - } + // Pick the first n messages, decrement n. + // For each message, add up to three objects; if more add the number of the remaining objects with the same message. + // Count the number of objects reporting messages not included in the above. + // Note: we are showing up to three objects because usually control plane has 3 machines, and we want to show all issues + // to control plane machines if any, + var other = 0 + for _, m := range messageIndex { + if *n == 0 { + other += len(messageObjMapForKind[m]) + continue + } + + msg := m + allObjects := messageObjMapForKind[m] + switch { + case len(allObjects) == 0: + // This should never happen, entry in the map exists only when an object report a message. + case len(allObjects) == 1: + msg += fmt.Sprintf(" from %s %s", kind, strings.Join(allObjects, ", ")) + case len(allObjects) <= 3: + msg += fmt.Sprintf(" from %s %s", kindPlural, strings.Join(allObjects, ", ")) + default: + msg += fmt.Sprintf(" from %s %s and %d more", kindPlural, strings.Join(allObjects[:3], ", "), len(allObjects)-3) + } - m := messageIndex[i] - allObjects := messageObjMap[messageIndex[i]] - if len(allObjects) <= 3 { - m += fmt.Sprintf(" from %s", strings.Join(allObjects, ", ")) - } else { - m += fmt.Sprintf(" from %s and %d other %s", strings.Join(allObjects[:3], ", "), len(allObjects)-3, kind) + messages = append(messages, msg) + *n-- } - messages = append(messages, m) - *n-- + if other == 1 { + messages = append(messages, fmt.Sprintf("%d %s %s", other, kind, otherMessage)) + } + if other > 1 { + messages = append(messages, fmt.Sprintf("%d %s %s", other, kindPlural, otherMessage)) + } } - return messages, other + return messages } // getConditionsWithOwnerInfo return all the conditions from an object each one with the corresponding ConditionOwnerInfo. @@ -370,9 +392,10 @@ func getConditionsWithOwnerInfo(obj runtime.Object) ([]ConditionWithOwnerInfo, e if err != nil { return nil, err } + ownerInfo := getConditionOwnerInfo(obj) for _, condition := range conditions { ret = append(ret, ConditionWithOwnerInfo{ - OwnerResource: getConditionOwnerInfo(obj), + OwnerResource: ownerInfo, Condition: condition, }) } @@ -380,10 +403,12 @@ func getConditionsWithOwnerInfo(obj runtime.Object) ([]ConditionWithOwnerInfo, e } // getConditionOwnerInfo return the ConditionOwnerInfo for the given object. +// Note: Given that controller runtime often does not set typeMeta for objects, +// in case kind is missing we are falling back to the type name, which in most cases +// is the same of kind. func getConditionOwnerInfo(obj runtime.Object) ConditionOwnerInfo { - var apiVersion, kind, name, namespace string + var kind, name string kind = obj.GetObjectKind().GroupVersionKind().Kind - apiVersion = obj.GetObjectKind().GroupVersionKind().GroupVersion().String() if kind == "" { t := reflect.TypeOf(obj) @@ -394,15 +419,10 @@ func getConditionOwnerInfo(obj runtime.Object) ConditionOwnerInfo { if objMeta, ok := obj.(metav1.Object); ok { name = objMeta.GetName() - namespace = objMeta.GetNamespace() } return ConditionOwnerInfo{ - TypeMeta: metav1.TypeMeta{ - Kind: kind, - APIVersion: apiVersion, - }, - Namespace: namespace, - Name: name, + Kind: kind, + Name: name, } } diff --git a/util/conditions/experimental/merge_strategies_test.go b/util/conditions/v1beta2/merge_strategies_test.go similarity index 78% rename from util/conditions/experimental/merge_strategies_test.go rename to util/conditions/v1beta2/merge_strategies_test.go index b541037a1253..c216016528b0 100644 --- a/util/conditions/experimental/merge_strategies_test.go +++ b/util/conditions/v1beta2/merge_strategies_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "testing" @@ -29,27 +29,31 @@ func TestAggregateMessages(t *testing.T) { g := NewWithT(t) conditions := []ConditionWithOwnerInfo{ - {OwnerResource: ConditionOwnerInfo{Name: "obj1"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, - {OwnerResource: ConditionOwnerInfo{Name: "obj2"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, - {OwnerResource: ConditionOwnerInfo{Name: "obj3"}, Condition: metav1.Condition{Type: "A", Message: "Message-2", Status: metav1.ConditionFalse}}, - {OwnerResource: ConditionOwnerInfo{Name: "obj4"}, Condition: metav1.Condition{Type: "A", Message: "Message-2", Status: metav1.ConditionFalse}}, - {OwnerResource: ConditionOwnerInfo{Name: "obj5"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, - {OwnerResource: ConditionOwnerInfo{Name: "obj6"}, Condition: metav1.Condition{Type: "A", Message: "Message-3", Status: metav1.ConditionFalse}}, - {OwnerResource: ConditionOwnerInfo{Name: "obj7"}, Condition: metav1.Condition{Type: "A", Message: "Message-4", Status: metav1.ConditionFalse}}, - {OwnerResource: ConditionOwnerInfo{Name: "obj8"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, - {OwnerResource: ConditionOwnerInfo{Name: "obj9"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj01"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj02"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj03"}, Condition: metav1.Condition{Type: "A", Message: "Message-2", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj04"}, Condition: metav1.Condition{Type: "A", Message: "Message-2", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj05"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj06"}, Condition: metav1.Condition{Type: "A", Message: "Message-3", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj07"}, Condition: metav1.Condition{Type: "A", Message: "Message-4", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj08"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj09"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineDeployment", Name: "obj10"}, Condition: metav1.Condition{Type: "A", Message: "Message-5", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineSet", Name: "obj11"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, + {OwnerResource: ConditionOwnerInfo{Kind: "MachineSet", Name: "obj12"}, Condition: metav1.Condition{Type: "A", Message: "Message-1", Status: metav1.ConditionFalse}}, } n := 3 - messages, others := aggregateMessages(conditions, &n, "objects", false) + messages := aggregateMessages(conditions, &n, false, "with other issues") g.Expect(n).To(Equal(0)) g.Expect(messages).To(Equal([]string{ - "(False): Message-1 from obj1, obj2, obj5 and 2 other objects", - "(False): Message-2 from obj3, obj4", - "(False): Message-3 from obj6", + "Message-1 from MachineDeployments obj01, obj02, obj05 and 2 more", // MachineDeployments obj08, obj09 + "Message-2 from MachineDeployments obj03, obj04", + "Message-3 from MachineDeployment obj06", + "2 MachineDeployments with other issues", // MachineDeployments obj07 (Message-4), obj10 (Message-5) + "2 MachineSets with other issues", // MachineSet obj11, obj12 (Message-1) })) - g.Expect(others).To(Equal(1)) } func TestSortConditions(t *testing.T) { diff --git a/util/conditions/experimental/mirror.go b/util/conditions/v1beta2/mirror.go similarity index 62% rename from util/conditions/experimental/mirror.go rename to util/conditions/v1beta2/mirror.go index 9501a615b4c8..9e14dfcb3eb4 100644 --- a/util/conditions/experimental/mirror.go +++ b/util/conditions/v1beta2/mirror.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "fmt" @@ -22,22 +22,22 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/klog/v2" ) // NotYetReportedReason is set on missing conditions generated during mirror, aggregate or summary operations. // Missing conditions are generated during the above operations when an expected condition does not exist on a object. +// TODO: Move to the API package. const NotYetReportedReason = "NotYetReported" -// MirrorOption is some configuration that modifies options for a mirrorInto request. +// MirrorOption is some configuration that modifies options for a mirror call. type MirrorOption interface { - // ApplyToMirror applies this configuration to the given mirrorInto options. + // ApplyToMirror applies this configuration to the given mirror options. ApplyToMirror(*MirrorOptions) } -// MirrorOptions allows to set options for the mirrorInto operation. +// MirrorOptions allows to set options for the mirror operation. type MirrorOptions struct { - overrideType string + targetConditionType string } // ApplyOptions applies the given list options on these options, @@ -53,53 +53,48 @@ func (o *MirrorOptions) ApplyOptions(opts []MirrorOption) *MirrorOptions { // a new condition with status Unknown, reason NotYetReported is created. // // By default, the Mirror condition has the same type of the source condition, but this can be changed by using -// the OverrideType option. -func NewMirrorCondition(obj runtime.Object, conditionType string, opts ...MirrorOption) (*metav1.Condition, error) { - mirrorOpt := &MirrorOptions{} +// the TargetConditionType option. +func NewMirrorCondition(sourceObj runtime.Object, sourceConditionType string, opts ...MirrorOption) (*metav1.Condition, error) { + mirrorOpt := &MirrorOptions{ + targetConditionType: sourceConditionType, + } mirrorOpt.ApplyOptions(opts) - conditions, err := GetAll(obj) + conditions, err := GetAll(sourceObj) if err != nil { return nil, err } - conditionOwner := getConditionOwnerInfo(obj) - conditionOwnerString := strings.TrimSpace(fmt.Sprintf("%s %s", conditionOwner.Kind, klog.KRef(conditionOwner.Namespace, conditionOwner.Name))) + conditionOwner := getConditionOwnerInfo(sourceObj) + conditionOwnerString := fmt.Sprintf("%s %s", conditionOwner.Kind, conditionOwner.Name) - var c *metav1.Condition for i := range conditions { - if conditions[i].Type == conditionType { - c = &metav1.Condition{ - Type: conditions[i].Type, + if conditions[i].Type == sourceConditionType { + return &metav1.Condition{ + Type: mirrorOpt.targetConditionType, Status: conditions[i].Status, ObservedGeneration: conditions[i].ObservedGeneration, LastTransitionTime: conditions[i].LastTransitionTime, Reason: conditions[i].Reason, Message: strings.TrimSpace(fmt.Sprintf("%s (from %s)", conditions[i].Message, conditionOwnerString)), - } - } - } - - if c == nil { - c = &metav1.Condition{ - Type: conditionType, - Status: metav1.ConditionUnknown, - LastTransitionTime: metav1.Now(), - Reason: NotYetReportedReason, - Message: fmt.Sprintf("Condition %s not yet reported from %s", conditionType, conditionOwnerString), + // NOTE: LastTransitionTime and ObservedGeneration will be set when this condition is added to an object by calling Set. + }, nil } } - if mirrorOpt.overrideType != "" { - c.Type = mirrorOpt.overrideType - } - - return c, nil + return &metav1.Condition{ + Type: mirrorOpt.targetConditionType, + Status: metav1.ConditionUnknown, + LastTransitionTime: metav1.Now(), + Reason: NotYetReportedReason, + Message: fmt.Sprintf("Condition %s not yet reported from %s", sourceConditionType, conditionOwnerString), + // NOTE: LastTransitionTime and ObservedGeneration will be set when this condition is added to an object by calling Set. + }, nil } // SetMirrorCondition is a convenience method that calls NewMirrorCondition to create a mirror condition from the source object, // and then calls Set to add the new condition to the target object. -func SetMirrorCondition(sourceObj, targetObj runtime.Object, conditionType string, opts ...MirrorOption) error { - mirrorCondition, err := NewMirrorCondition(sourceObj, conditionType, opts...) +func SetMirrorCondition(sourceObj, targetObj runtime.Object, sourceConditionType string, opts ...MirrorOption) error { + mirrorCondition, err := NewMirrorCondition(sourceObj, sourceConditionType, opts...) if err != nil { return err } diff --git a/util/conditions/experimental/mirror_test.go b/util/conditions/v1beta2/mirror_test.go similarity index 76% rename from util/conditions/experimental/mirror_test.go rename to util/conditions/v1beta2/mirror_test.go index 466d1a71eebd..73f226bdf9eb 100644 --- a/util/conditions/experimental/mirror_test.go +++ b/util/conditions/v1beta2/mirror_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "testing" @@ -41,16 +41,16 @@ func TestMirrorStatusCondition(t *testing.T) { }, conditionType: "Ready", options: []MirrorOption{}, - want: metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood!", Message: "We are good! (from Phase3Obj default/SourceObject)", ObservedGeneration: 10, LastTransitionTime: now}, + want: metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood!", Message: "We are good! (from Phase3Obj SourceObject)", ObservedGeneration: 10, LastTransitionTime: now}, }, { - name: "Mirror a condition with override type", + name: "Mirror a condition with target type", conditions: []metav1.Condition{ {Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood!", Message: "We are good!", ObservedGeneration: 10, LastTransitionTime: now}, }, conditionType: "Ready", - options: []MirrorOption{OverrideType("SomethingReady")}, - want: metav1.Condition{Type: "SomethingReady", Status: metav1.ConditionTrue, Reason: "AllGood!", Message: "We are good! (from Phase3Obj default/SourceObject)", ObservedGeneration: 10, LastTransitionTime: now}, + options: []MirrorOption{TargetConditionType("SomethingReady")}, + want: metav1.Condition{Type: "SomethingReady", Status: metav1.ConditionTrue, Reason: "AllGood!", Message: "We are good! (from Phase3Obj SourceObject)", ObservedGeneration: 10, LastTransitionTime: now}, }, { name: "Mirror a condition with empty message", @@ -59,21 +59,21 @@ func TestMirrorStatusCondition(t *testing.T) { }, conditionType: "Ready", options: []MirrorOption{}, - want: metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood!", Message: "(from Phase3Obj default/SourceObject)", ObservedGeneration: 10, LastTransitionTime: now}, + want: metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood!", Message: "(from Phase3Obj SourceObject)", ObservedGeneration: 10, LastTransitionTime: now}, }, { name: "Mirror a condition not yet reported", conditions: []metav1.Condition{}, conditionType: "Ready", options: []MirrorOption{}, - want: metav1.Condition{Type: "Ready", Status: metav1.ConditionUnknown, Reason: NotYetReportedReason, Message: "Condition Ready not yet reported from Phase3Obj default/SourceObject", LastTransitionTime: now}, + want: metav1.Condition{Type: "Ready", Status: metav1.ConditionUnknown, Reason: NotYetReportedReason, Message: "Condition Ready not yet reported from Phase3Obj SourceObject", LastTransitionTime: now}, }, { - name: "Mirror a condition not yet reported with override type", + name: "Mirror a condition not yet reported with target type", conditions: []metav1.Condition{}, conditionType: "Ready", - options: []MirrorOption{OverrideType("SomethingReady")}, - want: metav1.Condition{Type: "SomethingReady", Status: metav1.ConditionUnknown, Reason: NotYetReportedReason, Message: "Condition Ready not yet reported from Phase3Obj default/SourceObject", LastTransitionTime: now}, + options: []MirrorOption{TargetConditionType("SomethingReady")}, + want: metav1.Condition{Type: "SomethingReady", Status: metav1.ConditionUnknown, Reason: NotYetReportedReason, Message: "Condition Ready not yet reported from Phase3Obj SourceObject", LastTransitionTime: now}, }, } @@ -95,9 +95,7 @@ func TestMirrorStatusCondition(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) g.Expect(got).ToNot(BeNil()) - // TODO: think about how to improve this; the issue happens when the condition is not yet reported and one gets generated (using time.now) - Option is to make the time overridable got.LastTransitionTime = tt.want.LastTransitionTime - g.Expect(*got).To(Equal(tt.want)) }) } diff --git a/util/conditions/experimental/options.go b/util/conditions/v1beta2/options.go similarity index 65% rename from util/conditions/experimental/options.go rename to util/conditions/v1beta2/options.go index f7a55211249d..6fa2a3073a85 100644 --- a/util/conditions/experimental/options.go +++ b/util/conditions/v1beta2/options.go @@ -14,37 +14,29 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 -// ConditionFields defines the path where conditions are defined in Unstructured objects. -type ConditionFields []string - -// ApplyToSet applies this configuration to the given Set options. -func (f ConditionFields) ApplyToSet(opts *SetOptions) { - opts.conditionsFields = f -} - -// TODO: Think about using Less directly instead of defining a new option (might be only useful for the UX). +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" // ConditionSortFunc defines the sort order when conditions are assigned to an object. -type ConditionSortFunc Less +type ConditionSortFunc func(i, j metav1.Condition) bool // ApplyToSet applies this configuration to the given Set options. func (f ConditionSortFunc) ApplyToSet(opts *SetOptions) { - opts.less = Less(f) + opts.conditionSortFunc = f } -// OverrideType allows to override the type of new mirror or aggregate conditions. -type OverrideType string +// TargetConditionType allows to specify the type of new mirror or aggregate conditions. +type TargetConditionType string -// ApplyToMirror applies this configuration to the given mirrorInto options. -func (t OverrideType) ApplyToMirror(opts *MirrorOptions) { - opts.overrideType = string(t) +// ApplyToMirror applies this configuration to the given mirror options. +func (t TargetConditionType) ApplyToMirror(opts *MirrorOptions) { + opts.targetConditionType = string(t) } // ApplyToAggregate applies this configuration to the given aggregate options. -func (t OverrideType) ApplyToAggregate(opts *AggregateOptions) { - opts.overrideType = string(t) +func (t TargetConditionType) ApplyToAggregate(opts *AggregateOptions) { + opts.targetConditionType = string(t) } // ForConditionTypes allows to define the set of conditions in scope for a summary operation. @@ -56,14 +48,22 @@ func (t ForConditionTypes) ApplyToSummary(opts *SummaryOptions) { opts.conditionTypes = t } -// WithNegativeConditionTypes allows to define polarity for some of the conditions in scope for a summary operation. -type WithNegativeConditionTypes []string +// WithNegativePolarityConditionTypes allows to define polarity for some of the conditions in scope for a summary operation. +type WithNegativePolarityConditionTypes []string // ApplyToSummary applies this configuration to the given summary options. -func (t WithNegativeConditionTypes) ApplyToSummary(opts *SummaryOptions) { +func (t WithNegativePolarityConditionTypes) ApplyToSummary(opts *SummaryOptions) { opts.negativePolarityConditionTypes = t } +// IgnoreTypesIfMissing allows to define conditions types that should be ignored (not defaulted to unknown) when performing a summary operation. +type IgnoreTypesIfMissing []string + +// ApplyToSummary applies this configuration to the given summary options. +func (t IgnoreTypesIfMissing) ApplyToSummary(opts *SummaryOptions) { + opts.ignoreTypesIfMissing = t +} + // WithMergeStrategy allows to define a custom merge strategy when creating new summary or aggregate conditions. type WithMergeStrategy struct { MergeStrategy diff --git a/util/conditions/v1beta2/setter.go b/util/conditions/v1beta2/setter.go new file mode 100644 index 000000000000..5405475da654 --- /dev/null +++ b/util/conditions/v1beta2/setter.go @@ -0,0 +1,172 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta2 + +import ( + "reflect" + "sort" + + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +// SetOption is some configuration that modifies options for a Set request. +type SetOption interface { + // ApplyToSet applies this configuration to the given Set options. + ApplyToSet(option *SetOptions) +} + +// SetOptions allows to define options for the set operation. +type SetOptions struct { + conditionSortFunc ConditionSortFunc +} + +// ApplyOptions applies the given list options on these options, +// and then returns itself (for convenient chaining). +func (o *SetOptions) ApplyOptions(opts []SetOption) *SetOptions { + for _, opt := range opts { + opt.ApplyToSet(o) + } + return o +} + +// Set a condition on the given object. +// +// Set support adding conditions to objects at different stages of the transition to metav1.condition type: +// - Objects with metav1.condition in status.v1beta2.conditions conditions +// - Objects with metav1.condition in status.conditions +// +// When setting a condition: +// - condition.ObservedGeneration will be set to object.Metadata.Generation. +// - If the condition does not exist and condition.LastTransitionTime is not set, time.Now is used. +// - If the condition already exist, condition.Status is changing and condition.LastTransitionTime is not set, time.Now is used. +// - If the condition already exist, condition.Status is NOT changing, all the fields can be changed except for condition.LastTransitionTime. +// +// Set can't be used with unstructured objects. +// +// Additionally, Set enforce the a default condition order (Available and Ready fist, everything else in alphabetical order), +// but this can be changed by using the ConditionSortFunc option. +func Set(targetObj runtime.Object, condition metav1.Condition, opts ...SetOption) error { + conditions, err := GetAll(targetObj) + if err != nil { + return err + } + + if objMeta, ok := targetObj.(metav1.Object); ok { + condition.ObservedGeneration = objMeta.GetGeneration() + } + + if changed := meta.SetStatusCondition(&conditions, condition); !changed { + return nil + } + + return SetAll(targetObj, conditions, opts...) +} + +// SetAll the conditions on the given object. +// +// SetAll support adding conditions to objects at different stages of the transition to metav1.condition type: +// - Objects with metav1.condition in status.v1beta2.conditions conditions +// - Objects with metav1.condition in status.conditions +// +// SetAll can't be used with unstructured objects. +// +// Additionally, SetAll enforce a default condition order (Available and Ready fist, everything else in alphabetical order), +// but this can be changed by using the ConditionSortFunc option. +func SetAll(targetObj runtime.Object, conditions []metav1.Condition, opts ...SetOption) error { + setOpt := &SetOptions{ + // By default sort condition by the default condition order (first available, then ready, then the other conditions if alphabetical order. + conditionSortFunc: defaultSortLessFunc, + } + setOpt.ApplyOptions(opts) + + if setOpt.conditionSortFunc != nil { + sort.SliceStable(conditions, func(i, j int) bool { + return setOpt.conditionSortFunc(conditions[i], conditions[j]) + }) + } + + switch targetObj.(type) { + case runtime.Unstructured: + return errors.New("cannot set conditions on unstructured objects") + default: + return setToTypedObject(targetObj, conditions) + } +} + +var metav1ConditionsType = reflect.TypeOf([]metav1.Condition{}) + +func setToTypedObject(obj runtime.Object, conditions []metav1.Condition) error { + if obj == nil { + return errors.New("cannot set conditions on a nil object") + } + + ptr := reflect.ValueOf(obj) + if ptr.Kind() != reflect.Pointer { + return errors.New("cannot set conditions on a object that is not a pointer") + } + + elem := ptr.Elem() + if !elem.IsValid() { + return errors.New("obj must be a valid value (non zero value of its type)") + } + + ownerInfo := getConditionOwnerInfo(obj) + + statusField := elem.FieldByName("Status") + if statusField == (reflect.Value{}) { + return errors.Errorf("cannot set conditions on %s, status field is missing", ownerInfo) + } + + // Set conditions. + // NOTE: Given that we allow providers to migrate at different speed, it is required to support objects at the different stage of the transition from legacy conditions to metav1.conditions. + // In order to handle this, first try to set Status.V1Beta2.Conditions, then Status.Conditions. + // The V1Beta2 branch should be dropped when v1beta1 API are removed. + + if v1beta2Field := statusField.FieldByName("V1Beta2"); v1beta2Field != (reflect.Value{}) { + if conditionField := v1beta2Field.FieldByName("Conditions"); conditionField != (reflect.Value{}) { + if conditionField.Type() != metav1ConditionsType { + return errors.Errorf("cannot set conditions on %s.v1beta2.conditions, the field doesn't have []metav1.Condition type: %s type detected", ownerInfo, reflect.TypeOf(conditionField.Interface()).String()) + } + + setToTypedField(conditionField, conditions) + return nil + } + } + + if conditionField := statusField.FieldByName("Conditions"); conditionField != (reflect.Value{}) { + if conditionField.Type() != metav1ConditionsType { + return errors.Errorf("cannot set conditions on %s.conditions, the field doesn't have []metav1.Condition type []metav1.Condition: %s type detected", ownerInfo, reflect.TypeOf(conditionField.Interface()).String()) + } + + setToTypedField(conditionField, conditions) + return nil + } + + return errors.Errorf("cannot set conditions on %s both status.v1beta2.conditions and status.conditions fields are missing", ownerInfo) +} + +func setToTypedField(conditionField reflect.Value, conditions []metav1.Condition) { + n := len(conditions) + conditionField.Set(reflect.MakeSlice(conditionField.Type(), n, n)) + for i := range n { + itemField := conditionField.Index(i) + itemField.Set(reflect.ValueOf(conditions[i])) + } +} diff --git a/util/conditions/v1beta2/setter_test.go b/util/conditions/v1beta2/setter_test.go new file mode 100644 index 000000000000..6b446e771f15 --- /dev/null +++ b/util/conditions/v1beta2/setter_test.go @@ -0,0 +1,173 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta2 + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/internal/test/builder" +) + +func TestSetAll(t *testing.T) { + now := metav1.Now().Rfc3339Copy() + + conditions := []metav1.Condition{ + { + Type: "fooCondition", + Status: metav1.ConditionTrue, + ObservedGeneration: 10, + LastTransitionTime: now, + Reason: "FooReason", + Message: "FooMessage", + }, + } + + cloneConditions := func() []metav1.Condition { + ret := make([]metav1.Condition, len(conditions)) + copy(ret, conditions) + return ret + } + + t.Run("fails with nil", func(t *testing.T) { + g := NewWithT(t) + + conditions := cloneConditions() + err := SetAll(nil, conditions) + g.Expect(err).To(HaveOccurred()) + }) + + t.Run("fails for nil object", func(t *testing.T) { + g := NewWithT(t) + var foo *builder.Phase0Obj + + conditions := cloneConditions() + err := SetAll(foo, conditions) + g.Expect(err).To(HaveOccurred()) + }) + + t.Run("fails for Unstructured", func(t *testing.T) { + g := NewWithT(t) + fooUnstructured := &unstructured.Unstructured{} + + conditions := cloneConditions() + err := SetAll(fooUnstructured, conditions) + g.Expect(err).To(HaveOccurred()) + }) + + t.Run("v1beta object with legacy conditions", func(t *testing.T) { + g := NewWithT(t) + foo := &builder.Phase0Obj{ + Status: builder.Phase0ObjStatus{Conditions: nil}, + } + + conditions := cloneConditions() + err := SetAll(foo, conditions) + g.Expect(err).To(HaveOccurred()) // Can't set legacy conditions. + }) + + t.Run("v1beta1 object with both legacy and v1beta2 conditions", func(t *testing.T) { + g := NewWithT(t) + foo := &builder.Phase1Obj{ + Status: builder.Phase1ObjStatus{ + Conditions: clusterv1.Conditions{ + { + Type: "barCondition", + Status: corev1.ConditionFalse, + LastTransitionTime: now, + }, + }, + V1Beta2: builder.Phase1ObjStatusV1beta2{Conditions: nil}, + }, + } + + conditions := cloneConditions() + err := SetAll(foo, conditions) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(foo.Status.V1Beta2.Conditions).To(Equal(conditions), cmp.Diff(foo.Status.V1Beta2.Conditions, conditions)) + }) + + t.Run("v1beta2 object with conditions and backward compatible conditions", func(t *testing.T) { + g := NewWithT(t) + foo := &builder.Phase2Obj{ + Status: builder.Phase2ObjStatus{ + Conditions: nil, + Deprecated: builder.Phase2ObjStatusDeprecated{ + V1Beta1: builder.Phase2ObjStatusDeprecatedV1Beta2{ + Conditions: clusterv1.Conditions{ + { + Type: "barCondition", + Status: corev1.ConditionFalse, + LastTransitionTime: now, + }, + }, + }, + }, + }, + } + + conditions := cloneConditions() + err := SetAll(foo, conditions) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(foo.Status.Conditions).To(MatchConditions(conditions), cmp.Diff(foo.Status.Conditions, conditions)) + }) + + t.Run("v1beta2 object with conditions (end state)", func(t *testing.T) { + g := NewWithT(t) + foo := &builder.Phase3Obj{ + Status: builder.Phase3ObjStatus{ + Conditions: nil, + }, + } + + conditions := cloneConditions() + err := SetAll(foo, conditions) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(foo.Status.Conditions).To(Equal(conditions), cmp.Diff(foo.Status.Conditions, conditions)) + }) + + t.Run("Set infers ObservedGeneration", func(t *testing.T) { + g := NewWithT(t) + foo := &builder.Phase3Obj{ + ObjectMeta: metav1.ObjectMeta{Generation: 123}, + Status: builder.Phase3ObjStatus{ + Conditions: nil, + }, + } + + condition := metav1.Condition{ + Type: "fooCondition", + Status: metav1.ConditionTrue, + LastTransitionTime: now, + Reason: "FooReason", + Message: "FooMessage", + } + + err := Set(foo, condition) + g.Expect(err).ToNot(HaveOccurred()) + + condition.ObservedGeneration = foo.Generation + conditions := []metav1.Condition{condition} + g.Expect(foo.Status.Conditions).To(Equal(conditions), cmp.Diff(foo.Status.Conditions, conditions)) + }) +} diff --git a/util/conditions/experimental/sort.go b/util/conditions/v1beta2/sort.go similarity index 81% rename from util/conditions/experimental/sort.go rename to util/conditions/v1beta2/sort.go index eb5aaa6d9b3e..cdfedbdc8953 100644 --- a/util/conditions/experimental/sort.go +++ b/util/conditions/v1beta2/sort.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -26,13 +26,10 @@ const ( ReadyCondition = "Ready" ) -// Less reports whether the i condition must sort before j condition. -type Less func(i, j metav1.Condition) bool - -// lexicographicLess returns true if a condition is less than another with regards to the +// defaultSortLessFunc returns true if a condition is less than another with regards to the // to order of conditions designed for convenience of the consumer, i.e. kubectl get. // According to this order the Available and the Ready condition always goes first, followed by all the other // conditions sorted by Type. -func lexicographicLess(i, j metav1.Condition) bool { +func defaultSortLessFunc(i, j metav1.Condition) bool { return (i.Type == AvailableCondition || (i.Type == ReadyCondition || i.Type < j.Type) && j.Type != ReadyCondition) && j.Type != AvailableCondition } diff --git a/util/conditions/experimental/sort_test.go b/util/conditions/v1beta2/sort_test.go similarity index 89% rename from util/conditions/experimental/sort_test.go rename to util/conditions/v1beta2/sort_test.go index 48c778fe9edb..5f3e972634c1 100644 --- a/util/conditions/experimental/sort_test.go +++ b/util/conditions/v1beta2/sort_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "sort" @@ -24,7 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestLexicographicLess(t *testing.T) { +func TestDefaultSortLessFunc(t *testing.T) { g := NewWithT(t) conditions := []metav1.Condition{ @@ -36,7 +36,7 @@ func TestLexicographicLess(t *testing.T) { } sort.Slice(conditions, func(i, j int) bool { - return lexicographicLess(conditions[i], conditions[j]) + return defaultSortLessFunc(conditions[i], conditions[j]) }) g.Expect(conditions).To(Equal([]metav1.Condition{ diff --git a/util/conditions/experimental/summary.go b/util/conditions/v1beta2/summary.go similarity index 75% rename from util/conditions/experimental/summary.go rename to util/conditions/v1beta2/summary.go index bac71fe9517f..2b7d7bb56e0e 100644 --- a/util/conditions/experimental/summary.go +++ b/util/conditions/v1beta2/summary.go @@ -14,17 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "fmt" + "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" ) -// SummaryOption is some configuration that modifies options for a summary request. +// SummaryOption is some configuration that modifies options for a summary call. type SummaryOption interface { // ApplyToSummary applies this configuration to the given summary options. ApplyToSummary(*SummaryOptions) @@ -35,6 +36,7 @@ type SummaryOptions struct { mergeStrategy MergeStrategy conditionTypes []string negativePolarityConditionTypes []string + ignoreTypesIfMissing []string stepCounter bool } @@ -50,23 +52,21 @@ func (o *SummaryOptions) ApplyOptions(opts []SummaryOption) *SummaryOptions { // NewSummaryCondition creates a new condition by summarizing a set of conditions from an object. // If any of the condition in scope does not exist in the source object, missing conditions are considered Unknown, reason NotYetReported. // -// By default, the Aggregate condition has the same type of the source condition, but this can be changed by using -// the OverrideType option. -// // Additionally, it is possible to inject custom merge strategies using the WithMergeStrategy option or // to add a step counter to the generated message by using the WithStepCounter option. -func NewSummaryCondition(obj runtime.Object, conditionType string, opts ...SummaryOption) (*metav1.Condition, error) { +func NewSummaryCondition(sourceObj runtime.Object, targetConditionTYpe string, opts ...SummaryOption) (*metav1.Condition, error) { summarizeOpt := &SummaryOptions{ mergeStrategy: newDefaultMergeStrategy(), } summarizeOpt.ApplyOptions(opts) - conditions, err := getConditionsWithOwnerInfo(obj) + conditions, err := getConditionsWithOwnerInfo(sourceObj) if err != nil { return nil, err } expectedConditionTypes := sets.New[string](summarizeOpt.conditionTypes...) + ignoreTypesIfMissing := sets.New[string](summarizeOpt.ignoreTypesIfMissing...) existingConditionTypes := sets.New[string]() // Drops all the conditions not in scope for the merge operation @@ -81,25 +81,29 @@ func NewSummaryCondition(obj runtime.Object, conditionType string, opts ...Summa // Add the expected conditions which do net exists, so we are compliant with K8s guidelines // (all missing conditions should be considered unknown). - // TODO: consider if we want to allow exception to this rule. e.g it is ok that HealthCheckSucceeded is missing. - diff := expectedConditionTypes.Difference(existingConditionTypes).UnsortedList() + diff := expectedConditionTypes.Difference(existingConditionTypes).Difference(ignoreTypesIfMissing).UnsortedList() if len(diff) > 0 { - conditionOwner := getConditionOwnerInfo(obj) + conditionOwner := getConditionOwnerInfo(sourceObj) - for _, conditionType := range diff { + for _, c := range diff { conditionsInScope = append(conditionsInScope, ConditionWithOwnerInfo{ OwnerResource: conditionOwner, Condition: metav1.Condition{ - Type: conditionType, + Type: c, Status: metav1.ConditionUnknown, Reason: NotYetReportedReason, - Message: fmt.Sprintf("Condition %s not yet reported", conditionType), + Message: fmt.Sprintf("Condition %s not yet reported", c), + // NOTE: LastTransitionTime and ObservedGeneration are not relevant for merge. }, }) } } + if len(conditionsInScope) == 0 { + return nil, errors.New("summary can't be performed when the list of conditions to be summarized is empty") + } + status, reason, message, err := summarizeOpt.mergeStrategy.Merge( conditionsInScope, summarizeOpt.conditionTypes, @@ -111,17 +115,18 @@ func NewSummaryCondition(obj runtime.Object, conditionType string, opts ...Summa } return &metav1.Condition{ - Type: conditionType, + Type: targetConditionTYpe, Status: status, Reason: reason, Message: message, + // NOTE: LastTransitionTime and ObservedGeneration will be set when this condition is added to an object by calling Set. }, err } // SetSummaryCondition is a convenience method that calls NewSummaryCondition to create a summary condition from the source object, // and then calls Set to add the new condition to the target object. -func SetSummaryCondition(sourceObj, targetObj runtime.Object, conditionType string, opts ...MirrorOption) error { - mirrorCondition, err := NewMirrorCondition(sourceObj, conditionType, opts...) +func SetSummaryCondition(sourceObj, targetObj runtime.Object, targetConditionTYpe string, opts ...MirrorOption) error { + mirrorCondition, err := NewMirrorCondition(sourceObj, targetConditionTYpe, opts...) if err != nil { return err } diff --git a/util/conditions/experimental/summary_test.go b/util/conditions/v1beta2/summary_test.go similarity index 80% rename from util/conditions/experimental/summary_test.go rename to util/conditions/v1beta2/summary_test.go index e6f01aeb79f1..23637278319f 100644 --- a/util/conditions/experimental/summary_test.go +++ b/util/conditions/v1beta2/summary_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package experimental +package v1beta2 import ( "testing" @@ -41,7 +41,7 @@ func TestSummary(t *testing.T) { {Type: "!C", Status: metav1.ConditionTrue, Reason: "Reason-!C", Message: "Message-!C"}, // issue }, conditionType: AvailableCondition, - options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativeConditionTypes{"!C"}}, + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}}, want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionFalse, // False because there is one issue @@ -57,11 +57,11 @@ func TestSummary(t *testing.T) { {Type: "!C", Status: metav1.ConditionTrue, Reason: "Reason-!C", Message: "Message-!C"}, // issue }, conditionType: AvailableCondition, - options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativeConditionTypes{"!C"}}, + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}}, want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionFalse, // False because there are many issues - Reason: ManyIssuesReason, // Using a generic reason + Reason: MultipleIssuesReason, // Using a generic reason Message: "B (False): Message-B; !C (True): Message-!C", // messages from all the issues & unknown conditions (info dropped) }, }, @@ -73,11 +73,11 @@ func TestSummary(t *testing.T) { {Type: "!C", Status: metav1.ConditionTrue, Reason: "Reason-!C", Message: "Message-!C"}, // issue }, conditionType: AvailableCondition, - options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativeConditionTypes{"!C"}}, + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}}, want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionFalse, // False because there are many issues - Reason: ManyIssuesReason, // Using a generic reason + Reason: MultipleIssuesReason, // Using a generic reason Message: "B (False): Message-B; !C (True): Message-!C; A (Unknown): Message-A", // messages from all the issues & unknown conditions (info dropped) }, }, @@ -89,7 +89,7 @@ func TestSummary(t *testing.T) { {Type: "!C", Status: metav1.ConditionUnknown, Reason: "Reason-!C", Message: "Message-!C"}, // unknown }, conditionType: AvailableCondition, - options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativeConditionTypes{"!C"}}, + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}}, want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionUnknown, // Unknown because there is one unknown @@ -105,17 +105,15 @@ func TestSummary(t *testing.T) { {Type: "!C", Status: metav1.ConditionUnknown, Reason: "Reason-!C", Message: "Message-!C"}, // unknown }, conditionType: AvailableCondition, - options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativeConditionTypes{"!C"}}, + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}}, want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionUnknown, // Unknown because there are many unknown - Reason: ManyUnknownsReason, // Using a generic reason + Reason: MultipleUnknownReported, // Using a generic reason Message: "B (Unknown): Message-B; !C (Unknown): Message-!C", // messages from all the issues & unknown conditions (info dropped) }, }, - // TODO: Think about how to test One info (no issues, no unknown): If I use only one ConditionTypes is considered Aggregate, If I use more, it is not One info (no issues, no unknown) - { name: "More than one info (no issues, no unknown)", conditions: []metav1.Condition{ @@ -124,11 +122,11 @@ func TestSummary(t *testing.T) { {Type: "!C", Status: metav1.ConditionFalse, Reason: "Reason-!C", Message: "Message-!C"}, // info }, conditionType: AvailableCondition, - options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativeConditionTypes{"!C"}, WithMergeStrategy{newDefaultMergeStrategy()}}, + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}, WithMergeStrategy{newDefaultMergeStrategy()}}, want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionTrue, // True because there are many info - Reason: ManyInfoReason, // Using a generic reason + Reason: MultipleInfoReason, // Using a generic reason Message: "B (True): Message-B; !C (False): Message-!C", // messages from all the info conditions (empty messages are dropped) }, }, @@ -139,14 +137,29 @@ func TestSummary(t *testing.T) { // B and !C missing }, conditionType: AvailableCondition, - options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativeConditionTypes{"!C"}}, // B and !C are required! + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}}, // B and !C are required! want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionUnknown, // Unknown because there more than one unknown - Reason: ManyUnknownsReason, // Picking the reason from the only existing issue + Reason: MultipleUnknownReported, // Using a generic reason Message: "B (Unknown): Condition B not yet reported; !C (Unknown): Condition !C not yet reported", // messages from all the issues & unknown conditions (info dropped) }, }, + { + name: "Default missing conditions to unknown consider IgnoreTypesIfMissing", + conditions: []metav1.Condition{ + {Type: "A", Status: metav1.ConditionTrue, Reason: "Reason-A", Message: "Message-A"}, // info + // B and !C missing + }, + conditionType: AvailableCondition, + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}, IgnoreTypesIfMissing{"B"}}, // B and !C are required! + want: &metav1.Condition{ + Type: AvailableCondition, + Status: metav1.ConditionUnknown, // Unknown because there more than one unknown + Reason: NotYetReportedReason, // Picking the reason from the only existing issue, which is a default missing condition added for !C + Message: "!C (Unknown): Condition !C not yet reported", // messages from all the issues & unknown conditions (info dropped) + }, + }, { name: "Ignore conditions not in scope", conditions: []metav1.Condition{ @@ -159,7 +172,7 @@ func TestSummary(t *testing.T) { want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionTrue, // True because there are many info - Reason: ManyInfoReason, // Using a generic reason + Reason: MultipleInfoReason, // Using a generic reason Message: "B (True): Message-B", // messages from all the info conditions (empty messages are dropped) }, }, @@ -171,7 +184,7 @@ func TestSummary(t *testing.T) { {Type: "!C", Status: metav1.ConditionUnknown, Reason: "Reason-!C", Message: "Message-!C"}, // unknown }, conditionType: AvailableCondition, - options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativeConditionTypes{"!C"}, WithStepCounter(true)}, + options: []SummaryOption{ForConditionTypes{"A", "B", "!C"}, WithNegativePolarityConditionTypes{"!C"}, WithStepCounter(true)}, want: &metav1.Condition{ Type: AvailableCondition, Status: metav1.ConditionUnknown, // Unknown because there is one unknown @@ -201,4 +214,11 @@ func TestSummary(t *testing.T) { g.Expect(got).To(Equal(tt.want)) }) } + + t.Run("Fails if conditions in scope are empty", func(t *testing.T) { + g := NewWithT(t) + obj := &builder.Phase3Obj{} + _, err := NewSummaryCondition(obj, AvailableCondition) // no ForConditionTypes --> Condition in scope will be empty + g.Expect(err).To(HaveOccurred()) + }) }