Skip to content

Commit 53fdc7a

Browse files
fwieselfwiesel
committed
ApplyAggregates returns aggregate UUIDs
1 parent fc9ff3b commit 53fdc7a

4 files changed

Lines changed: 41 additions & 19 deletions

File tree

‎internal/controller/aggregates_controller.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ func (ac *AggregatesController) SetupWithManager(mgr ctrl.Manager) error {
115115
if ac.computeClient, err = openstack.GetServiceClient(ctx, "compute", nil); err != nil {
116116
return err
117117
}
118-
ac.computeClient.Microversion = "2.40" // gophercloud only supports numeric ids
119118

120119
return ctrl.NewControllerManagedBy(mgr).
121120
Named(AggregatesControllerName).

‎internal/controller/utils.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func ApplyAggregatesAndUpdateStatus(
166166
// Remove condition before OpenStack call to reflect unknown state on failure
167167
meta.RemoveStatusCondition(&hv.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)
168168

169-
if err := openstack.ApplyAggregates(ctx, computeClient, hv.Name, desiredAggregates); err != nil {
169+
if _, err := openstack.ApplyAggregates(ctx, computeClient, hv.Name, desiredAggregates); err != nil {
170170
return fmt.Errorf("failed to apply aggregates: %w", err)
171171
}
172172

‎internal/openstack/aggregates.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,24 @@ import (
4141
// aggregate is not found, an error is returned listing the missing aggregates.
4242
//
4343
// Pass an empty list to remove the host from all aggregates.
44-
func ApplyAggregates(ctx context.Context, serviceClient *gophercloud.ServiceClient, host string, desiredAggregates []string) error {
44+
func ApplyAggregates(ctx context.Context, serviceClient *gophercloud.ServiceClient, host string, desiredAggregates []string) ([]string, error) {
4545
log := logger.FromContext(ctx)
4646

47+
oldMicroVersion := serviceClient.Microversion
48+
serviceClient.Microversion = "2.93" // Something bigger than 2.41 for UUIDs
49+
defer func() {
50+
serviceClient.Microversion = oldMicroVersion
51+
}()
52+
4753
// Fetch all aggregates
4854
pages, err := aggregates.List(serviceClient).AllPages(ctx)
4955
if err != nil {
50-
return fmt.Errorf("failed to list aggregates: %w", err)
56+
return nil, fmt.Errorf("failed to list aggregates: %w", err)
5157
}
5258

5359
allAggregates, err := aggregates.ExtractAggregates(pages)
5460
if err != nil {
55-
return fmt.Errorf("failed to extract aggregates: %w", err)
61+
return nil, fmt.Errorf("failed to extract aggregates: %w", err)
5662
}
5763

5864
// Build desired set for lookups
@@ -61,6 +67,7 @@ func ApplyAggregates(ctx context.Context, serviceClient *gophercloud.ServiceClie
6167
desiredSet[name] = true
6268
}
6369

70+
var uuids []string
6471
var errs []error
6572
var toRemove []aggregates.Aggregate
6673

@@ -72,6 +79,7 @@ func ApplyAggregates(ctx context.Context, serviceClient *gophercloud.ServiceClie
7279
if aggregateDesired {
7380
// Mark as found
7481
delete(desiredSet, agg.Name)
82+
uuids = append(uuids, agg.UUID)
7583

7684
if !hostInAggregate {
7785
// Add host to this aggregate
@@ -107,5 +115,9 @@ func ApplyAggregates(ctx context.Context, serviceClient *gophercloud.ServiceClie
107115
}
108116
}
109117

110-
return errors.Join(errs...)
118+
if len(errs) > 0 {
119+
return nil, errors.Join(errs...)
120+
} else {
121+
return uuids, nil
122+
}
111123
}

‎internal/openstack/aggregates_test.go‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,23 @@ var _ = Describe("ApplyAggregates", func() {
3737
"availability_zone": "az1",
3838
"deleted": false,
3939
"id": 1,
40+
"uuid": "uuid-agg1",
4041
"hosts": ["test-host"]
4142
},
4243
{
4344
"name": "agg2",
4445
"availability_zone": "az2",
4546
"deleted": false,
4647
"id": 2,
48+
"uuid": "uuid-agg2",
4749
"hosts": ["test-host"]
4850
},
4951
{
5052
"name": "agg3",
5153
"availability_zone": "az3",
5254
"deleted": false,
5355
"id": 3,
56+
"uuid": "uuid-agg3",
5457
"hosts": []
5558
}
5659
]
@@ -62,6 +65,7 @@ var _ = Describe("ApplyAggregates", func() {
6265
"availability_zone": "az3",
6366
"deleted": false,
6467
"id": 3,
68+
"uuid": "uuid-agg3",
6569
"hosts": ["test-host"]
6670
}
6771
}`
@@ -72,6 +76,7 @@ var _ = Describe("ApplyAggregates", func() {
7276
"availability_zone": "az1",
7377
"deleted": false,
7478
"id": 1,
79+
"uuid": "uuid-agg1",
7580
"hosts": []
7681
}
7782
}`
@@ -108,8 +113,9 @@ var _ = Describe("ApplyAggregates", func() {
108113

109114
It("should add host to agg3", func() {
110115
serviceClient := client.ServiceClient(fakeServer)
111-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1", "agg2", "agg3"})
116+
uuids, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1", "agg2", "agg3"})
112117
Expect(err).NotTo(HaveOccurred())
118+
Expect(uuids).To(ConsistOf("uuid-agg1", "uuid-agg2", "uuid-agg3"))
113119
})
114120
})
115121

@@ -130,8 +136,9 @@ var _ = Describe("ApplyAggregates", func() {
130136

131137
It("should remove host from agg1", func() {
132138
serviceClient := client.ServiceClient(fakeServer)
133-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg2"})
139+
uuids, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg2"})
134140
Expect(err).NotTo(HaveOccurred())
141+
Expect(uuids).To(ConsistOf("uuid-agg2"))
135142
})
136143
})
137144

@@ -158,15 +165,16 @@ var _ = Describe("ApplyAggregates", func() {
158165
removeCalls++
159166
w.Header().Add("Content-Type", "application/json")
160167
w.WriteHeader(http.StatusOK)
161-
fmt.Fprint(w, `{"aggregate": {"name": "agg2", "id": 2, "hosts": []}}`)
168+
fmt.Fprint(w, `{"aggregate": {"name": "agg2", "id": 2, "uuid": "uuid-agg2", "hosts": []}}`)
162169
})
163170
})
164171

165172
It("should remove host from all aggregates", func() {
166173
serviceClient := client.ServiceClient(fakeServer)
167-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{})
174+
uuids, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{})
168175
Expect(err).NotTo(HaveOccurred())
169176
Expect(removeCalls).To(Equal(2))
177+
Expect(uuids).To(BeEmpty())
170178
})
171179
})
172180

@@ -181,8 +189,9 @@ var _ = Describe("ApplyAggregates", func() {
181189

182190
It("should not make any changes", func() {
183191
serviceClient := client.ServiceClient(fakeServer)
184-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1", "agg2"})
192+
uuids, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1", "agg2"})
185193
Expect(err).NotTo(HaveOccurred())
194+
Expect(uuids).To(ConsistOf("uuid-agg1", "uuid-agg2"))
186195
})
187196
})
188197

@@ -209,8 +218,9 @@ var _ = Describe("ApplyAggregates", func() {
209218

210219
It("should replace agg1 with agg3", func() {
211220
serviceClient := client.ServiceClient(fakeServer)
212-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg2", "agg3"})
221+
uuids, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg2", "agg3"})
213222
Expect(err).NotTo(HaveOccurred())
223+
Expect(uuids).To(ConsistOf("uuid-agg2", "uuid-agg3"))
214224
})
215225
})
216226

@@ -224,7 +234,7 @@ var _ = Describe("ApplyAggregates", func() {
224234

225235
It("should return an error", func() {
226236
serviceClient := client.ServiceClient(fakeServer)
227-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1"})
237+
_, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1"})
228238
Expect(err).To(HaveOccurred())
229239
Expect(err.Error()).To(ContainSubstring("failed to list aggregates"))
230240
})
@@ -246,7 +256,7 @@ var _ = Describe("ApplyAggregates", func() {
246256

247257
It("should return an error", func() {
248258
serviceClient := client.ServiceClient(fakeServer)
249-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1", "agg2", "agg3"})
259+
_, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1", "agg2", "agg3"})
250260
Expect(err).To(HaveOccurred())
251261
Expect(err.Error()).To(ContainSubstring("failed to add host"))
252262
})
@@ -268,7 +278,7 @@ var _ = Describe("ApplyAggregates", func() {
268278

269279
It("should return an error", func() {
270280
serviceClient := client.ServiceClient(fakeServer)
271-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg2"})
281+
_, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg2"})
272282
Expect(err).To(HaveOccurred())
273283
Expect(err.Error()).To(ContainSubstring("failed to remove host"))
274284
})
@@ -291,8 +301,9 @@ var _ = Describe("ApplyAggregates", func() {
291301

292302
It("should add new host to aggregate", func() {
293303
serviceClient := client.ServiceClient(fakeServer)
294-
err := ApplyAggregates(ctx, serviceClient, "new-host", []string{"agg3"})
304+
uuids, err := ApplyAggregates(ctx, serviceClient, "new-host", []string{"agg3"})
295305
Expect(err).NotTo(HaveOccurred())
306+
Expect(uuids).To(ConsistOf("uuid-agg3"))
296307
})
297308
})
298309

@@ -319,7 +330,7 @@ var _ = Describe("ApplyAggregates", func() {
319330

320331
It("should return combined errors", func() {
321332
serviceClient := client.ServiceClient(fakeServer)
322-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg2", "agg3"})
333+
_, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg2", "agg3"})
323334
Expect(err).To(HaveOccurred())
324335
// Verify it's a joined error with multiple failures
325336
Expect(err.Error()).To(And(ContainSubstring("failed to add host"), ContainSubstring("failed to remove host")))
@@ -337,7 +348,7 @@ var _ = Describe("ApplyAggregates", func() {
337348

338349
It("should return an error about missing aggregate", func() {
339350
serviceClient := client.ServiceClient(fakeServer)
340-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1", "agg2", "agg4"})
351+
_, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg1", "agg2", "agg4"})
341352
Expect(err).To(HaveOccurred())
342353
Expect(err.Error()).To(ContainSubstring("aggregates not found"))
343354
Expect(err.Error()).To(ContainSubstring("agg4"))
@@ -355,7 +366,7 @@ var _ = Describe("ApplyAggregates", func() {
355366

356367
It("should return an error listing all missing aggregates", func() {
357368
serviceClient := client.ServiceClient(fakeServer)
358-
err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg4", "agg5", "agg6"})
369+
_, err := ApplyAggregates(ctx, serviceClient, "test-host", []string{"agg4", "agg5", "agg6"})
359370
Expect(err).To(HaveOccurred())
360371
Expect(err.Error()).To(ContainSubstring("aggregates not found"))
361372
// Check that all missing aggregates are mentioned in the error

0 commit comments

Comments
 (0)