Skip to content

Commit c059af9

Browse files
skogtaskogta
authored
Use K8s conditions for CnsNodeVMBatchAttachment CRD (#3734)
Add back Error and Atatched fields temporarily so that it does not break VM op compatibility
1 parent 2342695 commit c059af9

24 files changed

+3597
-82
lines changed

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

Lines changed: 61 additions & 8 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,29 @@ type VolumeStatus struct {
118143
type PersistentVolumeClaimStatus struct {
119144
// ClaimName is the PVC name.
120145
ClaimName string `json:"claimName"`
146+
// +optional
147+
148+
// CnsVolumeID is the volume ID for the PVC.
149+
CnsVolumeID string `json:"cnsVolumeId,omitempty"`
150+
// +optional
151+
152+
// DiskUUID is the ID obtained when volume is attached to a VM.
153+
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"`
162+
163+
// TODO: remove this field once VM op changes are ready.
121164
// Attached indicates the attach status of a PVC.
122165
// If volume is not attached, Attached will be set to false.
123166
// If volume is attached, Attached will be set to true.
124167
// If volume is detached successfully, its entry will be removed from VolumeStatus.
125168
Attached bool `json:"attached"`
126-
// Error indicates the error which may have occurred during attach/detach.
127-
Error string `json:"error,omitempty"`
128-
// CnsVolumeID is the volume ID for the PVC.
129-
CnsVolumeID string `json:"cnsVolumeId,omitempty"`
130-
// DiskUUID is the ID obtained when volume is attached to a VM.
131-
DiskUUID string `json:"diskUUID,omitempty"`
132169
}
133170

134171
// +genclient
@@ -157,3 +194,19 @@ type CnsNodeVMBatchAttachmentList struct {
157194
metav1.ListMeta `json:"metadata,omitempty"`
158195
Items []CnsNodeVMBatchAttachment `json:"items"`
159196
}
197+
198+
func (in *CnsNodeVMBatchAttachment) GetConditions() []metav1.Condition {
199+
return in.Status.Conditions
200+
}
201+
202+
func (in *CnsNodeVMBatchAttachment) SetConditions(conditions []metav1.Condition) {
203+
in.Status.Conditions = conditions
204+
}
205+
206+
func (p *PersistentVolumeClaimStatus) GetConditions() []metav1.Condition {
207+
return p.Conditions
208+
}
209+
210+
func (p *PersistentVolumeClaimStatus) SetConditions(conditions []metav1.Condition) {
211+
p.Conditions = conditions
212+
}

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

Lines changed: 153 additions & 4 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.
@@ -128,6 +201,7 @@ spec:
128201
properties:
129202
attached:
130203
description: |-
204+
TODO: remove this field once VM op changes are ready.
131205
Attached indicates the attach status of a PVC.
132206
If volume is not attached, Attached will be set to false.
133207
If volume is attached, Attached will be set to true.
@@ -139,13 +213,88 @@ spec:
139213
cnsVolumeId:
140214
description: CnsVolumeID is the volume ID for the PVC.
141215
type: string
216+
conditions:
217+
description: Conditions describes any conditions associated
218+
with this volume.
219+
items:
220+
description: "Condition contains details for one aspect
221+
of the current state of this API Resource.\n---\nThis
222+
struct is intended for direct use as an array at the
223+
field path .status.conditions. For example,\n\n\n\ttype
224+
FooStatus struct{\n\t // Represents the observations
225+
of a foo's current state.\n\t // Known .status.conditions.type
226+
are: \"Available\", \"Progressing\", and \"Degraded\"\n\t
227+
\ // +patchMergeKey=type\n\t // +patchStrategy=merge\n\t
228+
\ // +listType=map\n\t // +listMapKey=type\n\t
229+
\ Conditions []metav1.Condition `json:\"conditions,omitempty\"
230+
patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t
231+
\ // other fields\n\t}"
232+
properties:
233+
lastTransitionTime:
234+
description: |-
235+
lastTransitionTime is the last time the condition transitioned from one status to another.
236+
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
237+
format: date-time
238+
type: string
239+
message:
240+
description: |-
241+
message is a human readable message indicating details about the transition.
242+
This may be an empty string.
243+
maxLength: 32768
244+
type: string
245+
observedGeneration:
246+
description: |-
247+
observedGeneration represents the .metadata.generation that the condition was set based upon.
248+
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
249+
with respect to the current state of the instance.
250+
format: int64
251+
minimum: 0
252+
type: integer
253+
reason:
254+
description: |-
255+
reason contains a programmatic identifier indicating the reason for the condition's last transition.
256+
Producers of specific condition types may define expected values and meanings for this field,
257+
and whether the values are considered a guaranteed API.
258+
The value should be a CamelCase string.
259+
This field may not be empty.
260+
maxLength: 1024
261+
minLength: 1
262+
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
263+
type: string
264+
status:
265+
description: status of the condition, one of True,
266+
False, Unknown.
267+
enum:
268+
- "True"
269+
- "False"
270+
- Unknown
271+
type: string
272+
type:
273+
description: |-
274+
type of condition in CamelCase or in foo.example.com/CamelCase.
275+
---
276+
Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
277+
useful (see .node.status.conditions), the ability to deconflict is important.
278+
The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
279+
maxLength: 316
280+
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])$
281+
type: string
282+
required:
283+
- lastTransitionTime
284+
- message
285+
- reason
286+
- status
287+
- type
288+
type: object
289+
type: array
142290
diskUUID:
143291
description: DiskUUID is the ID obtained when volume is
144292
attached to a VM.
145293
type: string
146294
error:
147-
description: Error indicates the error which may have occurred
148-
during attach/detach.
295+
description: |-
296+
TODO: remove this field once VM op changes are ready.
297+
Error indicates the error which may have occurred during attach/detach.
149298
type: string
150299
required:
151300
- attached
@@ -170,4 +319,4 @@ status:
170319
kind: ""
171320
plural: ""
172321
conditions: []
173-
storedVersions: []
322+
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)