Skip to content

Commit 2119899

Browse files
skogtaskogta
committed
Use K8s conditions for CnsNodeVMBatchAttachment CRD
1 parent 7a6acd9 commit 2119899

24 files changed

+3549
-111
lines changed

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

Lines changed: 48 additions & 10 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,15 @@ 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"`
109130
}
110131

111132
type VolumeStatus struct {
@@ -118,17 +139,18 @@ type VolumeStatus struct {
118139
type PersistentVolumeClaimStatus struct {
119140
// ClaimName is the PVC name.
120141
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"`
142+
// +optional
143+
128144
// CnsVolumeID is the volume ID for the PVC.
129145
CnsVolumeID string `json:"cnsVolumeId,omitempty"`
146+
// +optional
147+
130148
// DiskUUID is the ID obtained when volume is attached to a VM.
131149
DiskUUID string `json:"diskUUID,omitempty"`
150+
// +optional
151+
152+
// Conditions describes any conditions associated with this volume.
153+
Conditions []metav1.Condition `json:"conditions,omitempty"`
132154
}
133155

134156
// +genclient
@@ -138,7 +160,7 @@ type PersistentVolumeClaimStatus struct {
138160
// +kubebuilder:subresource:status
139161
// +kubebuilder:object:root=true
140162
// +kubebuilder:resource:shortName=batchattach
141-
// +kubebuilder:printcolumn:name="NodeUUID",type="string",JSONPath=".spec.nodeUUID"
163+
// +kubebuilder:printcolumn:name="InstanceUUID",type="string",JSONPath=".spec.instanceUUID"
142164

143165
// CnsNodeVMBatchAttachment is the Schema for the cnsnodevmbatchattachments API
144166
type CnsNodeVMBatchAttachment struct {
@@ -157,3 +179,19 @@ type CnsNodeVMBatchAttachmentList struct {
157179
metav1.ListMeta `json:"metadata,omitempty"`
158180
Items []CnsNodeVMBatchAttachment `json:"items"`
159181
}
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: 148 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ spec:
1717
scope: Namespaced
1818
versions:
1919
- additionalPrinterColumns:
20-
- jsonPath: .spec.nodeUUID
21-
name: NodeUUID
20+
- jsonPath: .spec.instanceUUID
21+
name: InstanceUUID
2222
type: string
2323
name: v1alpha1
2424
schema:
@@ -112,9 +112,77 @@ spec:
112112
description: CnsNodeVMBatchAttachmentStatus defines the observed state
113113
of CnsNodeVMBatchAttachment
114114
properties:
115-
error:
116-
description: Error is the overall error status for the instance.
117-
type: string
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
118186
volumes:
119187
description: VolumeStatus reflects the status for each volume.
120188
items:
@@ -126,29 +194,91 @@ spec:
126194
description: PersistentVolumeClaim contains details about the
127195
volume's current state.
128196
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
136197
claimName:
137198
description: ClaimName is the PVC name.
138199
type: string
139200
cnsVolumeId:
140201
description: CnsVolumeID is the volume ID for the PVC.
141202
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
142277
diskUUID:
143278
description: DiskUUID is the ID obtained when volume is
144279
attached to a VM.
145280
type: string
146-
error:
147-
description: Error indicates the error which may have occurred
148-
during attach/detach.
149-
type: string
150281
required:
151-
- attached
152282
- claimName
153283
type: object
154284
required:
@@ -170,4 +300,4 @@ status:
170300
kind: ""
171301
plural: ""
172302
conditions: []
173-
storedVersions: []
303+
storedVersions: []

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,8 +1304,10 @@ func (m *defaultManager) DetachVolume(ctx context.Context, vm *cnsvsphere.Virtua
13041304
}
13051305
}
13061306
}
1307-
return faultType, logger.LogNewErrorf(log, "failed to detach cns volume: %q from node vm: %+v. fault: %+v, opId: %q",
1307+
log.Errorf("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)
13091311
}
13101312
log.Infof("DetachVolume: Volume detached successfully. volumeID: %q, vm: %q, opId: %q",
13111313
volumeID, vm.String(), taskInfo.ActivationId)
@@ -3492,8 +3494,10 @@ func compileBatchAttachTaskResult(ctx context.Context, result cnstypes.BaseCnsVo
34923494
// In case of failure, set faultType and error.
34933495
faultType := ExtractFaultTypeFromVolumeResponseResult(ctx, volumeOperationResult)
34943496
batchAttachResult.FaultType = faultType
3495-
msg := fmt.Sprintf("failed to batch attach cns volume: %q to node vm: %q. fault: %q. opId: %q",
3497+
log.Errorf("failed to attach cns volume: %q to node vm: %q. fault: %q. opId: %q",
34963498
volumeId, vm.String(), faultType, activationId)
3499+
msg := fmt.Sprintf("failed to attach cns volume: %q Error: %s",
3500+
volumeId, volumeOperationResult.Fault.LocalizedMessage)
34973501
batchAttachResult.Error = errors.New(msg)
34983502
log.Infof("Constructed batch attach result for volume %s with failure", volumeId)
34993503
return batchAttachResult, nil

0 commit comments

Comments
 (0)