Skip to content

Commit 719cc45

Browse files
skogtaskogta
committed
Use K8s conditions for CnsNodeVMBatchAttachment CRD
Add back Error field temporarily so that it does break VM op compatibility
1 parent b28f50c commit 719cc45

24 files changed

+3582
-95
lines changed

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

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,24 @@ 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"
3957
)
4058

4159
// SharingMode is the sharing mode of the virtual disk.
@@ -100,12 +118,19 @@ type PersistentVolumeClaimSpec struct {
100118
// CnsNodeVMBatchAttachmentStatus defines the observed state of CnsNodeVMBatchAttachment
101119
// +k8s:openapi-gen=true
102120
type CnsNodeVMBatchAttachmentStatus struct {
103-
// Error is the overall error status for the instance.
104-
Error string `json:"error,omitempty"`
121+
// +optional
105122
// +listType=map
106123
// +listMapKey=name
107124
// VolumeStatus reflects the status for each volume.
108125
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"`
130+
131+
// TODO: remove this field once VM op changes are ready.
132+
// Error is the overall error status for the instance.
133+
Error string `json:"error,omitempty"`
109134
}
110135

111136
type VolumeStatus struct {
@@ -118,17 +143,22 @@ type VolumeStatus struct {
118143
type PersistentVolumeClaimStatus struct {
119144
// ClaimName is the PVC name.
120145
ClaimName string `json:"claimName"`
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"`
146+
// +optional
147+
128148
// CnsVolumeID is the volume ID for the PVC.
129149
CnsVolumeID string `json:"cnsVolumeId,omitempty"`
150+
// +optional
151+
130152
// DiskUUID is the ID obtained when volume is attached to a VM.
131153
DiskUUID string `json:"diskUUID,omitempty"`
154+
// +optional
155+
156+
// Conditions describes any conditions associated with this volume.
157+
Conditions []metav1.Condition `json:"conditions,omitempty"`
158+
159+
// TODO: remove this field once VM op changes are ready.
160+
// Error indicates the error which may have occurred during attach/detach.
161+
Error string `json:"error,omitempty"`
132162
}
133163

134164
// +genclient
@@ -157,3 +187,19 @@ type CnsNodeVMBatchAttachmentList struct {
157187
metav1.ListMeta `json:"metadata,omitempty"`
158188
Items []CnsNodeVMBatchAttachment `json:"items"`
159189
}
190+
191+
func (in *CnsNodeVMBatchAttachment) GetConditions() []metav1.Condition {
192+
return in.Status.Conditions
193+
}
194+
195+
func (in *CnsNodeVMBatchAttachment) SetConditions(conditions []metav1.Condition) {
196+
in.Status.Conditions = conditions
197+
}
198+
199+
func (p *PersistentVolumeClaimStatus) GetConditions() []metav1.Condition {
200+
return p.Conditions
201+
}
202+
203+
func (p *PersistentVolumeClaimStatus) SetConditions(conditions []metav1.Condition) {
204+
p.Conditions = conditions
205+
}

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

Lines changed: 151 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,81 @@ 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
115186
error:
116-
description: Error is the overall error status for the instance.
187+
description: |-
188+
TODO: remove this field once VM op changes are ready.
189+
Error is the overall error status for the instance.
117190
type: string
118191
volumes:
119192
description: VolumeStatus reflects the status for each volume.
@@ -126,29 +199,96 @@ spec:
126199
description: PersistentVolumeClaim contains details about the
127200
volume's current state.
128201
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
136202
claimName:
137203
description: ClaimName is the PVC name.
138204
type: string
139205
cnsVolumeId:
140206
description: CnsVolumeID is the volume ID for the PVC.
141207
type: string
208+
conditions:
209+
description: Conditions describes any conditions associated
210+
with this volume.
211+
items:
212+
description: "Condition contains details for one aspect
213+
of the current state of this API Resource.\n---\nThis
214+
struct is intended for direct use as an array at the
215+
field path .status.conditions. For example,\n\n\n\ttype
216+
FooStatus struct{\n\t // Represents the observations
217+
of a foo's current state.\n\t // Known .status.conditions.type
218+
are: \"Available\", \"Progressing\", and \"Degraded\"\n\t
219+
\ // +patchMergeKey=type\n\t // +patchStrategy=merge\n\t
220+
\ // +listType=map\n\t // +listMapKey=type\n\t
221+
\ Conditions []metav1.Condition `json:\"conditions,omitempty\"
222+
patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t
223+
\ // other fields\n\t}"
224+
properties:
225+
lastTransitionTime:
226+
description: |-
227+
lastTransitionTime is the last time the condition transitioned from one status to another.
228+
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
229+
format: date-time
230+
type: string
231+
message:
232+
description: |-
233+
message is a human readable message indicating details about the transition.
234+
This may be an empty string.
235+
maxLength: 32768
236+
type: string
237+
observedGeneration:
238+
description: |-
239+
observedGeneration represents the .metadata.generation that the condition was set based upon.
240+
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
241+
with respect to the current state of the instance.
242+
format: int64
243+
minimum: 0
244+
type: integer
245+
reason:
246+
description: |-
247+
reason contains a programmatic identifier indicating the reason for the condition's last transition.
248+
Producers of specific condition types may define expected values and meanings for this field,
249+
and whether the values are considered a guaranteed API.
250+
The value should be a CamelCase string.
251+
This field may not be empty.
252+
maxLength: 1024
253+
minLength: 1
254+
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
255+
type: string
256+
status:
257+
description: status of the condition, one of True,
258+
False, Unknown.
259+
enum:
260+
- "True"
261+
- "False"
262+
- Unknown
263+
type: string
264+
type:
265+
description: |-
266+
type of condition in CamelCase or in foo.example.com/CamelCase.
267+
---
268+
Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
269+
useful (see .node.status.conditions), the ability to deconflict is important.
270+
The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
271+
maxLength: 316
272+
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])$
273+
type: string
274+
required:
275+
- lastTransitionTime
276+
- message
277+
- reason
278+
- status
279+
- type
280+
type: object
281+
type: array
142282
diskUUID:
143283
description: DiskUUID is the ID obtained when volume is
144284
attached to a VM.
145285
type: string
146286
error:
147-
description: Error indicates the error which may have occurred
148-
during attach/detach.
287+
description: |-
288+
TODO: remove this field once VM op changes are ready.
289+
Error indicates the error which may have occurred during attach/detach.
149290
type: string
150291
required:
151-
- attached
152292
- claimName
153293
type: object
154294
required:
@@ -165,9 +305,3 @@ spec:
165305
storage: true
166306
subresources:
167307
status: {}
168-
status:
169-
acceptedNames:
170-
kind: ""
171-
plural: ""
172-
conditions: []
173-
storedVersions: []

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,8 +1307,10 @@ func (m *defaultManager) DetachVolume(ctx context.Context, vm *cnsvsphere.Virtua
13071307
}
13081308
}
13091309
}
1310-
return faultType, logger.LogNewErrorf(log, "failed to detach cns volume: %q from node vm: %+v. fault: %+v, opId: %q",
1310+
log.Errorf("failed to detach cns volume: %q from node vm: %+v. fault: %+v, opId: %q",
13111311
volumeID, vm, spew.Sdump(volumeOperationRes.Fault), taskInfo.ActivationId)
1312+
return faultType, fmt.Errorf("failed to detach cns volume: %q, Error: %s,",
1313+
volumeID, volumeOperationRes.Fault.LocalizedMessage)
13121314
}
13131315
log.Infof("DetachVolume: Volume detached successfully. volumeID: %q, vm: %q, opId: %q",
13141316
volumeID, vm.String(), taskInfo.ActivationId)
@@ -3495,8 +3497,10 @@ func compileBatchAttachTaskResult(ctx context.Context, result cnstypes.BaseCnsVo
34953497
// In case of failure, set faultType and error.
34963498
faultType := ExtractFaultTypeFromVolumeResponseResult(ctx, volumeOperationResult)
34973499
batchAttachResult.FaultType = faultType
3498-
msg := fmt.Sprintf("failed to batch attach cns volume: %q to node vm: %q. fault: %q. opId: %q",
3500+
log.Errorf("failed to attach cns volume: %q to node vm: %q. fault: %q. opId: %q",
34993501
volumeId, vm.String(), faultType, activationId)
3502+
msg := fmt.Sprintf("failed to attach cns volume: %q Error: %s",
3503+
volumeId, volumeOperationResult.Fault.LocalizedMessage)
35003504
batchAttachResult.Error = errors.New(msg)
35013505
log.Infof("Constructed batch attach result for volume %s with failure", volumeId)
35023506
return batchAttachResult, nil

0 commit comments

Comments
 (0)