Skip to content

Commit b7552df

Browse files
divyenpateldivyenpatel
authored
Merge pull request #3728 from skogta/topic/skogta/revertConditions
Revert "Use K8s conditions for CnsNodeVMBatchAttachment CRD (#3700)"
2 parents f8b6c3b + 714bf14 commit b7552df

24 files changed

+103
-3576
lines changed

‎pkg/apis/cnsoperator/cnsnodevmbatchattachment/v1alpha1/cnsnodebatchvmattachment_types.go‎

Lines changed: 10 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -36,24 +36,6 @@ const (
3636
IndependentNonPersistent = "independent_nonpersistent"
3737
// Changes to virtual disk are made to a redo log and discarded at power off.
3838
NonPersistent = "nonpersistent"
39-
40-
// This section defines the different conditions that CnsNodeVMBatchAttachment CR can take.
41-
// ConditionReady reflects the overall status of the CR.
42-
ConditionReady = "Ready"
43-
// ConditionAttached reflects whether the given volume was attached successfully.
44-
ConditionAttached = "VolumeAttached"
45-
// ConditionDetached reflects whether the given volume was detached successfully.
46-
ConditionDetached = "VolumeDetached"
47-
48-
// This section defines the different reasons for different conditions in the CnsNodeVMBatchAttachment CR.
49-
// ReasonAttachFailed reflects that the volume failed to get attached.
50-
// In case of successful attachment, reason is set to True.
51-
ReasonAttachFailed = "AttachFailed"
52-
// ReasonDetachFailed reflects that the volume failed to get detached.
53-
// In case of successful detach, the volume's entry is removed from the CR.
54-
ReasonDetachFailed = "DetachFailed"
55-
// ReasonFailed reflects that the CR instance is not yet ready.
56-
ReasonFailed = "Failed"
5739
)
5840

5941
// SharingMode is the sharing mode of the virtual disk.
@@ -118,15 +100,12 @@ type PersistentVolumeClaimSpec struct {
118100
// CnsNodeVMBatchAttachmentStatus defines the observed state of CnsNodeVMBatchAttachment
119101
// +k8s:openapi-gen=true
120102
type CnsNodeVMBatchAttachmentStatus struct {
121-
// +optional
103+
// Error is the overall error status for the instance.
104+
Error string `json:"error,omitempty"`
122105
// +listType=map
123106
// +listMapKey=name
124107
// VolumeStatus reflects the status for each volume.
125108
VolumeStatus []VolumeStatus `json:"volumes,omitempty"`
126-
// +optional
127-
128-
// Conditions describes any conditions associated with this CnsNodeVMBatchAttachment instance.
129-
Conditions []metav1.Condition `json:"conditions,omitempty"`
130109
}
131110

132111
type VolumeStatus struct {
@@ -139,18 +118,17 @@ type VolumeStatus struct {
139118
type PersistentVolumeClaimStatus struct {
140119
// ClaimName is the PVC name.
141120
ClaimName string `json:"claimName"`
142-
// +optional
143-
121+
// Attached indicates the attach status of a PVC.
122+
// If volume is not attached, Attached will be set to false.
123+
// If volume is attached, Attached will be set to true.
124+
// If volume is detached successfully, its entry will be removed from VolumeStatus.
125+
Attached bool `json:"attached"`
126+
// Error indicates the error which may have occurred during attach/detach.
127+
Error string `json:"error,omitempty"`
144128
// CnsVolumeID is the volume ID for the PVC.
145129
CnsVolumeID string `json:"cnsVolumeId,omitempty"`
146-
// +optional
147-
148130
// DiskUUID is the ID obtained when volume is attached to a VM.
149131
DiskUUID string `json:"diskUUID,omitempty"`
150-
// +optional
151-
152-
// Conditions describes any conditions associated with this volume.
153-
Conditions []metav1.Condition `json:"conditions,omitempty"`
154132
}
155133

156134
// +genclient
@@ -160,7 +138,7 @@ type PersistentVolumeClaimStatus struct {
160138
// +kubebuilder:subresource:status
161139
// +kubebuilder:object:root=true
162140
// +kubebuilder:resource:shortName=batchattach
163-
// +kubebuilder:printcolumn:name="InstanceUUID",type="string",JSONPath=".spec.instanceUUID"
141+
// +kubebuilder:printcolumn:name="NodeUUID",type="string",JSONPath=".spec.nodeUUID"
164142

165143
// CnsNodeVMBatchAttachment is the Schema for the cnsnodevmbatchattachments API
166144
type CnsNodeVMBatchAttachment struct {
@@ -179,19 +157,3 @@ type CnsNodeVMBatchAttachmentList struct {
179157
metav1.ListMeta `json:"metadata,omitempty"`
180158
Items []CnsNodeVMBatchAttachment `json:"items"`
181159
}
182-
183-
func (in *CnsNodeVMBatchAttachment) GetConditions() []metav1.Condition {
184-
return in.Status.Conditions
185-
}
186-
187-
func (in *CnsNodeVMBatchAttachment) SetConditions(conditions []metav1.Condition) {
188-
in.Status.Conditions = conditions
189-
}
190-
191-
func (p *PersistentVolumeClaimStatus) GetConditions() []metav1.Condition {
192-
return p.Conditions
193-
}
194-
195-
func (p *PersistentVolumeClaimStatus) SetConditions(conditions []metav1.Condition) {
196-
p.Conditions = conditions
197-
}

‎pkg/apis/cnsoperator/config/cns.vmware.com_cnsnodevmbatchattachments.yaml‎

Lines changed: 18 additions & 148 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ spec:
1717
scope: Namespaced
1818
versions:
1919
- additionalPrinterColumns:
20-
- jsonPath: .spec.instanceUUID
21-
name: InstanceUUID
20+
- jsonPath: .spec.nodeUUID
21+
name: NodeUUID
2222
type: string
2323
name: v1alpha1
2424
schema:
@@ -112,77 +112,9 @@ spec:
112112
description: CnsNodeVMBatchAttachmentStatus defines the observed state
113113
of CnsNodeVMBatchAttachment
114114
properties:
115-
conditions:
116-
description: Conditions describes any conditions associated with this
117-
CnsNodeVMBatchAttachment instance.
118-
items:
119-
description: "Condition contains details for one aspect of the current
120-
state of this API Resource.\n---\nThis struct is intended for
121-
direct use as an array at the field path .status.conditions. For
122-
example,\n\n\n\ttype FooStatus struct{\n\t // Represents the
123-
observations of a foo's current state.\n\t // Known .status.conditions.type
124-
are: \"Available\", \"Progressing\", and \"Degraded\"\n\t //
125-
+patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t
126-
\ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\"
127-
patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t
128-
\ // other fields\n\t}"
129-
properties:
130-
lastTransitionTime:
131-
description: |-
132-
lastTransitionTime is the last time the condition transitioned from one status to another.
133-
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
134-
format: date-time
135-
type: string
136-
message:
137-
description: |-
138-
message is a human readable message indicating details about the transition.
139-
This may be an empty string.
140-
maxLength: 32768
141-
type: string
142-
observedGeneration:
143-
description: |-
144-
observedGeneration represents the .metadata.generation that the condition was set based upon.
145-
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
146-
with respect to the current state of the instance.
147-
format: int64
148-
minimum: 0
149-
type: integer
150-
reason:
151-
description: |-
152-
reason contains a programmatic identifier indicating the reason for the condition's last transition.
153-
Producers of specific condition types may define expected values and meanings for this field,
154-
and whether the values are considered a guaranteed API.
155-
The value should be a CamelCase string.
156-
This field may not be empty.
157-
maxLength: 1024
158-
minLength: 1
159-
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
160-
type: string
161-
status:
162-
description: status of the condition, one of True, False, Unknown.
163-
enum:
164-
- "True"
165-
- "False"
166-
- Unknown
167-
type: string
168-
type:
169-
description: |-
170-
type of condition in CamelCase or in foo.example.com/CamelCase.
171-
---
172-
Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
173-
useful (see .node.status.conditions), the ability to deconflict is important.
174-
The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
175-
maxLength: 316
176-
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])$
177-
type: string
178-
required:
179-
- lastTransitionTime
180-
- message
181-
- reason
182-
- status
183-
- type
184-
type: object
185-
type: array
115+
error:
116+
description: Error is the overall error status for the instance.
117+
type: string
186118
volumes:
187119
description: VolumeStatus reflects the status for each volume.
188120
items:
@@ -194,91 +126,29 @@ spec:
194126
description: PersistentVolumeClaim contains details about the
195127
volume's current state.
196128
properties:
129+
attached:
130+
description: |-
131+
Attached indicates the attach status of a PVC.
132+
If volume is not attached, Attached will be set to false.
133+
If volume is attached, Attached will be set to true.
134+
If volume is detached successfully, its entry will be removed from VolumeStatus.
135+
type: boolean
197136
claimName:
198137
description: ClaimName is the PVC name.
199138
type: string
200139
cnsVolumeId:
201140
description: CnsVolumeID is the volume ID for the PVC.
202141
type: string
203-
conditions:
204-
description: Conditions describes any conditions associated
205-
with this volume.
206-
items:
207-
description: "Condition contains details for one aspect
208-
of the current state of this API Resource.\n---\nThis
209-
struct is intended for direct use as an array at the
210-
field path .status.conditions. For example,\n\n\n\ttype
211-
FooStatus struct{\n\t // Represents the observations
212-
of a foo's current state.\n\t // Known .status.conditions.type
213-
are: \"Available\", \"Progressing\", and \"Degraded\"\n\t
214-
\ // +patchMergeKey=type\n\t // +patchStrategy=merge\n\t
215-
\ // +listType=map\n\t // +listMapKey=type\n\t
216-
\ Conditions []metav1.Condition `json:\"conditions,omitempty\"
217-
patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t
218-
\ // other fields\n\t}"
219-
properties:
220-
lastTransitionTime:
221-
description: |-
222-
lastTransitionTime is the last time the condition transitioned from one status to another.
223-
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
224-
format: date-time
225-
type: string
226-
message:
227-
description: |-
228-
message is a human readable message indicating details about the transition.
229-
This may be an empty string.
230-
maxLength: 32768
231-
type: string
232-
observedGeneration:
233-
description: |-
234-
observedGeneration represents the .metadata.generation that the condition was set based upon.
235-
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
236-
with respect to the current state of the instance.
237-
format: int64
238-
minimum: 0
239-
type: integer
240-
reason:
241-
description: |-
242-
reason contains a programmatic identifier indicating the reason for the condition's last transition.
243-
Producers of specific condition types may define expected values and meanings for this field,
244-
and whether the values are considered a guaranteed API.
245-
The value should be a CamelCase string.
246-
This field may not be empty.
247-
maxLength: 1024
248-
minLength: 1
249-
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
250-
type: string
251-
status:
252-
description: status of the condition, one of True,
253-
False, Unknown.
254-
enum:
255-
- "True"
256-
- "False"
257-
- Unknown
258-
type: string
259-
type:
260-
description: |-
261-
type of condition in CamelCase or in foo.example.com/CamelCase.
262-
---
263-
Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
264-
useful (see .node.status.conditions), the ability to deconflict is important.
265-
The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
266-
maxLength: 316
267-
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])$
268-
type: string
269-
required:
270-
- lastTransitionTime
271-
- message
272-
- reason
273-
- status
274-
- type
275-
type: object
276-
type: array
277142
diskUUID:
278143
description: DiskUUID is the ID obtained when volume is
279144
attached to a VM.
280145
type: string
146+
error:
147+
description: Error indicates the error which may have occurred
148+
during attach/detach.
149+
type: string
281150
required:
151+
- attached
282152
- claimName
283153
type: object
284154
required:
@@ -300,4 +170,4 @@ status:
300170
kind: ""
301171
plural: ""
302172
conditions: []
303-
storedVersions: []
173+
storedVersions: []

‎pkg/common/cns-lib/volume/manager.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,10 +1304,8 @@ func (m *defaultManager) DetachVolume(ctx context.Context, vm *cnsvsphere.Virtua
13041304
}
13051305
}
13061306
}
1307-
log.Errorf("failed to detach cns volume: %q from node vm: %+v. fault: %+v, opId: %q",
1307+
return faultType, logger.LogNewErrorf(log, "failed to detach cns volume: %q from node vm: %+v. fault: %+v, opId: %q",
13081308
volumeID, vm, spew.Sdump(volumeOperationRes.Fault), taskInfo.ActivationId)
1309-
return faultType, fmt.Errorf("failed to detach cns volume: %q, Error: %s,",
1310-
volumeID, volumeOperationRes.Fault.LocalizedMessage)
13111309
}
13121310
log.Infof("DetachVolume: Volume detached successfully. volumeID: %q, vm: %q, opId: %q",
13131311
volumeID, vm.String(), taskInfo.ActivationId)
@@ -3494,10 +3492,8 @@ func compileBatchAttachTaskResult(ctx context.Context, result cnstypes.BaseCnsVo
34943492
// In case of failure, set faultType and error.
34953493
faultType := ExtractFaultTypeFromVolumeResponseResult(ctx, volumeOperationResult)
34963494
batchAttachResult.FaultType = faultType
3497-
log.Errorf("failed to attach cns volume: %q to node vm: %q. fault: %q. opId: %q",
3495+
msg := fmt.Sprintf("failed to batch attach cns volume: %q to node vm: %q. fault: %q. opId: %q",
34983496
volumeId, vm.String(), faultType, activationId)
3499-
msg := fmt.Sprintf("failed to attach cns volume: %q Error: %s",
3500-
volumeId, volumeOperationResult.Fault.LocalizedMessage)
35013497
batchAttachResult.Error = errors.New(msg)
35023498
log.Infof("Constructed batch attach result for volume %s with failure", volumeId)
35033499
return batchAttachResult, nil

0 commit comments

Comments
 (0)