Skip to content

Commit a84477e

Browse files
elivloelivlo
authored
Fix bug in cloud client missing the project id and refactor the package (#7)
* fix: use project id when getting VM by name * refactor: remove global projectID from cloudstack client * refactor: require arguments in snapshot functions * refactor: use project id default in cloudstack methods * docs: update default project id option description * fix: restore count checks in vms, volumes, snapshots * fix: add ttl for syncer job to allow helm upgrade * refactor: remove dead code in cloud package
1 parent 3c9de10 commit a84477e

7 files changed

Lines changed: 89 additions & 122 deletions

File tree

‎charts/cloudstack-csi/Chart.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ apiVersion: v2
22
name: cloudstack-csi
33
description: A Helm chart for CloudStack CSI driver
44
type: application
5-
version: 3.0.1
5+
version: 3.0.2
66
appVersion: 3.0.0
77
sources:
88
- https://github.com/cloudstack/cloudstack-csi-driver

‎charts/cloudstack-csi/templates/syncer-job.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ metadata:
1414
{{- end }}
1515
spec:
1616
backoffLimit: {{ .Values.syncer.backoffLimit }}
17+
ttlSecondsAfterFinished: {{ .Values.syncer.ttlSecondsAfterFinished }}
1718
template:
1819
spec:
1920
securityContext: {{- toYaml .Values.syncer.podSecurityContext | nindent 8 }}

‎charts/cloudstack-csi/values.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ syncer:
350350

351351
# Job configurations
352352
backoffLimit: 4
353+
ttlSecondsAfterFinished: 60
353354
restartPolicy: Never
354355

355356
# securityContext on the syncer job

‎pkg/cloud/cloud.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"errors"
2727

2828
"github.com/apache/cloudstack-go/v2/cloudstack"
29+
"k8s.io/klog/v2"
2930
)
3031

3132
// Interface is the CloudStack client interface.
@@ -70,6 +71,7 @@ type Volume struct {
7071
DeviceID string
7172
}
7273

74+
// Snapshot represents a CloudStack snapshot.
7375
type Snapshot struct {
7476
ID string
7577
Name string
@@ -91,20 +93,26 @@ type VM struct {
9193

9294
// Specific errors.
9395
var (
94-
ErrNotFound = errors.New("not found")
95-
ErrTooManyResults = errors.New("too many results")
96-
ErrAlreadyExists = errors.New("already exists")
96+
ErrNotFound = errors.New("not found")
97+
ErrAlreadyExists = errors.New("already exists")
9798
)
9899

99100
// client is the implementation of Interface.
100101
type client struct {
101102
*cloudstack.CloudStackClient
102-
projectID string
103+
projectID string // Used by some specific cloudstack api calls
103104
}
104105

105106
// New creates a new cloud connector, given its configuration.
106107
func New(config *Config) Interface {
107108
csClient := cloudstack.NewAsyncClient(config.APIURL, config.APIKey, config.SecretKey, config.VerifySSL)
108109

110+
// Set the project id to every request that support options.
111+
// This is possible because we also could work in one project only with the previous implementation.
112+
if config.ProjectID != "" {
113+
csClient.DefaultOptions(cloudstack.WithProject(config.ProjectID))
114+
klog.Background().V(2).Info("Set projectID to cloud connector", "projectID", config.ProjectID)
115+
}
116+
109117
return &client{csClient, config.ProjectID}
110118
}

‎pkg/cloud/snapshots.go‎

Lines changed: 31 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -30,46 +30,33 @@ import (
3030

3131
func (c *client) GetSnapshotByID(ctx context.Context, snapshotID string) (*Snapshot, error) {
3232
logger := klog.FromContext(ctx)
33-
p := c.Snapshot.NewListSnapshotsParams()
34-
if snapshotID != "" {
35-
p.SetId(snapshotID)
36-
}
37-
if c.projectID != "" {
38-
p.SetProjectid(c.projectID)
39-
}
40-
logger.V(2).Info("CloudStack API call", "command", "ListSnapshots", "params", map[string]string{
41-
"id": snapshotID,
42-
"projectid": c.projectID,
33+
logger.V(2).Info("CloudStack API call", "command", "GetSnapshotByID", "params", map[string]string{
34+
"id": snapshotID,
4335
})
44-
l, err := c.Snapshot.ListSnapshots(p)
36+
37+
snapshot, count, err := c.Snapshot.GetSnapshotByID(snapshotID)
4538
if err != nil {
39+
if count == 0 {
40+
return nil, ErrNotFound
41+
}
42+
4643
return nil, err
4744
}
48-
if l.Count == 0 {
49-
return nil, ErrNotFound
50-
}
51-
if l.Count > 1 {
52-
return nil, ErrTooManyResults
53-
}
54-
snapshot := l.Snapshots[0]
55-
s := Snapshot{
45+
46+
return &Snapshot{
5647
ID: snapshot.Id,
5748
Name: snapshot.Name,
5849
DomainID: snapshot.Domainid,
5950
ProjectID: snapshot.Projectid,
6051
ZoneID: snapshot.Zoneid,
6152
VolumeID: snapshot.Volumeid,
62-
}
63-
64-
return &s, nil
53+
}, nil
6554
}
6655

6756
func (c *client) CreateSnapshot(ctx context.Context, volumeID, name string) (*Snapshot, error) {
6857
logger := klog.FromContext(ctx)
6958
p := c.Snapshot.NewCreateSnapshotParams(volumeID)
70-
if name != "" {
71-
p.SetName(name)
72-
}
59+
p.SetName(name)
7360
logger.V(2).Info("CloudStack API call", "command", "CreateSnapshot", "params", map[string]string{
7461
"volumeid": volumeID,
7562
"name": name,
@@ -80,7 +67,7 @@ func (c *client) CreateSnapshot(ctx context.Context, volumeID, name string) (*Sn
8067
return nil, status.Errorf(codes.Internal, "Error %v", err)
8168
}
8269

83-
snap := Snapshot{
70+
return &Snapshot{
8471
ID: snapshot.Id,
8572
Name: snapshot.Name,
8673
Size: snapshot.Virtualsize,
@@ -89,9 +76,7 @@ func (c *client) CreateSnapshot(ctx context.Context, volumeID, name string) (*Sn
8976
ZoneID: snapshot.Zoneid,
9077
VolumeID: snapshot.Volumeid,
9178
CreatedAt: snapshot.Created,
92-
}
93-
94-
return &snap, nil
79+
}, nil
9580
}
9681

9782
func (c *client) DeleteSnapshot(_ context.Context, snapshotID string) error {
@@ -107,58 +92,49 @@ func (c *client) DeleteSnapshot(_ context.Context, snapshotID string) error {
10792

10893
func (c *client) GetSnapshotByName(ctx context.Context, name string) (*Snapshot, error) {
10994
logger := klog.FromContext(ctx)
110-
if name == "" {
111-
return nil, ErrNotFound
112-
}
113-
p := c.Snapshot.NewListSnapshotsParams()
114-
p.SetName(name)
115-
if c.projectID != "" {
116-
p.SetProjectid(c.projectID)
117-
}
118-
logger.V(2).Info("CloudStack API call", "command", "ListSnapshots", "params", map[string]string{
119-
"name": name,
120-
"projectid": c.projectID,
95+
logger.V(2).Info("CloudStack API call", "command", "GetSnapshotByName", "params", map[string]string{
96+
"name": name,
12197
})
122-
l, err := c.Snapshot.ListSnapshots(p)
98+
snapshot, count, err := c.Snapshot.GetSnapshotByName(name)
12399
if err != nil {
100+
if count == 0 {
101+
return nil, ErrNotFound
102+
}
103+
124104
return nil, err
125105
}
126-
if l.Count == 0 {
127-
return nil, ErrNotFound
128-
}
129-
if l.Count > 1 {
130-
return nil, ErrTooManyResults
131-
}
132-
snapshot := l.Snapshots[0]
133-
s := Snapshot{
106+
107+
return &Snapshot{
134108
ID: snapshot.Id,
135109
Name: snapshot.Name,
136110
DomainID: snapshot.Domainid,
137111
ProjectID: snapshot.Projectid,
138112
ZoneID: snapshot.Zoneid,
139113
VolumeID: snapshot.Volumeid,
140114
CreatedAt: snapshot.Created,
141-
}
142-
143-
return &s, nil
115+
}, nil
144116
}
145117

146118
func (c *client) ListSnapshots(ctx context.Context, volumeID, snapshotID string) ([]*Snapshot, error) {
147119
logger := klog.FromContext(ctx)
148120
p := c.Snapshot.NewListSnapshotsParams()
121+
// snapshotID is optional: csi.ListSnapshotsRequest
149122
if snapshotID != "" {
150123
p.SetId(snapshotID)
151124
}
125+
// volumeID is optional: csi.ListSnapshotsRequest
152126
if volumeID != "" {
153127
p.SetVolumeid(volumeID)
154128
}
129+
130+
// There is no list function that uses the client default project id option
155131
if c.projectID != "" {
156132
p.SetProjectid(c.projectID)
157133
}
134+
158135
logger.V(2).Info("CloudStack API call", "command", "ListSnapshots", "params", map[string]string{
159-
"id": snapshotID,
160-
"volumeid": volumeID,
161-
"projectid": c.projectID,
136+
"id": snapshotID,
137+
"volumeid": volumeID,
162138
})
163139
l, err := c.Snapshot.ListSnapshots(p)
164140
if err != nil {

‎pkg/cloud/vms.go‎

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,27 +27,18 @@ import (
2727

2828
func (c *client) GetVMByID(ctx context.Context, vmID string) (*VM, error) {
2929
logger := klog.FromContext(ctx)
30-
p := c.VirtualMachine.NewListVirtualMachinesParams()
31-
p.SetId(vmID)
32-
if c.projectID != "" {
33-
p.SetProjectid(c.projectID)
34-
}
35-
logger.V(2).Info("CloudStack API call", "command", "ListVirtualMachines", "params", map[string]string{
36-
"id": vmID,
37-
"projectID": c.projectID,
30+
logger.V(2).Info("CloudStack API call", "command", "GetVirtualMachineByID", "params", map[string]string{
31+
"id": vmID,
3832
})
39-
l, err := c.VirtualMachine.ListVirtualMachines(p)
33+
34+
vm, count, err := c.VirtualMachine.GetVirtualMachineByID(vmID)
4035
if err != nil {
36+
if count == 0 {
37+
return nil, ErrNotFound
38+
}
39+
4140
return nil, err
4241
}
43-
if l.Count == 0 {
44-
return nil, ErrNotFound
45-
}
46-
if l.Count > 1 {
47-
return nil, ErrTooManyResults
48-
}
49-
vm := l.VirtualMachines[0]
50-
logger.V(2).Info("Returning VM", "vmID", vm.Id, "zoneID", vm.Zoneid)
5142

5243
return &VM{
5344
ID: vm.Id,
@@ -57,22 +48,18 @@ func (c *client) GetVMByID(ctx context.Context, vmID string) (*VM, error) {
5748

5849
func (c *client) getVMByName(ctx context.Context, name string) (*VM, error) {
5950
logger := klog.FromContext(ctx)
60-
p := c.VirtualMachine.NewListVirtualMachinesParams()
61-
p.SetName(name)
62-
logger.V(2).Info("CloudStack API call", "command", "ListVirtualMachines", "params", map[string]string{
51+
logger.V(2).Info("CloudStack API call", "command", "GetVirtualMachineByName", "params", map[string]string{
6352
"name": name,
6453
})
65-
l, err := c.VirtualMachine.ListVirtualMachines(p)
54+
55+
vm, count, err := c.VirtualMachine.GetVirtualMachineByName(name)
6656
if err != nil {
57+
if count == 0 {
58+
return nil, ErrNotFound
59+
}
60+
6761
return nil, err
6862
}
69-
if l.Count == 0 {
70-
return nil, ErrNotFound
71-
}
72-
if l.Count > 1 {
73-
return nil, ErrTooManyResults
74-
}
75-
vm := l.VirtualMachines[0]
7663

7764
return &VM{
7865
ID: vm.Id,

0 commit comments

Comments
 (0)