diff --git a/pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1/cnsnodebatchvmattachment_types.go b/pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1/cnsnodebatchvmattachment_types.go index fcef560f1d..533dec7c2d 100644 --- a/pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1/cnsnodebatchvmattachment_types.go +++ b/pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1/cnsnodebatchvmattachment_types.go @@ -36,6 +36,24 @@ const ( IndependentNonPersistent = "independent_nonpersistent" // Changes to virtual disk are made to a redo log and discarded at power off. NonPersistent = "nonpersistent" + + // This section defines the different conditions that CnsNodeVMBatchAttachment CR can take. + // ConditionReady reflects the overall status of the CR. + ConditionReady = "Ready" + // ConditionAttached reflects whether the given volume was attached successfully. + ConditionAttached = "VolumeAttached" + // ConditionDetached reflects whether the given volume was detached successfully. + ConditionDetached = "VolumeDetached" + + // This section defines the different reasons for different conditions in the CnsNodeVMBatchAttachment CR. + // ReasonAttachFailed reflects that the volume failed to get attached. + // In case of successful attachment, reason is set to True. + ReasonAttachFailed = "AttachFailed" + // ReasonDetachFailed reflects that the volume failed to get detached. + // In case of successful detach, the volume's entry is removed from the CR. + ReasonDetachFailed = "DetachFailed" + // ReasonFailed reflects that the CR instance is not yet ready. + ReasonFailed = "Failed" ) // SharingMode is the sharing mode of the virtual disk. @@ -100,12 +118,15 @@ type PersistentVolumeClaimSpec struct { // CnsNodeVMBatchAttachmentStatus defines the observed state of CnsNodeVMBatchAttachment // +k8s:openapi-gen=true type CnsNodeVMBatchAttachmentStatus struct { - // Error is the overall error status for the instance. - Error string `json:"error,omitempty"` + // +optional // +listType=map // +listMapKey=name // VolumeStatus reflects the status for each volume. VolumeStatus []VolumeStatus `json:"volumes,omitempty"` + // +optional + + // Conditions describes any conditions associated with this CnsNodeVMBatchAttachment instance. + Conditions []metav1.Condition `json:"conditions,omitempty"` } type VolumeStatus struct { @@ -118,17 +139,18 @@ type VolumeStatus struct { type PersistentVolumeClaimStatus struct { // ClaimName is the PVC name. ClaimName string `json:"claimName"` - // Attached indicates the attach status of a PVC. - // If volume is not attached, Attached will be set to false. - // If volume is attached, Attached will be set to true. - // If volume is detached successfully, its entry will be removed from VolumeStatus. - Attached bool `json:"attached"` - // Error indicates the error which may have occurred during attach/detach. - Error string `json:"error,omitempty"` + // +optional + // CnsVolumeID is the volume ID for the PVC. CnsVolumeID string `json:"cnsVolumeId,omitempty"` + // +optional + // DiskUUID is the ID obtained when volume is attached to a VM. DiskUUID string `json:"diskUUID,omitempty"` + // +optional + + // Conditions describes any conditions associated with this volume. + Conditions []metav1.Condition `json:"conditions,omitempty"` } // +genclient @@ -157,3 +179,19 @@ type CnsNodeVMBatchAttachmentList struct { metav1.ListMeta `json:"metadata,omitempty"` Items []CnsNodeVMBatchAttachment `json:"items"` } + +func (in *CnsNodeVMBatchAttachment) GetConditions() []metav1.Condition { + return in.Status.Conditions +} + +func (in *CnsNodeVMBatchAttachment) SetConditions(conditions []metav1.Condition) { + in.Status.Conditions = conditions +} + +func (p *PersistentVolumeClaimStatus) GetConditions() []metav1.Condition { + return p.Conditions +} + +func (p *PersistentVolumeClaimStatus) SetConditions(conditions []metav1.Condition) { + p.Conditions = conditions +} diff --git a/pkg/apis/cnsoperator/config/cns.vmware.com_cnsnodevmbatchattachments.yaml b/pkg/apis/cnsoperator/config/cns.vmware.com_cnsnodevmbatchattachments.yaml index b2be2a7d14..435dd32dbd 100644 --- a/pkg/apis/cnsoperator/config/cns.vmware.com_cnsnodevmbatchattachments.yaml +++ b/pkg/apis/cnsoperator/config/cns.vmware.com_cnsnodevmbatchattachments.yaml @@ -112,9 +112,77 @@ spec: description: CnsNodeVMBatchAttachmentStatus defines the observed state of CnsNodeVMBatchAttachment properties: - error: - description: Error is the overall error status for the instance. - type: string + conditions: + description: Conditions describes any conditions associated with this + CnsNodeVMBatchAttachment instance. + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + 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. + --- + 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. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + 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 volumes: description: VolumeStatus reflects the status for each volume. items: @@ -126,29 +194,91 @@ spec: description: PersistentVolumeClaim contains details about the volume's current state. properties: - attached: - description: |- - Attached indicates the attach status of a PVC. - If volume is not attached, Attached will be set to false. - If volume is attached, Attached will be set to true. - If volume is detached successfully, its entry will be removed from VolumeStatus. - type: boolean claimName: description: ClaimName is the PVC name. type: string cnsVolumeId: description: CnsVolumeID is the volume ID for the PVC. type: string + conditions: + description: Conditions describes any conditions associated + with this volume. + items: + description: "Condition contains details for one aspect + of the current state of this API Resource.\n---\nThis + struct is intended for direct use as an array at the + field path .status.conditions. For example,\n\n\n\ttype + FooStatus struct{\n\t // Represents the observations + of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t + \ // +patchMergeKey=type\n\t // +patchStrategy=merge\n\t + \ // +listType=map\n\t // +listMapKey=type\n\t + \ Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + 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. + --- + 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. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + 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 diskUUID: description: DiskUUID is the ID obtained when volume is attached to a VM. type: string - error: - description: Error indicates the error which may have occurred - during attach/detach. - type: string required: - - attached - claimName type: object required: @@ -170,4 +300,4 @@ status: kind: "" plural: "" conditions: [] - storedVersions: [] \ No newline at end of file + storedVersions: [] diff --git a/pkg/common/cns-lib/volume/manager.go b/pkg/common/cns-lib/volume/manager.go index e09edd74ce..e21e927ae0 100644 --- a/pkg/common/cns-lib/volume/manager.go +++ b/pkg/common/cns-lib/volume/manager.go @@ -1304,8 +1304,10 @@ func (m *defaultManager) DetachVolume(ctx context.Context, vm *cnsvsphere.Virtua } } } - return faultType, logger.LogNewErrorf(log, "failed to detach cns volume: %q from node vm: %+v. fault: %+v, opId: %q", + log.Errorf("failed to detach cns volume: %q from node vm: %+v. fault: %+v, opId: %q", volumeID, vm, spew.Sdump(volumeOperationRes.Fault), taskInfo.ActivationId) + return faultType, fmt.Errorf("failed to detach cns volume: %q, Error: %s,", + volumeID, volumeOperationRes.Fault.LocalizedMessage) } log.Infof("DetachVolume: Volume detached successfully. volumeID: %q, vm: %q, opId: %q", volumeID, vm.String(), taskInfo.ActivationId) @@ -3492,8 +3494,10 @@ func compileBatchAttachTaskResult(ctx context.Context, result cnstypes.BaseCnsVo // In case of failure, set faultType and error. faultType := ExtractFaultTypeFromVolumeResponseResult(ctx, volumeOperationResult) batchAttachResult.FaultType = faultType - msg := fmt.Sprintf("failed to batch attach cns volume: %q to node vm: %q. fault: %q. opId: %q", + log.Errorf("failed to attach cns volume: %q to node vm: %q. fault: %q. opId: %q", volumeId, vm.String(), faultType, activationId) + msg := fmt.Sprintf("failed to attach cns volume: %q Error: %s", + volumeId, volumeOperationResult.Fault.LocalizedMessage) batchAttachResult.Error = errors.New(msg) log.Infof("Constructed batch attach result for volume %s with failure", volumeId) return batchAttachResult, nil diff --git a/pkg/common/conditions/conditions.go b/pkg/common/conditions/conditions.go new file mode 100644 index 0000000000..e358dd6a0a --- /dev/null +++ b/pkg/common/conditions/conditions.go @@ -0,0 +1,250 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "sort" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + // ReadyConditionType indicates a resource is ready. + ReadyConditionType = "Ready" +) + +// Conditions is an alias for a slice of metav1.Condition objects and provides +// helpful functions. +type Conditions []metav1.Condition + +// c4g returns a Conditions type from a Getter. This is used throughout this +// package to reduce the need to write "Conditions(obj.GetConditions())". +func c4g(g Getter) Conditions { + return g.GetConditions() +} + +// GetConditions allows the Conditions type to implement the Getter interface. +func (l Conditions) GetConditions() []metav1.Condition { + return l +} + +// Get returns the condition with the given type, otherwise nil is returned. +func (l Conditions) Get(t string) *metav1.Condition { + for _, c := range l { + if c.Type == t { + return &c + } + } + return nil +} + +// Has returns true if a condition with the given type exists. +func (l Conditions) Has(t string) bool { + return l.Get(t) != nil +} + +// IsTrue returns true if the condition with the given type exists and is +// True, otherwise false is returned. +func (l Conditions) IsTrue(t string) bool { + if c := l.Get(t); c != nil { + return c.Status == metav1.ConditionTrue + } + return false +} + +// IsFalse returns true if the condition with the given type exists and is +// False, otherwise false is returned. +func (l Conditions) IsFalse(t string) bool { + if c := l.Get(t); c != nil { + return c.Status == metav1.ConditionFalse + } + return false +} + +// IsUnknown returns true if the condition with the given type exists and is +// Unknown, otherwise false is returned. +func (l Conditions) IsUnknown(t string) bool { + if c := l.Get(t); c != nil { + return c.Status == metav1.ConditionUnknown + } + return true +} + +// GetReason returns the condition's reason or an empty string if the condition +// does not exist. +func (l Conditions) GetReason(t string) string { + if c := l.Get(t); c != nil { + return c.Reason + } + return "" +} + +// GetMessage returns the condition's message or an empty string if the +// condition does not exist. +func (l Conditions) GetMessage(t string) string { + if c := l.Get(t); c != nil { + return c.Message + } + return "" +} + +// GetLastTransitionTime returns the condition's last transition time or a nil +// value if the condition does not exist. +func (l Conditions) GetLastTransitionTime(t string) *metav1.Time { + if c := l.Get(t); c != nil { + return &c.LastTransitionTime + } + return nil +} + +// Set sets the given condition. +// If a condition with the same type already exists, its LastTransitionTime is +// only updated if a change is detected in one of the following fields: +// Status, Reason, or Message. +func (l Conditions) Set(c *metav1.Condition) Conditions { + if c == nil { + return l + } + + // Check if the new conditions already exists, and change it only if there + // is a status transition (otherwise preserve the current last transition + // time). + exists := false + for i := range l { + existingCondition := l[i] + if existingCondition.Type == c.Type { + exists = true + if !hasSameState(&existingCondition, c) { + c.LastTransitionTime = metav1.NewTime( + time.Now().UTC().Truncate(time.Second)) + l[i] = *c + break + } + c.LastTransitionTime = existingCondition.LastTransitionTime + break + } + } + + // If the condition does not exist, add it, setting the transition time only + // if it is not already set + if !exists { + if c.LastTransitionTime.IsZero() { + c.LastTransitionTime = metav1.NewTime( + time.Now().UTC().Truncate(time.Second)) + } + l = append(l, *c) + } + + // Sorts conditions for convenience of the consumer, i.e. kubectl. + sort.Slice(l, func(i, j int) bool { + return lexicographicLess(&l[i], &l[j]) + }) + + return l +} + +// MarkTrue sets Status=True for the condition with the given type. +func (l Conditions) MarkTrue(t string) Conditions { + return l.Set(TrueCondition(t)) +} + +// MarkUnknown sets Status=Unknown for the condition with the given type. +func (l Conditions) MarkUnknown( + t, reason, messageFormat string, messageArgs ...any) Conditions { + + return l.Set(UnknownCondition(t, reason, messageFormat, messageArgs...)) +} + +// MarkFalse sets Status=False for the condition with the given type. +func (l Conditions) MarkFalse( + t, reason, messageFormat string, messageArgs ...any) Conditions { + + return l.Set(FalseCondition(t, reason, messageFormat, messageArgs...)) +} + +// MarkError sets Status=False and the error message for the condition with the given type. +func (l Conditions) MarkError( + t, reason string, err error) Conditions { + + return l.Set(FalseCondition(t, reason, "%s", err.Error())) +} + +// SetSummary sets a Ready condition with a summary of all the existing +// conditions. If there are no existing conditions, no summary condition is +// generated. +func (l Conditions) SetSummary(options ...MergeOption) Conditions { + + return l.Set(summary(l, options...)) +} + +// SetMirror creates a new condition by mirroring the Ready condition from a +// source object. If the source object does not have a Ready condition, the +// target is not modified. +func (l Conditions) SetMirror( + targetCondition string, + from Getter, + options ...MirrorOptions) Conditions { + + return l.Set(mirror(from, targetCondition, options...)) +} + +// SetAggregate creates a new condition by aggregating all of the Ready +// conditions from a list of source objects. If a source object is missing the +// Ready condition, that object is excluded from aggregation. If none of the +// source objects have a Ready condition, the target is not modified. +func (l Conditions) SetAggregate( + targetCondition string, + from []Getter, + options ...MergeOption) Conditions { + + return l.Set(aggregate(from, targetCondition, options...)) +} + +// Delete removes the condition with the given type. +func (l Conditions) Delete(t string) Conditions { + if len(l) == 0 { + return l + } + newConditions := make([]metav1.Condition, 0, len(l)) + for _, c := range l { + if c.Type != t { + newConditions = append(newConditions, c) + } + } + return newConditions +} + +// hasSameState returns true if a condition has the same state of another; state +// is defined by the union of following fields: Type, Status, Reason, and +// Message. The field LastTransitionTime is excluded. +func hasSameState(a, b *metav1.Condition) bool { + return a.Type == b.Type && + a.Status == b.Status && + a.Reason == b.Reason && + a.Message == b.Message +} + +// lexicographicLess 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. According to this order the Ready condition always +// goes first, followed by all the other conditions sorted by Type. +func lexicographicLess(a, b *metav1.Condition) bool { + return (a.Type == ReadyConditionType || a.Type < b.Type) && + b.Type != ReadyConditionType +} diff --git a/pkg/common/conditions/conditions_suite_test.go b/pkg/common/conditions/conditions_suite_test.go new file mode 100644 index 0000000000..9a4b7a5704 --- /dev/null +++ b/pkg/common/conditions/conditions_suite_test.go @@ -0,0 +1,59 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + _ "github.com/onsi/ginkgo/v2" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +type nonKubeObj struct { + c []metav1.Condition +} + +func (o nonKubeObj) GetConditions() []metav1.Condition { + return o.c +} + +func (o *nonKubeObj) SetConditions(c []metav1.Condition) { + o.c = c +} + +type kubeObj corev1.Service + +func (o kubeObj) GetConditions() []metav1.Condition { + return o.Status.Conditions +} + +func (o *kubeObj) SetConditions(c []metav1.Condition) { + if o == nil { + return + } + o.Status.Conditions = c +} + +func (o *kubeObj) DeepCopyObject() runtime.Object { + if o == nil { + return nil + } + c := kubeObj(*((*corev1.Service)(o)).DeepCopy()) + return &c +} diff --git a/pkg/common/conditions/conditions_test.go b/pkg/common/conditions/conditions_test.go new file mode 100644 index 0000000000..8a84aae6a6 --- /dev/null +++ b/pkg/common/conditions/conditions_test.go @@ -0,0 +1,75 @@ +/* +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. +*/ + +//nolint:all +package conditions + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestDelete(t *testing.T) { + testCases := []struct { + name string + in Conditions + out Conditions + }{ + { + name: "nil input", + in: nil, + out: nil, + }, + { + name: "type to delete does not exist", + in: Conditions{ + { + Type: "Hello", + }, + }, + out: Conditions{ + { + Type: "Hello", + }, + }, + }, + { + name: "type to delete does exist", + in: Conditions{ + { + Type: "Hello", + }, + { + Type: ReadyConditionType, + }, + }, + out: Conditions{ + { + Type: "Hello", + }, + }, + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(tc.in.Delete(ReadyConditionType)).To(Equal(tc.out)) + }) + } +} diff --git a/pkg/common/conditions/getter.go b/pkg/common/conditions/getter.go new file mode 100644 index 0000000000..72084a43e4 --- /dev/null +++ b/pkg/common/conditions/getter.go @@ -0,0 +1,216 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Getter interface defines methods that an object should implement in order to +// use the conditions package for getting conditions. +type Getter interface { + // GetConditions returns the list of conditions for a cluster API object. + GetConditions() []metav1.Condition +} + +// Get returns the condition with the given type, otherwise nil is returned. +func Get(from Getter, t string) *metav1.Condition { + return c4g(from).Get(t) +} + +// Has returns true if a condition with the given type exists. +func Has(from Getter, t string) bool { + return c4g(from).Has(t) +} + +// IsTrue returns true if the condition with the given type exists and is +// True, otherwise false is returned. +func IsTrue(from Getter, t string) bool { + return c4g(from).IsTrue(t) +} + +// IsFalse returns true if the condition with the given type exists and is +// False, otherwise false is returned. +func IsFalse(from Getter, t string) bool { + return c4g(from).IsFalse(t) +} + +// IsUnknown returns true if the condition with the given type exists and is +// Unknown, otherwise false is returned. +func IsUnknown(from Getter, t string) bool { + return c4g(from).IsUnknown(t) +} + +// GetReason returns the condition's reason or an empty string if the condition +// does not exist. +func GetReason(from Getter, t string) string { + return c4g(from).GetReason(t) +} + +// GetMessage returns the condition's message or an empty string if the +// condition does not exist. +func GetMessage(from Getter, t string) string { + return c4g(from).GetMessage(t) +} + +// GetLastTransitionTime returns the condition's last transition time or a nil +// value if the condition does not exist. +func GetLastTransitionTime(from Getter, t string) *metav1.Time { + return c4g(from).GetLastTransitionTime(t) +} + +// summary returns a Ready condition with the summary of all the conditions existing +// on an object. If the object does not have other conditions, no summary condition is generated. +func summary(from Getter, options ...MergeOption) *metav1.Condition { + conditions := from.GetConditions() + + mergeOpt := &mergeOptions{} + for _, o := range options { + o(mergeOpt) + } + + // Identifies the conditions in scope for the Summary by taking all the existing conditions except Ready, + // or, if a list of conditions types is specified, only the conditions the condition in that list. + conditionsInScope := make([]localizedCondition, 0, len(conditions)) + for i := range conditions { + c := conditions[i] + if c.Type == ReadyConditionType { + continue + } + + if mergeOpt.conditionTypes != nil { + found := false + for _, t := range mergeOpt.conditionTypes { + if c.Type == t { + found = true + break + } + } + if !found { + continue + } + } + + conditionsInScope = append(conditionsInScope, localizedCondition{ + Condition: &c, + Getter: from, + }) + } + + // If it is required to add a step counter only if a subset of condition exists, check if the conditions + // in scope are included in this subset or not. + if mergeOpt.addStepCounterIfOnlyConditionTypes != nil { + for _, c := range conditionsInScope { + found := false + for _, t := range mergeOpt.addStepCounterIfOnlyConditionTypes { + if c.Type == t { + found = true + break + } + } + if !found { + mergeOpt.addStepCounter = false + break + } + } + } + + // If it is required to add a step counter, determine the total number of conditions defaulting + // to the selected conditions or, if defined, to the total number of conditions type to be considered. + if mergeOpt.addStepCounter { + mergeOpt.stepCounter = len(conditionsInScope) + if mergeOpt.conditionTypes != nil { + mergeOpt.stepCounter = len(mergeOpt.conditionTypes) + } + if mergeOpt.addStepCounterIfOnlyConditionTypes != nil { + mergeOpt.stepCounter = len(mergeOpt.addStepCounterIfOnlyConditionTypes) + } + } + + return merge(conditionsInScope, ReadyConditionType, mergeOpt) +} + +// mirrorOptions allows to set options for the mirror operation. +type mirrorOptions struct { + fallbackTo *bool + fallbackReason string + fallbackMessage string +} + +// MirrorOptions defines an option for mirroring conditions. +type MirrorOptions func(*mirrorOptions) + +// WithFallbackValue specify a fallback value to use in case the mirrored condition does not exists; +// in case the fallbackValue is false, given values for reason, severity and message will be used. +func WithFallbackValue(fallbackValue bool, reason string, message string) MirrorOptions { + return func(c *mirrorOptions) { + c.fallbackTo = &fallbackValue + c.fallbackReason = reason + c.fallbackMessage = message + } +} + +// mirror mirrors the Ready condition from a dependent object into the target condition; +// if the Ready condition does not exists in the source object, no target conditions is generated. +func mirror(from Getter, targetCondition string, options ...MirrorOptions) *metav1.Condition { + mirrorOpt := &mirrorOptions{} + for _, o := range options { + o(mirrorOpt) + } + + condition := Get(from, ReadyConditionType) + + if mirrorOpt.fallbackTo != nil && condition == nil { + switch *mirrorOpt.fallbackTo { + case true: + condition = TrueCondition(targetCondition) + case false: + condition = FalseCondition(targetCondition, mirrorOpt.fallbackReason, "%s", mirrorOpt.fallbackMessage) + } + } + + if condition != nil { + condition.Type = targetCondition + } + + return condition +} + +// Aggregates all the Ready condition from a list of dependent objects into the target object; +// if the Ready condition does not exists in one of the source object, the object is excluded from +// the aggregation; if none of the source object have ready condition, no target conditions is generated. +func aggregate(from []Getter, targetCondition string, options ...MergeOption) *metav1.Condition { + conditionsInScope := make([]localizedCondition, 0, len(from)) + for i := range from { + condition := Get(from[i], ReadyConditionType) + + conditionsInScope = append(conditionsInScope, localizedCondition{ + Condition: condition, + Getter: from[i], + }) + } + + mergeOpt := &mergeOptions{ + addStepCounter: true, + stepCounter: len(from), + } + for _, o := range options { + o(mergeOpt) + } + return merge(conditionsInScope, targetCondition, mergeOpt) +} diff --git a/pkg/common/conditions/getter_test.go b/pkg/common/conditions/getter_test.go new file mode 100644 index 0000000000..6acbf3a2ef --- /dev/null +++ b/pkg/common/conditions/getter_test.go @@ -0,0 +1,321 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "errors" + "testing" + + . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" + "github.com/onsi/gomega/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var ( + nil1 *metav1.Condition + true1 = TrueCondition("true1") + unknown1 = UnknownCondition("unknown1", "reason unknown1", "message unknown1") + falseInfo1 = FalseCondition("falseInfo1", "reason falseInfo1", "message falseInfo1") + falseWarning1 = FalseCondition("falseWarning1", "reason falseWarning1", "message falseWarning1") + falseError1 = FalseCondition("falseError1", "reason falseError1", "message falseError1") +) + +func TestGetAndHas(t *testing.T) { + g := NewWithT(t) + + var obj nonKubeObj + + g.Expect(Has(&obj, "conditionBaz")).To(BeFalse()) + g.Expect(Get(&obj, "conditionBaz")).To(BeNil()) + + obj.SetConditions(conditionList(TrueCondition("conditionBaz"))) + + g.Expect(Has(&obj, "conditionBaz")).To(BeTrue()) + g.Expect(Get(&obj, "conditionBaz")).To(haveSameStateOf(TrueCondition("conditionBaz"))) +} + +func TestIsMethods(t *testing.T) { + g := NewWithT(t) + + obj := getterWithConditions(nil1, true1, unknown1, falseInfo1, falseWarning1, falseError1) + + // test isTrue + g.Expect(IsTrue(obj, "nil1")).To(BeFalse()) + g.Expect(IsTrue(obj, "true1")).To(BeTrue()) + g.Expect(IsTrue(obj, "falseInfo1")).To(BeFalse()) + g.Expect(IsTrue(obj, "unknown1")).To(BeFalse()) + + // test isFalse + g.Expect(IsFalse(obj, "nil1")).To(BeFalse()) + g.Expect(IsFalse(obj, "true1")).To(BeFalse()) + g.Expect(IsFalse(obj, "falseInfo1")).To(BeTrue()) + g.Expect(IsFalse(obj, "unknown1")).To(BeFalse()) + + // test isUnknown + g.Expect(IsUnknown(obj, "nil1")).To(BeTrue()) + g.Expect(IsUnknown(obj, "true1")).To(BeFalse()) + g.Expect(IsUnknown(obj, "falseInfo1")).To(BeFalse()) + g.Expect(IsUnknown(obj, "unknown1")).To(BeTrue()) + + // test GetReason + g.Expect(GetReason(obj, "nil1")).To(Equal("")) + g.Expect(GetReason(obj, "falseInfo1")).To(Equal("reason falseInfo1")) + + // test GetMessage + g.Expect(GetMessage(obj, "nil1")).To(Equal("")) + g.Expect(GetMessage(obj, "falseInfo1")).To(Equal("message falseInfo1")) + + // test GetMessage + g.Expect(GetLastTransitionTime(obj, "nil1")).To(BeNil()) + g.Expect(GetLastTransitionTime(obj, "falseInfo1")).ToNot(BeNil()) +} + +func TestMirror(t *testing.T) { + foo := FalseCondition("foo", "reason foo", "message foo") + ready := TrueCondition(ReadyConditionType) + readyBar := ready.DeepCopy() + readyBar.Type = "bar" + + tests := []struct { + name string + from Getter + t string + want *metav1.Condition + }{ + { + name: "Returns nil when the ready condition does not exists", + from: getterWithConditions(foo), + want: nil, + }, + { + name: "Returns ready condition from source", + from: getterWithConditions(ready, foo), + t: "bar", + want: readyBar, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := mirror(tt.from, tt.t) + if tt.want == nil { + g.Expect(got).To(BeNil()) + return + } + g.Expect(got).To(haveSameStateOf(tt.want)) + }) + } +} + +func TestSummary(t *testing.T) { + foo := TrueCondition("foo") + bar := FalseCondition("bar", "reason falseInfo1", "message falseInfo1") + baz := FalseCondition("baz", "reason falseInfo2", "message falseInfo2") + existingReady := FalseCondition(ReadyConditionType, "reason falseError1", "message falseError1") // NB. existing ready has higher priority than other conditions + + tests := []struct { + name string + from Getter + options []MergeOption + want *metav1.Condition + }{ + { + name: "Returns nil when there are no conditions to summarize", + from: getterWithConditions(), + want: nil, + }, + { + name: "Returns ready condition with the summary of existing conditions (with default options)", + from: getterWithConditions(foo, bar), + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "message falseInfo1"), + }, + { + name: "Returns ready condition with the summary of existing conditions (using WithStepCounter options)", + from: getterWithConditions(foo, bar), + options: []MergeOption{WithStepCounter()}, + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "1 of 2 completed"), + }, + { + name: "Returns ready condition with the summary of existing conditions (using WithStepCounterIf options)", + from: getterWithConditions(foo, bar), + options: []MergeOption{WithStepCounterIf(false)}, + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "message falseInfo1"), + }, + { + name: "Returns ready condition with the summary of existing conditions (using WithStepCounterIf options)", + from: getterWithConditions(foo, bar), + options: []MergeOption{WithStepCounterIf(true)}, + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "1 of 2 completed"), + }, + { + name: "Returns ready condition with the summary of existing conditions (using WithStepCounterIf and WithStepCounterIfOnly options)", + from: getterWithConditions(bar), + options: []MergeOption{WithStepCounter(), WithStepCounterIfOnly("bar")}, + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "0 of 1 completed"), + }, + { + name: "Returns ready condition with the summary of existing conditions (using WithStepCounterIf and WithStepCounterIfOnly options)", + from: getterWithConditions(foo, bar), + options: []MergeOption{WithStepCounter(), WithStepCounterIfOnly("foo")}, + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "message falseInfo1"), + }, + { + name: "Returns ready condition with the summary of selected conditions (using WithConditions options)", + from: getterWithConditions(foo, bar), + options: []MergeOption{WithConditions("foo")}, // bar should be ignored + want: TrueCondition(ReadyConditionType), + }, + { + name: "Returns ready condition with the summary of selected conditions (using WithConditions and WithStepCounter options)", + from: getterWithConditions(foo, bar, baz), + options: []MergeOption{WithConditions("foo", "bar"), WithStepCounter()}, // baz should be ignored, total steps should be 2 + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "1 of 2 completed"), + }, + { + name: "Returns ready condition with the summary of selected conditions (using WithConditions and WithStepCounterIfOnly options)", + from: getterWithConditions(bar), + options: []MergeOption{WithConditions("bar", "baz"), WithStepCounter(), WithStepCounterIfOnly("bar")}, // there is only bar, the step counter should be set and counts only a subset of conditions + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "0 of 1 completed"), + }, + { + name: "Returns ready condition with the summary of selected conditions (using WithConditions and WithStepCounterIfOnly options - with inconsistent order between the two)", + from: getterWithConditions(bar), + options: []MergeOption{WithConditions("baz", "bar"), WithStepCounter(), WithStepCounterIfOnly("bar", "baz")}, // conditions in WithStepCounterIfOnly could be in different order than in WithConditions + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "0 of 2 completed"), + }, + { + name: "Returns ready condition with the summary of selected conditions (using WithConditions and WithStepCounterIfOnly options)", + from: getterWithConditions(bar, baz), + options: []MergeOption{WithConditions("bar", "baz"), WithStepCounter(), WithStepCounterIfOnly("bar")}, // there is also baz, so the step counter should not be set + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "message falseInfo1"), + }, + { + name: "Ready condition respects merge order", + from: getterWithConditions(bar, baz), + options: []MergeOption{WithConditions("baz", "bar")}, // baz should take precedence on bar + want: FalseCondition(ReadyConditionType, "reason falseInfo2", "message falseInfo2"), + }, + { + name: "Ignores existing Ready condition when computing the summary", + from: getterWithConditions(existingReady, foo, bar), + want: FalseCondition(ReadyConditionType, "reason falseInfo1", "message falseInfo1"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := summary(tt.from, tt.options...) + if tt.want == nil { + g.Expect(got).To(BeNil()) + return + } + g.Expect(got).To(haveSameStateOf(tt.want)) + }) + } +} + +func TestAggregate(t *testing.T) { + ready1 := TrueCondition(ReadyConditionType) + ready2 := FalseCondition(ReadyConditionType, "reason falseInfo1", "message falseInfo1") + bar := FalseCondition("bar", "reason falseError1", "message falseError1") // NB. bar has higher priority than other conditions + + tests := []struct { + name string + from []Getter + t string + want *metav1.Condition + }{ + { + name: "Returns nil when there are no conditions to aggregate", + from: []Getter{}, + want: nil, + }, + { + name: "Returns foo condition with the aggregation of object's ready conditions", + from: []Getter{ + getterWithConditions(ready1), + getterWithConditions(ready1), + getterWithConditions(ready2, bar), + getterWithConditions(), + getterWithConditions(bar), + }, + t: "foo", + want: FalseCondition("foo", "reason falseInfo1", "2 of 5 completed"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := aggregate(tt.from, tt.t) + if tt.want == nil { + g.Expect(got).To(BeNil()) + return + } + g.Expect(got).To(haveSameStateOf(tt.want)) + }) + } +} + +func getterWithConditions(conditions ...*metav1.Condition) Getter { + var obj nonKubeObj + obj.SetConditions(conditionList(conditions...)) + return obj +} + +func conditionList(conditions ...*metav1.Condition) []metav1.Condition { + var cs []metav1.Condition + for _, x := range conditions { + if x != nil { + cs = append(cs, *x) + } + } + return cs +} + +func haveSameStateOf(expected *metav1.Condition) types.GomegaMatcher { + return &ConditionMatcher{ + Expected: expected, + } +} + +type ConditionMatcher struct { + Expected *metav1.Condition +} + +func (matcher *ConditionMatcher) Match(actual interface{}) (success bool, err error) { + actualCondition, ok := actual.(*metav1.Condition) + if !ok { + return false, errors.New("Value should be a condition") + } + + return hasSameState(actualCondition, matcher.Expected), nil +} + +func (matcher *ConditionMatcher) FailureMessage(actual interface{}) (message string) { + return format.Message(actual, "to have the same state of", matcher.Expected) +} +func (matcher *ConditionMatcher) NegatedFailureMessage(actual interface{}) (message string) { + return format.Message(actual, "not to have the same state of", matcher.Expected) +} diff --git a/pkg/common/conditions/matcher.go b/pkg/common/conditions/matcher.go new file mode 100644 index 0000000000..5acab9b86e --- /dev/null +++ b/pkg/common/conditions/matcher.go @@ -0,0 +1,99 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "fmt" + + "github.com/onsi/gomega" + "github.com/onsi/gomega/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// MatchConditions returns a custom matcher to check equality of metav1.Conditions. +func MatchConditions(expected []metav1.Condition) types.GomegaMatcher { + return &matchConditions{ + expected: expected, + } +} + +type matchConditions struct { + expected []metav1.Condition +} + +func (m matchConditions) Match(actual interface{}) (success bool, err error) { + elems := make([]interface{}, 0, len(m.expected)) + for _, condition := range m.expected { + elems = append(elems, MatchCondition(condition)) + } + + return gomega.ConsistOf(elems).Match(actual) +} + +func (m matchConditions) FailureMessage(actual interface{}) (message string) { + return fmt.Sprintf("expected\n\t%#v\nto match\n\t%#v\n", actual, m.expected) +} + +func (m matchConditions) NegatedFailureMessage(actual interface{}) (message string) { + 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 metav1.Condition. +func MatchCondition(expected metav1.Condition) types.GomegaMatcher { + return &matchCondition{ + expected: expected, + } +} + +type matchCondition struct { + expected metav1.Condition +} + +func (m matchCondition) Match(actual interface{}) (success bool, err error) { + actualCondition, ok := actual.(metav1.Condition) + if !ok { + return false, fmt.Errorf("actual should be of type Condition") + } + + ok, err = gomega.Equal(m.expected.Type).Match(actualCondition.Type) + if !ok { + return ok, err + } + ok, err = gomega.Equal(m.expected.Status).Match(actualCondition.Status) + if !ok { + return ok, err + } + ok, err = gomega.Equal(m.expected.Reason).Match(actualCondition.Reason) + if !ok { + return ok, err + } + ok, err = gomega.Equal(m.expected.Message).Match(actualCondition.Message) + if !ok { + return ok, err + } + + return ok, err +} + +func (m matchCondition) FailureMessage(actual interface{}) (message string) { + return fmt.Sprintf("expected\n\t%#v\nto match\n\t%#v\n", actual, m.expected) +} + +func (m matchCondition) NegatedFailureMessage(actual interface{}) (message string) { + return fmt.Sprintf("expected\n\t%#v\nto not match\n\t%#v\n", actual, m.expected) +} diff --git a/pkg/common/conditions/matcher_test.go b/pkg/common/conditions/matcher_test.go new file mode 100644 index 0000000000..c4eadd782e --- /dev/null +++ b/pkg/common/conditions/matcher_test.go @@ -0,0 +1,274 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestMatchConditions(t *testing.T) { + testCases := []struct { + name string + actual interface{} + expected []metav1.Condition + expectMatch bool + }{ + { + name: "with an empty conditions", + actual: []metav1.Condition{}, + expected: []metav1.Condition{}, + expectMatch: true, + }, + { + name: "with matching conditions", + actual: []metav1.Condition{ + { + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + }, + expected: []metav1.Condition{ + { + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + }, + expectMatch: true, + }, + { + name: "with non-matching conditions", + actual: []metav1.Condition{ + { + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + { + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + }, + expected: []metav1.Condition{ + { + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + { + Type: "different", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "different", + Message: "different", + }, + }, + expectMatch: false, + }, + { + name: "with a different number of conditions", + actual: []metav1.Condition{ + { + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + { + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + }, + expected: []metav1.Condition{ + { + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + }, + expectMatch: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + if tc.expectMatch { + g.Expect(tc.actual).To(MatchConditions(tc.expected)) + } else { + g.Expect(tc.actual).ToNot(MatchConditions(tc.expected)) + } + }) + } +} + +func TestMatchCondition(t *testing.T) { + testCases := []struct { + name string + actual interface{} + expected metav1.Condition + expectMatch bool + }{ + { + name: "with an empty condition", + actual: metav1.Condition{}, + expected: metav1.Condition{}, + expectMatch: true, + }, + { + name: "with a matching condition", + actual: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expected: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expectMatch: true, + }, + { + name: "with a different time", + actual: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expected: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Time{}, + Reason: "reason", + Message: "message", + }, + expectMatch: true, + }, + { + name: "with a different type", + actual: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expected: metav1.Condition{ + Type: "different", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expectMatch: false, + }, + { + name: "with a different status", + actual: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expected: metav1.Condition{ + Type: "type", + Status: metav1.ConditionFalse, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expectMatch: false, + }, + { + name: "with a different reason", + actual: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expected: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "different", + Message: "message", + }, + expectMatch: false, + }, + { + name: "with a different message", + actual: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "message", + }, + expected: metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Reason: "reason", + Message: "different", + }, + expectMatch: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + if tc.expectMatch { + g.Expect(tc.actual).To(MatchCondition(tc.expected)) + } else { + g.Expect(tc.actual).ToNot(MatchCondition(tc.expected)) + } + }) + } +} diff --git a/pkg/common/conditions/merge.go b/pkg/common/conditions/merge.go new file mode 100644 index 0000000000..d16f9acb01 --- /dev/null +++ b/pkg/common/conditions/merge.go @@ -0,0 +1,195 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "sort" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// localizedCondition defines a condition with the information of the object the conditions +// was originated from. +type localizedCondition struct { + *metav1.Condition + Getter +} + +// merge a list of condition into a single one. +// This operation is designed to ensure visibility of the most relevant conditions for defining the +// operational state of a component. E.g. If there is one error in the condition list, this one takes +// priority over the other conditions and it is should be reflected in the target condition. +// +// More specifically: +// 1. Conditions are grouped by status, severity +// 2. The resulting condition groups are sorted according to the following priority: +// - P0 - Status=False, Severity=Error +// - P1 - Status=False, Severity=Warning +// - P2 - Status=False, Severity=Info +// - P3 - Status=True +// - P4 - Status=Unknown +// +// 3. The group with highest priority is used to determine status, severity and other info of the target condition. +// +// Please note that the last operation includes also the task of computing the Reason and the Message for the target +// condition; in order to complete such task some trade-off should be made, because there is no a golden rule +// for summarizing many Reason/Message into single Reason/Message. +// mergeOptions allows the user to adapt this process to the specific needs by exposing a set of merge strategies. +func merge(conditions []localizedCondition, targetCondition string, options *mergeOptions) *metav1.Condition { + g := getConditionGroups(conditions) + if len(g) == 0 { + return nil + } + + if g.TopGroup().status == metav1.ConditionTrue { + return TrueCondition(targetCondition) + } + + targetReason := getReason(g, options) + targetMessage := getMessage(g, options) + if g.TopGroup().status == metav1.ConditionFalse { + return FalseCondition(targetCondition, targetReason, "%s", targetMessage) + } + return UnknownCondition(targetCondition, targetReason, "%s", targetMessage) +} + +// getConditionGroups groups a list of conditions according to status, severity values. +// Additionally, the resulting groups are sorted by mergePriority. +func getConditionGroups(conditions []localizedCondition) conditionGroups { + groups := conditionGroups{} + + for _, condition := range conditions { + if condition.Condition == nil { + continue + } + + added := false + for i := range groups { + if groups[i].status == condition.Status { + groups[i].conditions = append(groups[i].conditions, condition) + added = true + break + } + } + if !added { + groups = append(groups, conditionGroup{ + conditions: []localizedCondition{condition}, + status: condition.Status, + }) + } + } + + // sort groups by priority + sort.Sort(groups) + + // sorts conditions in the TopGroup so we ensure predictable result for merge strategies. + // condition are sorted using the same lexicographic order used by Set; in case two conditions + // have the same type, condition are sorted using according to the alphabetical order of the source object name. + if len(groups) > 0 { + sort.Slice(groups[0].conditions, func(i, j int) bool { + a := groups[0].conditions[i] + b := groups[0].conditions[j] + if a.Type != b.Type { + return lexicographicLess(a.Condition, b.Condition) + } + + ta, aok := a.Getter.(metav1.Object) + tb, bok := b.Getter.(metav1.Object) + + switch { + case aok && bok: + return ta.GetName() < tb.GetName() + case bok: + return true + default: + return false + } + }) + } + + return groups +} + +// conditionGroups provides supports for grouping a list of conditions to be +// merged into a single condition. ConditionGroups can be sorted by mergePriority. +type conditionGroups []conditionGroup + +func (g conditionGroups) Len() int { + return len(g) +} + +func (g conditionGroups) Less(i, j int) bool { + return g[i].mergePriority() < g[j].mergePriority() +} + +func (g conditionGroups) Swap(i, j int) { + g[i], g[j] = g[j], g[i] +} + +// TopGroup returns the condition group with the highest mergePriority. +func (g conditionGroups) TopGroup() *conditionGroup { + if len(g) == 0 { + return nil + } + return &g[0] +} + +// TrueGroup returns the condition group with status True, if any. +func (g conditionGroups) TrueGroup() *conditionGroup { + return g.getByStatusAndSeverity(metav1.ConditionTrue) +} + +// FalseGroup returns the condition group with status False, if any. +func (g conditionGroups) FalseGroup() *conditionGroup { + return g.getByStatusAndSeverity(metav1.ConditionFalse) +} + +func (g conditionGroups) getByStatusAndSeverity(status metav1.ConditionStatus) *conditionGroup { + if len(g) == 0 { + return nil + } + for _, group := range g { + if group.status == status { + return &group + } + } + return nil +} + +// conditionGroup define a group of conditions with the same status, +// and thus with the same priority when merging into a Ready condition. +type conditionGroup struct { + status metav1.ConditionStatus + conditions []localizedCondition +} + +// mergePriority provides a priority value for the status and severity tuple that identifies this +// condition group. The mergePriority value allows an easier sorting of conditions groups. +func (g conditionGroup) mergePriority() int { + switch g.status { + case metav1.ConditionFalse: + return 2 + case metav1.ConditionTrue: + return 3 + case metav1.ConditionUnknown: + return 4 + } + + // this should never happen + return 99 +} diff --git a/pkg/common/conditions/merge_strategies.go b/pkg/common/conditions/merge_strategies.go new file mode 100644 index 0000000000..8827fa98c5 --- /dev/null +++ b/pkg/common/conditions/merge_strategies.go @@ -0,0 +1,182 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "fmt" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +// mergeOptions allows to set strategies for merging a set of conditions into a single condition, +// and more specifically for computing the target Reason and the target Message. +type mergeOptions struct { + conditionTypes []string + addSourceRef bool + addStepCounter bool + addStepCounterIfOnlyConditionTypes []string + stepCounter int +} + +// MergeOption defines an option for computing a summary of conditions. +type MergeOption func(*mergeOptions) + +// WithConditions instructs merge about the condition types to consider when doing a merge operation; +// if this option is not specified, all the conditions (excepts Ready) will be considered. This is required +// so we can provide some guarantees about the semantic of the target condition without worrying about +// side effects if someone or something adds custom conditions to the objects. +// +// NOTE: The order of conditions types defines the priority for determining the Reason and Message for the +// target condition. +// IMPORTANT: This options works only while generating the Summary condition. +func WithConditions(t ...string) MergeOption { + return func(c *mergeOptions) { + c.conditionTypes = t + } +} + +// WithStepCounter instructs merge to add a "x of y completed" string to the message, +// where x is the number of conditions with Status=true and y is the number of conditions in scope. +func WithStepCounter() MergeOption { + return func(c *mergeOptions) { + c.addStepCounter = true + } +} + +// WithStepCounterIf adds a step counter if the value is true. +// This can be used e.g. to add a step counter only if the object is not being deleted. +// +// IMPORTANT: This options works only while generating the Summary condition. +func WithStepCounterIf(value bool) MergeOption { + return func(c *mergeOptions) { + c.addStepCounter = value + } +} + +// WithStepCounterIfOnly ensure a step counter is show only if a subset of condition exists. +// This applies for example on Machines, where we want to use +// the step counter notation while provisioning the machine, but then we want to move away from this notation +// as soon as the machine is provisioned and e.g. a Machine health check condition is generated +// +// IMPORTANT: This options requires WithStepCounter or WithStepCounterIf to be set. +// IMPORTANT: This options works only while generating the Summary condition. +func WithStepCounterIfOnly(t ...string) MergeOption { + return func(c *mergeOptions) { + c.addStepCounterIfOnlyConditionTypes = t + } +} + +// AddSourceRef instructs merge to add info about the originating object to the target Reason. +func AddSourceRef() MergeOption { + return func(c *mergeOptions) { + c.addSourceRef = true + } +} + +// getReason returns the reason to be applied to the condition resulting by merging a set of condition groups. +// The reason is computed according to the given mergeOptions. +func getReason(groups conditionGroups, options *mergeOptions) string { + return getFirstReason(groups, options.conditionTypes, options.addSourceRef) +} + +// getFirstReason returns the first reason from the ordered list of conditions in the top group. +// If required, the reason gets localized with the source object reference. +func getFirstReason(g conditionGroups, order []string, addSourceRef bool) string { + if condition := getFirstCondition(g, order); condition != nil { + reason := condition.Reason + if addSourceRef { + return localizeReason(reason, condition.Getter) + } + return reason + } + return "" +} + +// localizeReason adds info about the originating object to the target Reason. +func localizeReason(reason string, from Getter) string { + if strings.Contains(reason, "@") { + return reason + } + + tf1, ok1 := from.(runtime.Object) + tf2, ok2 := from.(metav1.Object) + + if !ok1 || !ok2 { + return reason + } + + return fmt.Sprintf("%s @ %s/%s", + reason, + tf1.GetObjectKind().GroupVersionKind().Kind, + tf2.GetName()) +} + +// getMessage returns the message to be applied to the condition resulting by merging a set of condition groups. +// The message is computed according to the given mergeOptions, but in case of errors or warning a +// summary of existing errors is automatically added. +func getMessage(groups conditionGroups, options *mergeOptions) string { + if options.addStepCounter { + return getStepCounterMessage(groups, options.stepCounter) + } + + return getFirstMessage(groups, options.conditionTypes) +} + +// getStepCounterMessage returns a message "x of y completed", where x is the number of conditions +// with Status=true and y is the number passed to this method. +func getStepCounterMessage(groups conditionGroups, to int) string { + ct := 0 + if trueGroup := groups.TrueGroup(); trueGroup != nil { + ct = len(trueGroup.conditions) + } + return fmt.Sprintf("%d of %d completed", ct, to) +} + +// getFirstMessage returns the message from the ordered list of conditions in the top group. +func getFirstMessage(groups conditionGroups, order []string) string { + if condition := getFirstCondition(groups, order); condition != nil { + return condition.Message + } + return "" +} + +// getFirstCondition returns a first condition from the ordered list of conditions in the top group. +func getFirstCondition(g conditionGroups, priority []string) *localizedCondition { + topGroup := g.TopGroup() + if topGroup == nil { + return nil + } + + switch len(topGroup.conditions) { + case 0: + return nil + case 1: + return &topGroup.conditions[0] + default: + for _, p := range priority { + for _, c := range topGroup.conditions { + if c.Type == p { + return &c + } + } + } + return &topGroup.conditions[0] + } +} diff --git a/pkg/common/conditions/merge_strategies_test.go b/pkg/common/conditions/merge_strategies_test.go new file mode 100644 index 0000000000..b183254cd8 --- /dev/null +++ b/pkg/common/conditions/merge_strategies_test.go @@ -0,0 +1,171 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGetStepCounterMessage(t *testing.T) { + testCases := []struct { + name string + obj Setter + }{ + { + name: "Kube object", + obj: &kubeObj{}, + }, + { + name: "Non-kube object", + obj: &nonKubeObj{}, + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + groups := getConditionGroups(conditionsWithSource(tc.obj, + nil1, + true1, true1, + falseInfo1, + falseWarning1, falseWarning1, + falseError1, + unknown1, + )) + + got := getStepCounterMessage(groups, 8) + + // step count message should report n° if true conditions over to number + g.Expect(got).To(Equal("2 of 8 completed")) + }) + } + +} + +func TestLocalizeReason(t *testing.T) { + testCases := []struct { + name string + obj Setter + exp string + }{ + { + name: "Kube object", + obj: &kubeObj{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + }, + }, + exp: "foo @ Service/my-service", + }, + { + name: "Non-kube object", + obj: &nonKubeObj{}, + exp: "foo", + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + // localize should reason location + got := localizeReason("foo", tc.obj) + g.Expect(got).To(Equal(tc.exp)) + + // localize should not alter existing location + got = localizeReason("foo @ SomeKind/some-name", tc.obj) + g.Expect(got).To(Equal("foo @ SomeKind/some-name")) + }) + } +} + +func TestGetFirstReasonAndMessage(t *testing.T) { + + testCases := []struct { + name string + obj Setter + exp string + }{ + { + name: "Kube object", + obj: &kubeObj{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + }, + }, + exp: "falseBar @ Service/my-service", + }, + { + name: "Non-kube object", + obj: &nonKubeObj{}, + exp: "falseBar", + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + foo := FalseCondition("foo", "falseFoo", "message falseFoo") + bar := FalseCondition("bar", "falseBar", "message falseBar") + + groups := getConditionGroups(conditionsWithSource(tc.obj, foo, bar)) + + // getFirst should report first condition in lexicografical order if no order is specified + gotReason := getFirstReason(groups, nil, false) + g.Expect(gotReason).To(Equal("falseBar")) + gotMessage := getFirstMessage(groups, nil) + g.Expect(gotMessage).To(Equal("message falseBar")) + + // getFirst should report should respect order + gotReason = getFirstReason(groups, []string{"foo", "bar"}, false) + g.Expect(gotReason).To(Equal("falseFoo")) + gotMessage = getFirstMessage(groups, []string{"foo", "bar"}) + g.Expect(gotMessage).To(Equal("message falseFoo")) + + // getFirst should report should respect order in case of missing conditions + gotReason = getFirstReason(groups, []string{"missingBaz", "foo", "bar"}, false) + g.Expect(gotReason).To(Equal("falseFoo")) + gotMessage = getFirstMessage(groups, []string{"missingBaz", "foo", "bar"}) + g.Expect(gotMessage).To(Equal("message falseFoo")) + + // getFirst should fallback to first condition if none of the conditions in the list exists + gotReason = getFirstReason(groups, []string{"missingBaz"}, false) + g.Expect(gotReason).To(Equal("falseBar")) + gotMessage = getFirstMessage(groups, []string{"missingBaz"}) + g.Expect(gotMessage).To(Equal("message falseBar")) + + // getFirstReason should localize reason if required + gotReason = getFirstReason(groups, nil, true) + g.Expect(gotReason).To(Equal(tc.exp)) + }) + } +} diff --git a/pkg/common/conditions/merge_test.go b/pkg/common/conditions/merge_test.go new file mode 100644 index 0000000000..fd0368d3fa --- /dev/null +++ b/pkg/common/conditions/merge_test.go @@ -0,0 +1,160 @@ +/* +Copyright 2025 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. +*/ + +//nolint:scopelint +package conditions + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestNewConditionsGroup(t *testing.T) { + testCases := []struct { + name string + setter Setter + }{ + { + name: "Kube object", + setter: &kubeObj{}, + }, + { + name: "Non-kube object", + setter: &nonKubeObj{}, + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + got := getConditionGroups(conditionsWithSource( + tc.setter, + []*metav1.Condition{ + nil1, + true1, + true1, + falseInfo1, + falseWarning1, + falseWarning1, + falseError1, + unknown1, + }...)) + + g.Expect(got).ToNot(BeNil()) + g.Expect(got).To(HaveLen(3)) + + // The top group should be false and it should have four conditions + g.Expect(got.TopGroup().status).To(Equal(metav1.ConditionFalse)) + g.Expect(got.TopGroup().conditions).To(HaveLen(4)) + + // The true group should be true and it should have two conditions + g.Expect(got.TrueGroup().status).To(Equal(metav1.ConditionTrue)) + g.Expect(got.TrueGroup().conditions).To(HaveLen(2)) + + // The error group should be false and it should have one condition + g.Expect(got.FalseGroup().status).To(Equal(metav1.ConditionFalse)) + g.Expect(got.FalseGroup().conditions).To(HaveLen(4)) + + // got[0] should be False and it should have four conditions + g.Expect(got[0].status).To(Equal(metav1.ConditionFalse)) + g.Expect(got[0].conditions).To(HaveLen(4)) + + // got[1] should be True and it should have two conditions + g.Expect(got[1].status).To(Equal(metav1.ConditionTrue)) + g.Expect(got[1].conditions).To(HaveLen(2)) + + // got[4] should be Unknown and it should have one condition + g.Expect(got[2].status).To(Equal(metav1.ConditionUnknown)) + g.Expect(got[2].conditions).To(HaveLen(1)) + }) + } +} + +func TestMergeRespectPriority(t *testing.T) { + tests := []struct { + name string + conditions []*metav1.Condition + want *metav1.Condition + }{ + { + name: "aggregate nil list return nil", + conditions: nil, + want: nil, + }, + { + name: "aggregate empty list return nil", + conditions: []*metav1.Condition{}, + want: nil, + }, + { + name: "When there is false/error it returns false/error", + conditions: []*metav1.Condition{falseError1, falseWarning1, falseInfo1, unknown1, true1}, + want: FalseCondition("foo", "reason falseError1", "message falseError1"), + }, + { + name: "When there is false/info and no false/error or false/warning, it returns false/info", + conditions: []*metav1.Condition{falseInfo1, unknown1, true1}, + want: FalseCondition("foo", "reason falseInfo1", "message falseInfo1"), + }, + { + name: "When there is true and no false/*, it returns info", + conditions: []*metav1.Condition{unknown1, true1}, + want: TrueCondition("foo"), + }, + { + name: "When there is unknown and no true or false/*, it returns unknown", + conditions: []*metav1.Condition{unknown1}, + want: UnknownCondition("foo", "reason unknown1", "message unknown1"), + }, + { + name: "nil conditions are ignored", + conditions: []*metav1.Condition{nil1, nil1, nil1}, + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := merge(conditionsWithSource(&kubeObj{}, tt.conditions...), "foo", &mergeOptions{}) + + if tt.want == nil { + g.Expect(got).To(BeNil()) + return + } + g.Expect(got).To(haveSameStateOf(tt.want)) + }) + } +} + +func conditionsWithSource(obj Setter, conditions ...*metav1.Condition) []localizedCondition { + obj.SetConditions(conditionList(conditions...)) + + ret := make([]localizedCondition, 0, len(conditions)) + for i := range conditions { + ret = append(ret, localizedCondition{ + Condition: conditions[i], + Getter: obj, + }) + } + + return ret +} diff --git a/pkg/common/conditions/patch.go b/pkg/common/conditions/patch.go new file mode 100644 index 0000000000..91914c0650 --- /dev/null +++ b/pkg/common/conditions/patch.go @@ -0,0 +1,201 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "fmt" + "reflect" + + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Patch defines a list of operations to change a list of conditions into another. +type Patch []PatchOperation + +// PatchOperation define an operation that changes a single condition. +type PatchOperation struct { + Before *metav1.Condition + After *metav1.Condition + Op PatchOperationType +} + +// PatchOperationType defines patch operation types. +type PatchOperationType string + +const ( + // AddConditionPatch defines an add condition patch operation. + AddConditionPatch PatchOperationType = "Add" + + // ChangeConditionPatch defines an change condition patch operation. + ChangeConditionPatch PatchOperationType = "Change" + + // RemoveConditionPatch defines a remove condition patch operation. + RemoveConditionPatch PatchOperationType = "Remove" +) + +// NewPatch returns the list of Patch required to align source conditions to after conditions. +func NewPatch(before Getter, after Getter) Patch { + var patch Patch + + // Identify AddCondition and ModifyCondition changes. + targetConditions := after.GetConditions() + for i := range targetConditions { + targetCondition := targetConditions[i] + currentCondition := Get(before, targetCondition.Type) + if currentCondition == nil { + patch = append(patch, PatchOperation{Op: AddConditionPatch, After: &targetCondition}) + continue + } + + if !reflect.DeepEqual(&targetCondition, currentCondition) { + patch = append(patch, PatchOperation{Op: ChangeConditionPatch, After: &targetCondition, Before: currentCondition}) + } + } + + // Identify RemoveCondition changes. + baseConditions := before.GetConditions() + for i := range baseConditions { + baseCondition := baseConditions[i] + targetCondition := Get(after, baseCondition.Type) + if targetCondition == nil { + patch = append(patch, PatchOperation{Op: RemoveConditionPatch, Before: &baseCondition}) + } + } + return patch +} + +// applyOptions allows to set strategies for patch apply. +type applyOptions struct { + ownedConditions []string + forceOverwrite bool +} + +func (o *applyOptions) isOwnedCondition(t string) bool { + for _, i := range o.ownedConditions { + if i == t { + return true + } + } + return false +} + +// ApplyOption defines an option for applying a condition patch. +type ApplyOption func(*applyOptions) + +// WithOwnedConditions allows to define condition types owned by the controller. +// In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller. +func WithOwnedConditions(t ...string) ApplyOption { + return func(c *applyOptions) { + c.ownedConditions = t + } +} + +// WithForceOverwrite In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller. +func WithForceOverwrite(v bool) ApplyOption { + return func(c *applyOptions) { + c.forceOverwrite = v + } +} + +// Apply executes a three-way merge of a list of Patch. +// When merge conflicts are detected (latest deviated from before in an incompatible way), an error is returned. +func (p Patch) Apply(latest Setter, options ...ApplyOption) error { + if len(p) == 0 { + return nil + } + + applyOpt := &applyOptions{} + for _, o := range options { + o(applyOpt) + } + + for _, conditionPatch := range p { + switch conditionPatch.Op { + case AddConditionPatch: + // If the conditions is owned, always keep the after value. + if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.After.Type) { + Set(latest, conditionPatch.After) + continue + } + + // If the condition is already on latest, check if latest and after agree on the change; if not, this is a conflict. + if latestCondition := Get(latest, conditionPatch.After.Type); latestCondition != nil { + // If latest and after agree on the change, then it is a conflict. + if !hasSameState(latestCondition, conditionPatch.After) { + return fmt.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/AddCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After)) + } + // otherwise, the latest is already as intended. + // NOTE: We are preserving LastTransitionTime from the latest in order to avoid altering the existing value. + continue + } + // If the condition does not exists on the latest, add the new after condition. + Set(latest, conditionPatch.After) + + case ChangeConditionPatch: + // If the conditions is owned, always keep the after value. + if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.After.Type) { + Set(latest, conditionPatch.After) + continue + } + + latestCondition := Get(latest, conditionPatch.After.Type) + + // If the condition does not exist anymore on the latest, this is a conflict. + if latestCondition == nil { + return fmt.Errorf("error patching conditions: The condition %q was deleted by a different process and this caused a merge/ChangeCondition conflict", conditionPatch.After.Type) + } + + // If the condition on the latest is different from the base condition, check if + // the after state corresponds to the desired value. If not this is a conflict (unless we should ignore conflicts for this condition type). + if !reflect.DeepEqual(latestCondition, conditionPatch.Before) { + if !hasSameState(latestCondition, conditionPatch.After) { + return fmt.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/ChangeCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After)) + } + // Otherwise the latest is already as intended. + // NOTE: We are preserving LastTransitionTime from the latest in order to avoid altering the existing value. + continue + } + // Otherwise apply the new after condition. + Set(latest, conditionPatch.After) + + case RemoveConditionPatch: + // If the conditions is owned, always keep the after value (condition should be deleted). + if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.Before.Type) { + Delete(latest, conditionPatch.Before.Type) + continue + } + + // If the condition is still on the latest, check if it is changed in the meantime; + // if so then this is a conflict. + if latestCondition := Get(latest, conditionPatch.Before.Type); latestCondition != nil { + if !hasSameState(latestCondition, conditionPatch.Before) { + return fmt.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/RemoveCondition conflict: %v", conditionPatch.Before.Type, cmp.Diff(latestCondition, conditionPatch.Before)) + } + } + // Otherwise the latest and after agreed on the delete operation, so there's nothing to change. + Delete(latest, conditionPatch.Before.Type) + } + } + return nil +} + +// IsZero returns true if the patch has no changes. +func (p Patch) IsZero() bool { + return len(p) == 0 +} diff --git a/pkg/common/conditions/patch_test.go b/pkg/common/conditions/patch_test.go new file mode 100644 index 0000000000..097501c5a3 --- /dev/null +++ b/pkg/common/conditions/patch_test.go @@ -0,0 +1,283 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "testing" + "time" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestNewPatch(t *testing.T) { + fooTrue := TrueCondition("foo") + fooFalse := FalseCondition("foo", "reason foo", "message foo") + + tests := []struct { + name string + before Getter + after Getter + want Patch + }{ + { + name: "No changes return empty patch", + before: getterWithConditions(), + after: getterWithConditions(), + want: nil, + }, + { + name: "No changes return empty patch", + before: getterWithConditions(fooTrue), + after: getterWithConditions(fooTrue), + want: nil, + }, + { + name: "Detects AddConditionPatch", + before: getterWithConditions(), + after: getterWithConditions(fooTrue), + want: Patch{ + { + Before: nil, + After: fooTrue, + Op: AddConditionPatch, + }, + }, + }, + { + name: "Detects ChangeConditionPatch", + before: getterWithConditions(fooTrue), + after: getterWithConditions(fooFalse), + want: Patch{ + { + Before: fooTrue, + After: fooFalse, + Op: ChangeConditionPatch, + }, + }, + }, + { + name: "Detects RemoveConditionPatch", + before: getterWithConditions(fooTrue), + after: getterWithConditions(), + want: Patch{ + { + Before: fooTrue, + After: nil, + Op: RemoveConditionPatch, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := NewPatch(tt.before, tt.after) + + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func TestApply(t *testing.T) { + fooTrue := TrueCondition("foo") + fooFalse := FalseCondition("foo", "reason foo", "message foo") + fooFalse2 := FalseCondition("foo", "different reason foo", "message foo") + + tests := []struct { + name string + before Getter + after Getter + latest Setter + options []ApplyOption + want []metav1.Condition + wantErr bool + }{ + { + name: "No patch return same list", + before: getterWithConditions(fooTrue), + after: getterWithConditions(fooTrue), + latest: setterWithConditions(fooTrue), + want: conditionList(fooTrue), + wantErr: false, + }, + { + name: "Add: When a condition does not exists, it should add", + before: getterWithConditions(), + after: getterWithConditions(fooTrue), + latest: setterWithConditions(), + want: conditionList(fooTrue), + wantErr: false, + }, + { + name: "Add: When a condition already exists but without conflicts, it should add", + before: getterWithConditions(), + after: getterWithConditions(fooTrue), + latest: setterWithConditions(fooTrue), + want: conditionList(fooTrue), + wantErr: false, + }, + { + name: "Add: When a condition already exists but with conflicts, it should error", + before: getterWithConditions(), + after: getterWithConditions(fooTrue), + latest: setterWithConditions(fooFalse), + want: nil, + wantErr: true, + }, + { + name: "Add: When a condition already exists but with conflicts, it should not error if the condition is owned", + before: getterWithConditions(), + after: getterWithConditions(fooTrue), + latest: setterWithConditions(fooFalse), + options: []ApplyOption{WithOwnedConditions("foo")}, + want: conditionList(fooTrue), // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Remove: When a condition was already deleted, it should pass", + before: getterWithConditions(fooTrue), + after: getterWithConditions(), + latest: setterWithConditions(), + want: conditionList(), + wantErr: false, + }, + { + name: "Remove: When a condition already exists but without conflicts, it should delete", + before: getterWithConditions(fooTrue), + after: getterWithConditions(), + latest: setterWithConditions(fooTrue), + want: conditionList(), + wantErr: false, + }, + { + name: "Remove: When a condition already exists but with conflicts, it should error", + before: getterWithConditions(fooTrue), + after: getterWithConditions(), + latest: setterWithConditions(fooFalse), + want: nil, + wantErr: true, + }, + { + name: "Remove: When a condition already exists but with conflicts, it should not error if the condition is owned", + before: getterWithConditions(fooTrue), + after: getterWithConditions(), + latest: setterWithConditions(fooFalse), + options: []ApplyOption{WithOwnedConditions("foo")}, + want: conditionList(), // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Change: When a condition exists without conflicts, it should change", + before: getterWithConditions(fooTrue), + after: getterWithConditions(fooFalse), + latest: setterWithConditions(fooTrue), + want: conditionList(fooFalse), + wantErr: false, + }, + { + name: "Change: When a condition exists with conflicts but there is agreement on the final state, it should change", + before: getterWithConditions(fooFalse), + after: getterWithConditions(fooTrue), + latest: setterWithConditions(fooTrue), + want: conditionList(fooTrue), + wantErr: false, + }, + + { + name: "Change: When a condition exists with conflicts but there is no agreement on the final state, it should error", + before: getterWithConditions(fooFalse2), + after: getterWithConditions(fooFalse), + latest: setterWithConditions(fooTrue), + want: nil, + wantErr: true, + }, + { + name: "Change: When a condition exists with conflicts but there is no agreement on the final state, it should not error if the condition is owned", + before: getterWithConditions(fooFalse2), + after: getterWithConditions(fooFalse), + latest: setterWithConditions(fooTrue), + options: []ApplyOption{WithOwnedConditions("foo")}, + want: conditionList(fooFalse), // after condition should be kept in case of error + wantErr: false, + }, + + { + name: "Change: When a condition was deleted, it should error", + before: getterWithConditions(fooTrue), + after: getterWithConditions(fooFalse), + latest: setterWithConditions(), + want: nil, + wantErr: true, + }, + { + name: "Change: When a condition was deleted, it should not error if the condition is owned", + before: getterWithConditions(fooTrue), + after: getterWithConditions(fooFalse), + latest: setterWithConditions(), + options: []ApplyOption{WithOwnedConditions("foo")}, + want: conditionList(fooFalse), // after condition should be kept in case of error + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + patch := NewPatch(tt.before, tt.after) + + err := patch.Apply(tt.latest, tt.options...) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(tt.latest.GetConditions()).To(haveSameConditionsOf(tt.want)) + }) + } +} + +func TestApplyDoesNotAlterLastTransitionTime(t *testing.T) { + g := NewWithT(t) + + var before nonKubeObj + after := nonKubeObj{ + c: []metav1.Condition{ + { + Type: "foo", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(time.Now().UTC().Truncate(time.Second)), + }, + }, + } + var latest nonKubeObj + + // latest has no conditions, so we are actually adding the condition but in + // this case we should not set the lastTransitionTime, but we should + // preserve the LastTransition set in after + + diff := NewPatch(before, after) + err := diff.Apply(&latest) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(latest.GetConditions()).To(Equal(after.GetConditions())) +} diff --git a/pkg/common/conditions/setter.go b/pkg/common/conditions/setter.go new file mode 100644 index 0000000000..130b72d00a --- /dev/null +++ b/pkg/common/conditions/setter.go @@ -0,0 +1,126 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Setter interface defines methods that a Cluster API object should implement in order to +// use the conditions package for setting conditions. +type Setter interface { + Getter + SetConditions([]metav1.Condition) +} + +// Set sets the given condition. +// If a condition with the same type already exists, its LastTransitionTime is +// only updated if a change is detected in one of the following fields: +// Status, Reason, or Message. +func Set(to Setter, condition *metav1.Condition) { + to.SetConditions(c4g(to).Set(condition)) +} + +// TrueCondition returns a condition with Status=True and the given type. +func TrueCondition(t string) *metav1.Condition { + return &metav1.Condition{ + Type: t, + Status: metav1.ConditionTrue, + // This is a non-empty field in metav1.Conditions, when it was not in our v1a1 Conditions. This + // really doesn't work with how we've defined our conditions so do something to make things + // work for now. + Reason: string(metav1.ConditionTrue), + } +} + +// FalseCondition returns a condition with Status=False and the given type. +func FalseCondition(t string, reason string, messageFormat string, messageArgs ...interface{}) *metav1.Condition { + if reason == "" { + reason = string(metav1.ConditionFalse) + } + + return &metav1.Condition{ + Type: t, + Status: metav1.ConditionFalse, + Reason: reason, + Message: fmt.Sprintf(messageFormat, messageArgs...), + } +} + +// UnknownCondition returns a condition with Status=Unknown and the given type. +func UnknownCondition(t string, reason string, messageFormat string, messageArgs ...interface{}) *metav1.Condition { + if reason == "" { + reason = string(metav1.ConditionUnknown) + } + + return &metav1.Condition{ + Type: t, + Status: metav1.ConditionUnknown, + Reason: reason, + Message: fmt.Sprintf(messageFormat, messageArgs...), + } +} + +// MarkTrue sets Status=True for the condition with the given type. +func MarkTrue(to Setter, t string) { + to.SetConditions(c4g(to).MarkTrue(t)) +} + +// MarkUnknown sets Status=Unknown for the condition with the given type. +func MarkUnknown(to Setter, t string, reason, messageFormat string, messageArgs ...interface{}) { + to.SetConditions(c4g(to).MarkUnknown(t, reason, messageFormat, messageArgs...)) +} + +// MarkFalse sets Status=False for the condition with the given type. +func MarkFalse(to Setter, t string, reason string, messageFormat string, messageArgs ...interface{}) { + to.SetConditions(c4g(to).MarkFalse(t, reason, messageFormat, messageArgs...)) +} + +// MarkError sets Status=False and the error message for the condition with the given type. +func MarkError(to Setter, t string, reason string, err error) { + to.SetConditions(c4g(to).MarkFalse(t, reason, "%s", err.Error())) +} + +// SetSummary sets a Ready condition with a summary of all the existing +// conditions. If there are no existing conditions, no summary condition is +// generated. +func SetSummary(to Setter, options ...MergeOption) { + to.SetConditions(c4g(to).SetSummary(options...)) +} + +// SetMirror creates a new condition by mirroring the Ready condition from a +// source object. If the source object does not have a Ready condition, the +// target is not modified. +func SetMirror(to Setter, targetCondition string, from Getter, options ...MirrorOptions) { + to.SetConditions(c4g(to).SetMirror(targetCondition, from, options...)) +} + +// SetAggregate creates a new condition by aggregating all of the Ready +// conditions from a list of source objects. If a source object is missing the +// Ready condition, that object is excluded from aggregation. If none of the +// source objects have a Ready condition, the target is not modified. +func SetAggregate(to Setter, targetCondition string, from []Getter, options ...MergeOption) { + to.SetConditions(c4g(to).SetAggregate(targetCondition, from, options...)) +} + +// Delete deletes the condition with the given type. +func Delete(to Setter, t string) { + to.SetConditions(c4g(to).Delete(t)) +} diff --git a/pkg/common/conditions/setter_test.go b/pkg/common/conditions/setter_test.go new file mode 100644 index 0000000000..6bc9ed436b --- /dev/null +++ b/pkg/common/conditions/setter_test.go @@ -0,0 +1,308 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "errors" + "fmt" + "testing" + "time" + + . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" + "github.com/onsi/gomega/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestHasSameState(t *testing.T) { + g := NewWithT(t) + + // same condition + falseInfo2 := falseInfo1.DeepCopy() + g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeTrue()) + + // different LastTransitionTime does not impact state + falseInfo2 = falseInfo1.DeepCopy() + falseInfo2.LastTransitionTime = metav1.NewTime(time.Date(1900, time.November, 10, 23, 0, 0, 0, time.UTC)) + g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeTrue()) + + // different Type, Status, Reason, Severity and Message determine different state + falseInfo2 = falseInfo1.DeepCopy() + falseInfo2.Type = "another type" + g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse()) + + falseInfo2 = falseInfo1.DeepCopy() + falseInfo2.Status = metav1.ConditionTrue + g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse()) + + falseInfo2 = falseInfo1.DeepCopy() + falseInfo2.Reason = "another severity" + g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse()) + + falseInfo2 = falseInfo1.DeepCopy() + falseInfo2.Message = "another message" + g.Expect(hasSameState(falseInfo1, falseInfo2)).To(BeFalse()) +} + +func TestLexicographicLess(t *testing.T) { + g := NewWithT(t) + + // alphabetical order of Type is respected + a := TrueCondition("A") + b := TrueCondition("B") + g.Expect(lexicographicLess(a, b)).To(BeTrue()) + + a = TrueCondition("B") + b = TrueCondition("A") + g.Expect(lexicographicLess(a, b)).To(BeFalse()) + + // Ready condition is treated as an exception and always goes first + a = TrueCondition(ReadyConditionType) + b = TrueCondition("A") + g.Expect(lexicographicLess(a, b)).To(BeTrue()) + + a = TrueCondition("A") + b = TrueCondition(ReadyConditionType) + g.Expect(lexicographicLess(a, b)).To(BeFalse()) +} + +func TestSet(t *testing.T) { + a := TrueCondition("a") + b := TrueCondition("b") + ready := TrueCondition(ReadyConditionType) + + tests := []struct { + name string + to Setter + condition *metav1.Condition + want []metav1.Condition + }{ + { + name: "Set specifies nil condition", + to: setterWithConditions(a), + condition: nil, + want: conditionList(a), + }, + { + name: "Set adds a condition", + to: setterWithConditions(), + condition: a, + want: conditionList(a), + }, + { + name: "Set adds more conditions", + to: setterWithConditions(a), + condition: b, + want: conditionList(a, b), + }, + { + name: "Set does not duplicate existing conditions", + to: setterWithConditions(a, b), + condition: a, + want: conditionList(a, b), + }, + { + name: "Set sorts conditions in lexicographic order", + to: setterWithConditions(b, a), + condition: ready, + want: conditionList(ready, a, b), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + Set(tt.to, tt.condition) + + g.Expect(tt.to.GetConditions()).To(haveSameConditionsOf(tt.want)) + }) + } +} + +func TestSetLastTransitionTime(t *testing.T) { + x := metav1.Date(2012, time.January, 1, 12, 15, 30, 5e8, time.UTC) + + foo := FalseCondition("foo", "reason foo", "message foo") + fooWithLastTransitionTime := FalseCondition("foo", "reason foo", "message foo") + fooWithLastTransitionTime.LastTransitionTime = x + fooWithAnotherState := TrueCondition("foo") + + tests := []struct { + name string + to Setter + new *metav1.Condition + LastTransitionTimeCheck func(*WithT, metav1.Time) + }{ + { + name: "Set a condition that does not exists should set the last transition time if not defined", + to: setterWithConditions(), + new: foo, + LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) { + g.Expect(lastTransitionTime).ToNot(BeZero()) + }, + }, + { + name: "Set a condition that does not exists should preserve the last transition time if defined", + to: setterWithConditions(), + new: fooWithLastTransitionTime, + LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) { + g.Expect(lastTransitionTime).To(Equal(x)) + }, + }, + { + name: "Set a condition that already exists with the same state should preserves the last transition time", + to: setterWithConditions(fooWithLastTransitionTime), + new: foo, + LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) { + g.Expect(lastTransitionTime).To(Equal(x)) + }, + }, + { + name: "Set a condition that already exists but with different state should changes the last transition time", + to: setterWithConditions(fooWithLastTransitionTime), + new: fooWithAnotherState, + LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) { + g.Expect(lastTransitionTime).ToNot(Equal(x)) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + Set(tt.to, tt.new) + + tt.LastTransitionTimeCheck(g, Get(tt.to, "foo").LastTransitionTime) + }) + } +} + +func TestMarkMethods(t *testing.T) { + g := NewWithT(t) + + var obj nonKubeObj + + // test MarkTrue + MarkTrue(&obj, "conditionFoo") + g.Expect(Get(obj, "conditionFoo")).To(haveSameStateOf(&metav1.Condition{ + Type: "conditionFoo", + Status: metav1.ConditionTrue, + Reason: "True", + })) + + // test MarkFalse + MarkFalse(&obj, "conditionBar", "reasonBar", "messageBar") + g.Expect(Get(obj, "conditionBar")).To(haveSameStateOf(&metav1.Condition{ + Type: "conditionBar", + Status: metav1.ConditionFalse, + Reason: "reasonBar", + Message: "messageBar", + })) + + // test MarkError + tErr := fmt.Errorf("errorBar") + MarkError(&obj, "conditionBar", "reasonBar", tErr) + g.Expect(Get(obj, "conditionBar")).To(haveSameStateOf(&metav1.Condition{ + Type: "conditionBar", + Status: metav1.ConditionFalse, + Reason: "reasonBar", + Message: "errorBar", + })) + + // test MarkUnknown + MarkUnknown(&obj, "conditionBaz", "reasonBaz", "messageBaz") + g.Expect(Get(obj, "conditionBaz")).To(haveSameStateOf(&metav1.Condition{ + Type: "conditionBaz", + Status: metav1.ConditionUnknown, + Reason: "reasonBaz", + Message: "messageBaz", + })) +} + +func TestSetSummary(t *testing.T) { + g := NewWithT(t) + target := setterWithConditions(TrueCondition("foo")) + + SetSummary(target) + + g.Expect(Has(target, ReadyConditionType)).To(BeTrue()) +} + +func TestSetMirror(t *testing.T) { + g := NewWithT(t) + source := getterWithConditions(TrueCondition(ReadyConditionType)) + target := setterWithConditions() + + SetMirror(target, "foo", source) + + g.Expect(Has(target, "foo")).To(BeTrue()) +} + +func TestSetAggregate(t *testing.T) { + g := NewWithT(t) + source1 := getterWithConditions(TrueCondition(ReadyConditionType)) + source2 := getterWithConditions(TrueCondition(ReadyConditionType)) + target := setterWithConditions() + + SetAggregate(target, "foo", []Getter{source1, source2}) + + g.Expect(Has(target, "foo")).To(BeTrue()) +} + +func setterWithConditions(conditions ...*metav1.Condition) Setter { + var obj nonKubeObj + obj.SetConditions(conditionList(conditions...)) + return &obj +} + +func haveSameConditionsOf(expected []metav1.Condition) types.GomegaMatcher { + return &ConditionsMatcher{ + Expected: expected, + } +} + +type ConditionsMatcher struct { + Expected []metav1.Condition +} + +func (matcher *ConditionsMatcher) Match(actual interface{}) (success bool, err error) { + actualConditions, ok := actual.([]metav1.Condition) + if !ok { + return false, errors.New("Value should be a conditions list") + } + + if len(actualConditions) != len(matcher.Expected) { + return false, nil + } + + for i := range actualConditions { + if !hasSameState(&actualConditions[i], &matcher.Expected[i]) { + return false, nil + } + } + return true, nil +} + +func (matcher *ConditionsMatcher) FailureMessage(actual interface{}) (message string) { + return format.Message(actual, "to have the same conditions of", matcher.Expected) +} +func (matcher *ConditionsMatcher) NegatedFailureMessage(actual interface{}) (message string) { + return format.Message(actual, "not to have the same conditions of", matcher.Expected) +} diff --git a/pkg/common/conditions/unstructured.go b/pkg/common/conditions/unstructured.go new file mode 100644 index 0000000000..8845f80924 --- /dev/null +++ b/pkg/common/conditions/unstructured.go @@ -0,0 +1,83 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +// UnstructuredGetter return a Getter object that can read conditions from an Unstructured object. +// Important. This method should be used only with types implementing Cluster API conditions. +func UnstructuredGetter(u *unstructured.Unstructured) Getter { + return &unstructuredWrapper{Unstructured: u} +} + +// UnstructuredSetter return a Setter object that can set conditions from an Unstructured object. +// Important. This method should be used only with types implementing Cluster API conditions. +func UnstructuredSetter(u *unstructured.Unstructured) Setter { + return &unstructuredWrapper{Unstructured: u} +} + +type unstructuredWrapper struct { + *unstructured.Unstructured +} + +// GetConditions returns the list of conditions from an Unstructured object. +// +// NOTE: Due to the constraints of JSON-unmarshal, this operation is to be considered best effort. +// In more details: +// - Errors during JSON-unmarshal are ignored and a empty collection list is returned. +// - It's not possible to detect if the object has an empty condition list or if it does not implement conditions; +// in both cases the operation returns an empty slice is returned. +// - If the object doesn't implement conditions on under status as defined in Cluster API, +// JSON-unmarshal matches incoming object keys to the keys; this can lead to conditions values partially set. +func (c *unstructuredWrapper) GetConditions() []metav1.Condition { + var conditions []metav1.Condition + if err := UnstructuredUnmarshalField(c.Unstructured, &conditions, "status", "conditions"); err != nil { + return nil + } + return conditions +} + +// SetConditions set the conditions into an Unstructured object. +// +// NOTE: Due to the constraints of JSON-unmarshal, this operation is to be considered best effort. +// In more details: +// - Errors during JSON-unmarshal are ignored and a empty collection list is returned. +// - It's not possible to detect if the object has an empty condition list or if it does not implement conditions; +// in both cases the operation returns an empty slice is returned. +func (c *unstructuredWrapper) SetConditions(conditions []metav1.Condition) { + v := make([]interface{}, 0, len(conditions)) + for i := range conditions { + m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&conditions[i]) + if err != nil { + log.Log.Error(err, "Failed to convert Condition to unstructured map. This error shouldn't have occurred, please file an issue.", "groupVersionKind", c.GroupVersionKind(), "name", c.GetName(), "namespace", c.GetNamespace()) + continue + } + v = append(v, m) + } + // unstructured.SetNestedField returns an error only if value cannot be set because one of + // the nesting levels is not a map[string]interface{}; this is not the case so the error should never happen here. + err := unstructured.SetNestedField(c.Unstructured.Object, v, "status", "conditions") + if err != nil { + log.Log.Error(err, "Failed to set Conditions on unstructured object. This error shouldn't have occurred, please file an issue.", "groupVersionKind", c.GroupVersionKind(), "name", c.GetName(), "namespace", c.GetNamespace()) + } +} diff --git a/pkg/common/conditions/unstructured_test.go b/pkg/common/conditions/unstructured_test.go new file mode 100644 index 0000000000..d3762d2fca --- /dev/null +++ b/pkg/common/conditions/unstructured_test.go @@ -0,0 +1,95 @@ +/* +Copyright 2025 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 conditions + +import ( + "testing" + + . "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" +) + +func TestUnstructuredGetConditions(t *testing.T) { + g := NewWithT(t) + + scheme := runtime.NewScheme() + g.Expect(corev1.AddToScheme(scheme)).To(Succeed()) + scheme.AddKnownTypes(corev1.SchemeGroupVersion, &kubeObj{}) + + // GetConditions should return conditions from an unstructured object + c := &kubeObj{} + c.SetConditions(conditionList(true1)) + u := &unstructured.Unstructured{} + g.Expect(scheme.Convert(c, u, nil)).To(Succeed()) + + g.Expect(UnstructuredGetter(u).GetConditions()).To(haveSameConditionsOf(conditionList(true1))) + + // GetConditions should return nil for an unstructured object with empty conditions + c = &kubeObj{} + u = &unstructured.Unstructured{} + g.Expect(scheme.Convert(c, u, nil)).To(Succeed()) + + g.Expect(UnstructuredGetter(u).GetConditions()).To(BeNil()) + + // GetConditions should return nil for an unstructured object without conditions + cm := &corev1.ConfigMap{} + u = &unstructured.Unstructured{} + g.Expect(scheme.Convert(cm, u, nil)).To(Succeed()) + + g.Expect(UnstructuredGetter(u).GetConditions()).To(BeNil()) + + // GetConditions should return conditions from an unstructured object with a different type of conditions. + p := &corev1.Pod{Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + { + Type: "foo", + Status: "foo", + LastProbeTime: metav1.Time{}, + LastTransitionTime: metav1.Time{}, + Reason: "foo", + Message: "foo", + }, + }, + }} + u = &unstructured.Unstructured{} + g.Expect(scheme.Convert(p, u, nil)).To(Succeed()) + + g.Expect(UnstructuredGetter(u).GetConditions()).To(HaveLen(1)) +} + +func TestUnstructuredSetConditions(t *testing.T) { + g := NewWithT(t) + + // gets an unstructured with empty conditions + scheme := runtime.NewScheme() + g.Expect(corev1.AddToScheme(scheme)).To(Succeed()) + scheme.AddKnownTypes(corev1.SchemeGroupVersion, &kubeObj{}) + + c := &kubeObj{} + u := &unstructured.Unstructured{} + g.Expect(scheme.Convert(c, u, nil)).To(Succeed()) + + // set conditions + conditions := conditionList(true1, falseInfo1) + + s := UnstructuredSetter(u) + s.SetConditions(conditions) + g.Expect(s.GetConditions()).To(Equal(conditions)) +} diff --git a/pkg/common/conditions/utils.go b/pkg/common/conditions/utils.go new file mode 100644 index 0000000000..2d8596535f --- /dev/null +++ b/pkg/common/conditions/utils.go @@ -0,0 +1,52 @@ +/* +Copyright 2025 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. +*/ + +//nolint:all +package conditions + +import ( + "encoding/json" + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +// Copied from cluster-api util/util.go + +var ( + ErrUnstructuredFieldNotFound = fmt.Errorf("field not found") +) + +// UnstructuredUnmarshalField is a wrapper around json and unstructured objects to decode and copy a specific field +// value into an object. +func UnstructuredUnmarshalField(obj *unstructured.Unstructured, v interface{}, fields ...string) error { + value, found, err := unstructured.NestedFieldNoCopy(obj.Object, fields...) + if err != nil { + return fmt.Errorf("failed to retrieve field %q from %q: %w", strings.Join(fields, "."), obj.GroupVersionKind(), err) + } + if !found || value == nil { + return ErrUnstructuredFieldNotFound + } + valueBytes, err := json.Marshal(value) + if err != nil { + return fmt.Errorf("failed to json-encode field %q value from %q: %w", strings.Join(fields, "."), obj.GroupVersionKind(), err) + } + if err := json.Unmarshal(valueBytes, v); err != nil { + return fmt.Errorf("failed to json-decode field %q value from %q: %w", strings.Join(fields, "."), obj.GroupVersionKind(), err) + } + return nil +} diff --git a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go index b0d346e5d7..0187d8530d 100644 --- a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go +++ b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go @@ -31,6 +31,8 @@ import ( "sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco" cnsoperatortypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types" + "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/conditions" + vmoperatortypes "github.com/vmware-tanzu/vm-operator/api/v1alpha5" cnstypes "github.com/vmware/govmomi/cns/types" v1 "k8s.io/api/core/v1" @@ -297,15 +299,20 @@ func (r *Reconciler) Reconcile(ctx context.Context, err := removePvcFinalizer(ctx, r.client, k8sClient, volume.PersistentVolumeClaim.ClaimName, instance.Namespace, instance.Spec.InstanceUUID) if err != nil { + updateInstanceVolumeStatus(ctx, instance, volume.Name, volume.PersistentVolumeClaim.ClaimName, "", "", err, + v1alpha1.ConditionDetached, v1alpha1.ReasonDetachFailed) log.Errorf("failed to remove finalizer from PVC %s. Err: %s", volume.PersistentVolumeClaim.ClaimName, err) return r.completeReconciliationWithError(batchAttachCtx, instance, request.NamespacedName, timeout, err) + } else { + updateInstanceVolumeStatus(ctx, instance, volume.Name, volume.PersistentVolumeClaim.ClaimName, "", "", err, + v1alpha1.ConditionDetached, "") } } patchErr := removeFinalizerFromCRDInstance(batchAttachCtx, instance, r.client) if patchErr != nil { - log.Errorf("failed to update CnsNodeVMBatchAttachment %s. Err: +%v", instance.Name, patchErr) + log.Errorf("failed to update CnsNodeVMBatchAttachment %s. Err: %s", instance.Name, patchErr) return r.completeReconciliationWithError(batchAttachCtx, instance, request.NamespacedName, timeout, err) } log.Infof("Successfully removed finalizer %s from instance %s", @@ -314,7 +321,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, backOffDurationMapMutex.Lock() delete(backOffDuration, request.NamespacedName) backOffDurationMapMutex.Unlock() - return r.completeReconciliationWithSuccess(batchAttachCtx, instance, request.NamespacedName, timeout) } @@ -456,7 +462,9 @@ func (r *Reconciler) detachVolumes(ctx context.Context, log.Errorf("failed to detach volume %s from VM %s. Fault: %s Err: %s", pvc, instance.Spec.InstanceUUID, faulttype, detachErr) // Update the instance with error for this PVC. - updateInstanceWithErrorForPvc(instance, pvc, detachErr.Error()) + updateInstanceVolumeStatus(ctx, instance, "", pvc, "", "", detachErr, + v1alpha1.ConditionDetached, v1alpha1.ReasonDetachFailed) + volumesThatFailedToDetach = append(volumesThatFailedToDetach, pvc) } } else { @@ -483,7 +491,9 @@ func removeFinalizerAndStatusEntry(ctx context.Context, client client.Client, k8 err := removePvcFinalizer(ctx, client, k8sClient, pvc, instance.Namespace, instance.Spec.InstanceUUID) if err != nil { log.Errorf("failed to remove finalizer from PVC %s. Err: %s", pvc, err) - updateInstanceWithErrorForPvc(instance, pvc, err.Error()) + updateInstanceVolumeStatus(ctx, instance, "", pvc, "", "", err, + v1alpha1.ConditionDetached, v1alpha1.ReasonDetachFailed) + volumesThatFailedToDetach = append(volumesThatFailedToDetach, pvc) } else { // Remove entry of this volume from the instance's status. @@ -515,9 +525,9 @@ func (r *Reconciler) processBatchAttach(ctx context.Context, k8sClient kubernete // Call CNS AttachVolume batchAttachResult, faultType, attachErr := r.volumeManager.BatchAttachVolumes(ctx, vm, batchAttachRequest) if attachErr != nil { - log.Errorf("failed to batch attach all volumes. Fault: %s Err: %s", faultType, attachErr) + log.Errorf("failed to attach all volumes. Fault: %s Err: %s", faultType, attachErr) } else { - log.Infof("Successfully batch attached all volumes") + log.Infof("Successfully attached all volumes") } // Update instance based on the result of BatchAttach @@ -534,18 +544,23 @@ func (r *Reconciler) processBatchAttach(ctx context.Context, k8sClient kubernete } + reason := v1alpha1.ReasonAttachFailed // If attach was successful, add finalizer to the PVC. if result.Error == nil { + reason = "" // Add finalizer on PVC as attach was successful. err = addPvcFinalizer(ctx, r.client, k8sClient, pvcName, instance.Namespace, instance.Spec.InstanceUUID) if err != nil { log.Errorf("failed to add finalizer %s on PVC %s", cnsoperatortypes.CNSPvcFinalizer, pvcName) result.Error = err - attachErr = err + attachErr = errors.Join(attachErr, + fmt.Errorf("failure during attach of PVC %s", pvcName)) } } - // Update instance with attach result - updateInstanceWithAttachVolumeResult(instance, volumeName, pvcName, result) + // Update instance with attach result. + updateInstanceVolumeStatus(ctx, instance, volumeName, pvcName, result.VolumeID, result.DiskUUID, result.Error, + v1alpha1.ConditionAttached, reason) + } return attachErr } @@ -584,11 +599,12 @@ func (r *Reconciler) completeReconciliationWithSuccess(ctx context.Context, inst namespaceName types.NamespacedName, timeout time.Duration) (reconcile.Result, error) { log := logger.GetLogger(ctx) - instance.Status.Error = "" + conditions.MarkTrue(instance, v1alpha1.ConditionReady) + updateErr := updateInstanceStatus(ctx, r.client, instance) if updateErr != nil { recordEvent(ctx, r, instance, v1.EventTypeWarning, updateErr.Error()) - log.Errorf("failed to update CnsNodeVMBatchAttachment %s. Err: +%v", namespaceName, updateErr) + log.Errorf("failed to update CnsNodeVMBatchAttachment %s. Err: %s", namespaceName, updateErr) return reconcile.Result{RequeueAfter: timeout}, nil } @@ -606,11 +622,14 @@ func (r *Reconciler) completeReconciliationWithSuccess(ctx context.Context, inst func (r *Reconciler) completeReconciliationWithError(ctx context.Context, instance *v1alpha1.CnsNodeVMBatchAttachment, namespaceName types.NamespacedName, timeout time.Duration, err error) (reconcile.Result, error) { log := logger.GetLogger(ctx) - instance.Status.Error = err.Error() + + trimmedError := trimMessage(err) + conditions.MarkError(instance, v1alpha1.ConditionReady, v1alpha1.ReasonFailed, trimmedError) + updateErr := updateInstanceStatus(ctx, r.client, instance) if updateErr != nil { recordEvent(ctx, r, instance, v1.EventTypeWarning, updateErr.Error()) - log.Errorf("failed to update CnsNodeVMBatchAttachment %s. Err: +%v", namespaceName, updateErr) + log.Errorf("failed to update CnsNodeVMBatchAttachment %s. Err: %s", namespaceName, updateErr) return reconcile.Result{RequeueAfter: timeout}, nil } recordEvent(ctx, r, instance, v1.EventTypeWarning, err.Error()) diff --git a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go index 7304c8b882..0505c9d1dd 100644 --- a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go +++ b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go @@ -43,6 +43,8 @@ import ( k8s "sigs.k8s.io/vsphere-csi-driver/v3/pkg/kubernetes" cnsoperatortypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types" cnsoperatorutil "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/util" + + "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/conditions" ) var ( @@ -51,9 +53,27 @@ var ( ) const ( - detachSuffix = ":detaching" + detachSuffix = ":detaching" + MaxConditionMessageLength = 32768 ) +// trimMessage safely truncates a message to the Kubernetes status.conditions max length. +func trimMessage(err error) error { + if err == nil { + return nil + } + + msg := err.Error() + runes := []rune(msg) + + if len(runes) > MaxConditionMessageLength { + trimmed := string(runes[:MaxConditionMessageLength-3]) + "..." + return fmt.Errorf("%s", trimmed) + } + + return err +} + // removeFinalizerFromCRDInstance will remove the CNS Finalizer, cns.vmware.com, // from a given nodevmbatchattachment instance. func removeFinalizerFromCRDInstance(ctx context.Context, @@ -224,7 +244,7 @@ func getVolumesToDetachForVmFromVC(ctx context.Context, log.Errorf("failed to find volumes to attach and volumes to detach from instance spec. Err: %s", err) return pvcsToDetach, err } - log.Debugf("Obtained volumes to detach %+v for instance %s", pvcsToDetach, instance.Name) + log.Infof("Obtained volumes to detach %+v for instance %s", pvcsToDetach, instance.Name) updatePvcStatusEntryName(ctx, instance, pvcsToDetach) @@ -279,56 +299,6 @@ func updateInstanceStatus(ctx context.Context, cnsoperatorclient client.Client, return nil } -// updateInstanceWithAttachVolumeResult finds the given's volumeName's status in the instance status -// and updates it with error. -// It will add a new status for the volume if it does not already exist. -func updateInstanceWithAttachVolumeResult(instance *v1alpha1.CnsNodeVMBatchAttachment, - volumeName string, pvc string, result volumes.BatchAttachResult) { - - errMsg := "" - attached := true - if result.Error != nil { - attached = false - errMsg = result.Error.Error() - } - - newVolumeStatus := v1alpha1.VolumeStatus{ - Name: volumeName, - PersistentVolumeClaim: v1alpha1.PersistentVolumeClaimStatus{ - ClaimName: pvc, - Attached: attached, - Error: errMsg, - CnsVolumeID: result.VolumeID, - DiskUUID: result.DiskUUID, - }, - } - - for i, volume := range instance.Status.VolumeStatus { - if volume.Name != volumeName { - continue - } - // Update existing entry - instance.Status.VolumeStatus[i] = newVolumeStatus - return - } - - // Add new entry instatus if it does not already exist. - instance.Status.VolumeStatus = append(instance.Status.VolumeStatus, newVolumeStatus) -} - -// updateInstanceWithErrorVolumeName finds the given's PVC's status in the instance status -// and updates it with error. -func updateInstanceWithErrorForPvc(instance *v1alpha1.CnsNodeVMBatchAttachment, - pvc string, errMsg string) { - for i, volume := range instance.Status.VolumeStatus { - if volume.PersistentVolumeClaim.ClaimName != pvc { - continue - } - instance.Status.VolumeStatus[i].PersistentVolumeClaim.Error = errMsg - return - } -} - // deleteVolumeFromStatus finds the status of the given volumeName in an instance and deletes its entry. func deleteVolumeFromStatus(pvc string, instance *v1alpha1.CnsNodeVMBatchAttachment) { instance.Status.VolumeStatus = slices.DeleteFunc(instance.Status.VolumeStatus, @@ -586,7 +556,7 @@ func patchPVCAnnotations(ctx context.Context, k8sClient kubernetes.Interface, log.Infof("Removing annotation %s from PVC %s", key, pvc.Name) patchAnnotations[key] = nil } else { - log.Infof("Adding annotation %s on PVC", key, pvc.Name) + log.Infof("Adding annotation %s on PVC %s", key, pvc.Name) patchAnnotations[key] = "" } @@ -672,7 +642,6 @@ func addPvcFinalizer(ctx context.Context, client client.Client, } // Add annotation indicating that the PVC is being used by this VM. - log.Infof("PVC %s is shared", pvc.Name) err = addPvcAnnotation(ctx, k8sClient, vmInstanceUUID, pvc) if err != nil { log.Errorf("failed to add annotation %s to PVC %s in namespace %s for VM %s", cnsoperatortypes.CNSPvcFinalizer, @@ -744,7 +713,6 @@ func removePvcFinalizer(ctx context.Context, client client.Client, } // Remove usedby annotation - log.Infof("PVC %s is shared", pvc.Name) err = removePvcAnnotation(ctx, k8sClient, vmInstanceUUID, pvc) if err != nil { return err @@ -797,3 +765,68 @@ func removePvcFinalizer(ctx context.Context, client client.Client, VolumeLock.Delete(namespacedVolumeName) return nil } + +// updateInstanceVolumeStatus updates the status for a given volume in the instance. +func updateInstanceVolumeStatus( + ctx context.Context, + instance *v1alpha1.CnsNodeVMBatchAttachment, + volumeName, pvc string, + volumeID, diskUUID string, + err error, + conditionType, reason string) { + log := logger.GetLogger(ctx) + + trimmedError := trimMessage(err) + for i, volumeStatus := range instance.Status.VolumeStatus { + + if volumeStatus.PersistentVolumeClaim.ClaimName != pvc { + continue + } + + if volumeID != "" { + volumeStatus.PersistentVolumeClaim.CnsVolumeID = volumeID + } + if diskUUID != "" { + volumeStatus.PersistentVolumeClaim.DiskUUID = diskUUID + } + + // Ensure conditions are initialized + if volumeStatus.PersistentVolumeClaim.Conditions == nil { + volumeStatus.PersistentVolumeClaim.Conditions = []metav1.Condition{} + } + + // Apply condition + if err != nil { + conditions.MarkError(&volumeStatus.PersistentVolumeClaim, conditionType, reason, trimmedError) + } else { + conditions.MarkTrue(&volumeStatus.PersistentVolumeClaim, conditionType) + } + + instance.Status.VolumeStatus[i] = volumeStatus + return + } + + if volumeName == "" { + log.Infof("VolumeName is empty for PVC %s. Skip adding a new entry.", pvc) + return + } + + // Not found — create a new entry + newVolumeStatus := v1alpha1.VolumeStatus{ + Name: volumeName, + PersistentVolumeClaim: v1alpha1.PersistentVolumeClaimStatus{ + ClaimName: pvc, + CnsVolumeID: volumeID, + DiskUUID: diskUUID, + Conditions: []metav1.Condition{}, + }, + } + + if err != nil { + conditions.MarkError(&newVolumeStatus.PersistentVolumeClaim, conditionType, reason, trimmedError) + } else { + conditions.MarkTrue(&newVolumeStatus.PersistentVolumeClaim, conditionType) + } + + instance.Status.VolumeStatus = append(instance.Status.VolumeStatus, newVolumeStatus) +} diff --git a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_test.go b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_test.go index a01e5949f4..41fa9fd66b 100644 --- a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_test.go +++ b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_test.go @@ -18,6 +18,7 @@ package cnsnodevmbatchattachment import ( "context" + "errors" "fmt" "sync" "testing" @@ -25,6 +26,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/vmware/govmomi/object" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -90,7 +92,6 @@ func setupTestCnsNodeVMBatchAttachment() v1alpha1.CnsNodeVMBatchAttachment { ClaimName: pvc1, DiskUUID: "123456", CnsVolumeID: "67890", - Attached: true, }, }, { @@ -99,7 +100,6 @@ func setupTestCnsNodeVMBatchAttachment() v1alpha1.CnsNodeVMBatchAttachment { ClaimName: pvc2, DiskUUID: "123456", CnsVolumeID: "67890", - Attached: true, }, }, }, @@ -114,7 +114,6 @@ func setupTestCnsNodeVMBatchAttachment() v1alpha1.CnsNodeVMBatchAttachment { func setTestEnvironment(testCnsNodeVMBatchAttachment *v1alpha1.CnsNodeVMBatchAttachment, setDeletionTimestamp bool) *Reconciler { cnsNodeVmBatchAttachment := testCnsNodeVMBatchAttachment.DeepCopy() - //objs := []runtime.Object{cnsNodeVmBatchAttachment} if setDeletionTimestamp { currentTime := time.Now() @@ -228,7 +227,7 @@ func TestCnsNodeVMBatchAttachmentWhenVmOnVcenterReturnsError(t *testing.T) { } expectedReconcileError := fmt.Errorf("some error occurred while getting VM") - assert.EqualError(t, expectedReconcileError, updatedCnsNodeVMBatchAttachment.Status.Error) + assert.EqualError(t, expectedReconcileError, updatedCnsNodeVMBatchAttachment.Status.Conditions[0].Message) }) } @@ -272,7 +271,7 @@ func TestCnsNodeVMBatchAttachmentWhenVmOnVcenterReturnsNotFoundErrorAndInstanceI "Vm is CR is deleted or is being deleted but"+ "CnsNodeVMBatchAttachmentInstance %s is not being deleted", nodeUUID, testCnsNodeVMBatchAttachmentName) expectedErrorMsg := expectedReconcileError.Error() - assert.Equal(t, expectedErrorMsg, updatedCnsNodeVMBatchAttachment.Status.Error) + assert.Equal(t, expectedErrorMsg, updatedCnsNodeVMBatchAttachment.Status.Conditions[0].Message) }) } @@ -791,3 +790,98 @@ func MockGetVMFromVcenter(ctx context.Context, nodeUUID string, } return &cnsvsphere.VirtualMachine{}, nil } + +func TestUpdateInstanceVolume_AddsNewVolumeStatus_WhenNotFound(t *testing.T) { + instance := &v1alpha1.CnsNodeVMBatchAttachment{} + + updateInstanceVolumeStatus( + context.TODO(), + instance, + "vol1", "pvc1", + "vol-id-123", "uuid-456", + nil, + v1alpha1.ConditionAttached, "Success", + ) + + assert.Len(t, instance.Status.VolumeStatus, 1) + vs := instance.Status.VolumeStatus[0] + assert.Equal(t, "vol1", vs.Name) + assert.Equal(t, "pvc1", vs.PersistentVolumeClaim.ClaimName) + assert.Equal(t, "vol-id-123", vs.PersistentVolumeClaim.CnsVolumeID) + assert.Equal(t, "uuid-456", vs.PersistentVolumeClaim.DiskUUID) + assert.Equal(t, "", vs.PersistentVolumeClaim.Conditions[0].Message) + assert.Equal(t, "True", vs.PersistentVolumeClaim.Conditions[0].Reason) + assert.Equal(t, metav1.ConditionTrue, vs.PersistentVolumeClaim.Conditions[0].Status) + assert.Equal(t, v1alpha1.ConditionAttached, vs.PersistentVolumeClaim.Conditions[0].Type) +} + +func TestUpdateInstanceVolume_UpdatesExistingVolumeStatus(t *testing.T) { + instance := &v1alpha1.CnsNodeVMBatchAttachment{ + Status: v1alpha1.CnsNodeVMBatchAttachmentStatus{ + VolumeStatus: []v1alpha1.VolumeStatus{ + { + Name: "vol1", + PersistentVolumeClaim: v1alpha1.PersistentVolumeClaimStatus{ + ClaimName: "pvc1", + }, + }, + }, + }, + } + + updateInstanceVolumeStatus( + context.TODO(), + instance, + "vol1", "pvc1", + "new-vol-id", "new-uuid", + nil, + v1alpha1.ConditionAttached, "Success", + ) + + assert.Len(t, instance.Status.VolumeStatus, 1) + vs := instance.Status.VolumeStatus[0] + assert.Equal(t, "new-vol-id", vs.PersistentVolumeClaim.CnsVolumeID) + assert.Equal(t, "new-uuid", vs.PersistentVolumeClaim.DiskUUID) + assert.Equal(t, "", vs.PersistentVolumeClaim.Conditions[0].Message) + assert.Equal(t, "True", vs.PersistentVolumeClaim.Conditions[0].Reason) + assert.Equal(t, metav1.ConditionTrue, vs.PersistentVolumeClaim.Conditions[0].Status) + assert.Equal(t, v1alpha1.ConditionAttached, vs.PersistentVolumeClaim.Conditions[0].Type) +} + +func TestUpdateInstanceVolume_MarksErrorCondition_WhenErrorProvided(t *testing.T) { + instance := &v1alpha1.CnsNodeVMBatchAttachment{} + + err := errors.New("failed to attach") + updateInstanceVolumeStatus( + context.TODO(), + instance, + "vol2", "pvc2", + "", "", + err, + v1alpha1.ConditionAttached, v1alpha1.ReasonAttachFailed, + ) + + assert.Len(t, instance.Status.VolumeStatus, 1) + vs := instance.Status.VolumeStatus[0] + assert.Equal(t, "pvc2", vs.PersistentVolumeClaim.ClaimName) + assert.Equal(t, "failed to attach", vs.PersistentVolumeClaim.Conditions[0].Message) + assert.Equal(t, v1alpha1.ReasonAttachFailed, vs.PersistentVolumeClaim.Conditions[0].Reason) + assert.Equal(t, metav1.ConditionFalse, vs.PersistentVolumeClaim.Conditions[0].Status) + assert.Equal(t, v1alpha1.ConditionAttached, vs.PersistentVolumeClaim.Conditions[0].Type) + +} + +func TestUpdateInstanceVolume_WhenVolumeNameIsEmpty(t *testing.T) { + instance := &v1alpha1.CnsNodeVMBatchAttachment{} + + updateInstanceVolumeStatus( + context.TODO(), + instance, + "", "pvc-xyz", + "vol-id-123", "uuid-456", + nil, + v1alpha1.ConditionAttached, "Success", + ) + + assert.Len(t, instance.Status.VolumeStatus, 0) +}