Skip to content

Commit fdf636d

Browse files
abhay-nutanixabhay-nutanix
authored
fix: Support multiple values by category (#611)
* fix: Support multiple values by category (#575) * fix: Support multiple values by category * fix: nix-action version name change * removing flatmaps and keeping useCategoryMapping * fixing additional categories E2E * fixing additional categories E2E * using assertion in test, Addressing comments * addressing comments * reverting vals changes * fix: lint error
1 parent adf0cac commit fdf636d

File tree

6 files changed

+89
-33
lines changed

6 files changed

+89
-33
lines changed

‎controllers/helpers.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"reflect"
23+
"slices"
2324
"strings"
2425
"time"
2526

@@ -599,11 +600,19 @@ func getOrCreateCategory(ctx context.Context, client *prismclientv3.Client, cate
599600
return categoryValue, nil
600601
}
601602

602-
// GetCategoryVMSpec returns a flatmap of categories and their values
603-
func GetCategoryVMSpec(ctx context.Context, client *prismclientv3.Client, categoryIdentifiers []*infrav1.NutanixCategoryIdentifier) (map[string]string, error) {
603+
// GetCategoryVMSpec returns the categories_mapping supporting multiple values per key.
604+
func GetCategoryVMSpec(
605+
ctx context.Context,
606+
client *prismclientv3.Client,
607+
categoryIdentifiers []*infrav1.NutanixCategoryIdentifier,
608+
) (map[string][]string, error) {
604609
log := ctrl.LoggerFrom(ctx)
605-
categorySpec := map[string]string{}
610+
categorySpec := map[string][]string{}
611+
606612
for _, ci := range categoryIdentifiers {
613+
if ci == nil {
614+
return nil, fmt.Errorf("category identifier cannot be nil")
615+
}
607616
categoryValue, err := getCategoryValue(ctx, client, ci.Key, ci.Value)
608617
if err != nil {
609618
errorMsg := fmt.Errorf("error occurred while to retrieving category value %s in category %s. error: %v", ci.Value, ci.Key, err)
@@ -615,8 +624,11 @@ func GetCategoryVMSpec(ctx context.Context, client *prismclientv3.Client, catego
615624
log.Error(errorMsg, "category value not found")
616625
return nil, errorMsg
617626
}
618-
categorySpec[ci.Key] = ci.Value
627+
if !slices.Contains(categorySpec[ci.Key], ci.Value) {
628+
categorySpec[ci.Key] = append(categorySpec[ci.Key], ci.Value)
629+
}
619630
}
631+
620632
return categorySpec, nil
621633
}
622634

‎controllers/nutanixmachine_controller.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -596,18 +596,19 @@ func (r *NutanixMachineReconciler) getOrCreateVM(rctx *nctx.MachineContext) (*pr
596596
}
597597
}
598598

599-
// Set Categories to VM Sepc before creating VM
600-
categories, err := GetCategoryVMSpec(ctx, v3Client, r.getMachineCategoryIdentifiers(rctx))
599+
// Set categories on VM; support multiple values via categories_mapping when possible
600+
categoriesMapping, err := GetCategoryVMSpec(ctx, v3Client, r.getMachineCategoryIdentifiers(rctx))
601601
if err != nil {
602602
errorMsg := fmt.Errorf("error occurred while creating category spec for vm %s: %v", vmName, err)
603603
rctx.SetFailureStatus(capierrors.CreateMachineError, errorMsg)
604604
return nil, errorMsg
605605
}
606606

607607
vmMetadata := &prismclientv3.Metadata{
608-
Kind: utils.StringPtr("vm"),
609-
SpecVersion: utils.Int64Ptr(1),
610-
Categories: categories,
608+
Kind: utils.StringPtr("vm"),
609+
SpecVersion: utils.Int64Ptr(1),
610+
UseCategoriesMapping: ptr.To(true),
611+
CategoriesMapping: categoriesMapping,
611612
}
612613
// Set Project in VM Spec before creating VM
613614
err = r.addVMToProject(rctx, vmMetadata)

‎controllers/nutanixmachine_controller_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,3 +605,32 @@ func TestNutanixClusterReconcilerGetDiskList(t *testing.T) {
605605
})
606606
}
607607
}
608+
609+
func TestReconcile_VMMetadataCategoriesMapping_MultipleValues(t *testing.T) {
610+
ctrl := gomock.NewController(t)
611+
defer ctrl.Finish()
612+
613+
ctx := context.Background()
614+
mockV3 := mocknutanixv3.NewMockService(ctrl)
615+
client := &prismclientv3.Client{V3: mockV3}
616+
617+
// Prepare inputs
618+
clusterName := "TestCluster"
619+
620+
// Default category key/value lookups used by GetCategoryVMSpecMapping
621+
defaultKey := infrav1.DefaultCAPICategoryKeyForName
622+
mockV3.EXPECT().GetCategoryValue(ctx, defaultKey, clusterName).Return(&prismclientv3.CategoryValueStatus{Value: &clusterName}, nil)
623+
mockV3.EXPECT().GetCategoryValue(ctx, "TestCategory", "TestValue1").Return(&prismclientv3.CategoryValueStatus{Value: ptr.To("TestValue1")}, nil)
624+
mockV3.EXPECT().GetCategoryValue(ctx, "TestCategory", "TestValue2").Return(&prismclientv3.CategoryValueStatus{Value: ptr.To("TestValue2")}, nil)
625+
mockV3.EXPECT().GetCategoryValue(ctx, "TestCategory", "TestValue1").Return(&prismclientv3.CategoryValueStatus{Value: ptr.To("TestValue1")}, nil)
626+
627+
ids := []*infrav1.NutanixCategoryIdentifier{
628+
{Key: defaultKey, Value: clusterName},
629+
{Key: "TestCategory", Value: "TestValue1"},
630+
{Key: "TestCategory", Value: "TestValue2"},
631+
{Key: "TestCategory", Value: "TestValue1"},
632+
}
633+
mapping, err := GetCategoryVMSpec(ctx, client, ids)
634+
require.NoError(t, err)
635+
require.ElementsMatch(t, []string{"TestValue1", "TestValue2"}, mapping["TestCategory"])
636+
}

‎test/e2e/categories_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ var _ = Describe("Nutanix categories", Label("capx-feature-test", "categories"),
9494
})
9595

9696
By("Checking if there are VMs assigned to this category", func() {
97-
expectedCategories := map[string]string{
98-
expectedClusterNameCategoryKey: clusterName,
97+
expectedCategories := map[string][]string{
98+
expectedClusterNameCategoryKey: {clusterName},
9999
}
100100
testHelper.verifyCategoriesNutanixMachines(ctx, clusterName, namespace.Name, expectedCategories)
101101
})
@@ -121,10 +121,10 @@ var _ = Describe("Nutanix categories", Label("capx-feature-test", "categories"),
121121

122122
By("Verify if additional categories are assigned to the vms", func() {
123123
expectedClusterNameCategoryKey := infrav1.DefaultCAPICategoryKeyForName
124-
expectedCategories := map[string]string{
125-
expectedClusterNameCategoryKey: clusterName,
126-
"AppType": "Kubernetes",
127-
"Environment": "Dev",
124+
expectedCategories := map[string][]string{
125+
expectedClusterNameCategoryKey: {clusterName},
126+
"AppType": {"Kubernetes"},
127+
"Environment": {"Dev", "Testing"},
128128
}
129129

130130
testHelper.verifyCategoriesNutanixMachines(ctx, clusterName, namespace.Name, expectedCategories)
@@ -209,8 +209,8 @@ var _ = Describe("Nutanix categories", Label("capx-feature-test", "categories"),
209209
})
210210

211211
By("Checking if there are VMs assigned to this category", func() {
212-
expectedCategories := map[string]string{
213-
expectedClusterNameCategoryKey: clusterName,
212+
expectedCategories := map[string][]string{
213+
expectedClusterNameCategoryKey: {clusterName},
214214
}
215215
testHelper.verifyCategoriesNutanixMachines(ctx, clusterName, namespace.Name, expectedCategories)
216216
})
@@ -266,8 +266,8 @@ var _ = Describe("Nutanix categories", Label("capx-feature-test", "categories"),
266266
})
267267

268268
By("Checking if there are VMs assigned to this category", func() {
269-
expectedCategories := map[string]string{
270-
expectedClusterNameCategoryKey: clusterName,
269+
expectedCategories := map[string][]string{
270+
expectedClusterNameCategoryKey: {clusterName},
271271
}
272272
testHelper.verifyCategoriesNutanixMachines(ctx, clusterName, namespace2.Name, expectedCategories)
273273
})

‎test/e2e/data/infrastructure-nutanix/v1beta1/cluster-template-additional-categories/nmt.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,6 @@ spec:
1414
# Use System category Environment:Dev
1515
- key: Environment
1616
value: Dev
17+
# Use System category Environment:Testing
18+
- key: Environment
19+
value: Testing

‎test/e2e/test_helpers.go‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ type testHelperInterface interface {
149149
updateVariableInE2eConfig(variableKey string, variableValue string)
150150
stripNutanixIDFromProviderID(providerID string) string
151151
verifyCategoryExists(ctx context.Context, categoryKey, categoyValue string)
152-
verifyCategoriesNutanixMachines(ctx context.Context, clusterName, namespace string, expectedCategories map[string]string)
152+
verifyCategoriesNutanixMachines(ctx context.Context, clusterName, namespace string, expectedCategories map[string][]string)
153153
verifyConditionOnNutanixCluster(params verifyConditionParams)
154154
verifyConditionOnNutanixMachines(params verifyConditionParams)
155155
verifyFailureDomainsOnClusterMachines(ctx context.Context, params verifyFailureDomainsOnClusterMachinesParams)
@@ -593,19 +593,30 @@ func (t testHelper) verifyCategoryExists(ctx context.Context, categoryKey, categ
593593
Expect(err).ShouldNot(HaveOccurred())
594594
}
595595

596-
func (t testHelper) verifyCategoriesNutanixMachines(ctx context.Context, clusterName, namespace string, expectedCategories map[string]string) {
597-
nutanixMachines := t.getMachinesForCluster(ctx, clusterName, namespace, bootstrapClusterProxy)
598-
for _, m := range nutanixMachines.Items {
599-
machineProviderID := m.Spec.ProviderID
600-
Expect(machineProviderID).NotTo(BeNil())
601-
machineVmUUID := t.stripNutanixIDFromProviderID(*machineProviderID)
602-
vm, err := t.nutanixClient.V3.GetVM(ctx, machineVmUUID)
603-
Expect(err).ShouldNot(HaveOccurred())
604-
categoriesMeta := vm.Metadata.Categories
605-
for k, v := range expectedCategories {
606-
Expect(categoriesMeta).To(HaveKeyWithValue(k, v))
607-
}
608-
}
596+
func (t testHelper) verifyCategoriesNutanixMachines(ctx context.Context, clusterName, namespace string, expectedCategories map[string][]string) {
597+
Eventually(
598+
func(g Gomega) {
599+
nutanixMachines := t.getMachinesForCluster(ctx, clusterName, namespace, bootstrapClusterProxy)
600+
for _, m := range nutanixMachines.Items {
601+
machineProviderID := m.Spec.ProviderID
602+
g.Expect(machineProviderID).NotTo(BeNil())
603+
machineVmUUID := t.stripNutanixIDFromProviderID(*machineProviderID)
604+
vm, err := t.nutanixClient.V3.GetVM(ctx, machineVmUUID)
605+
g.Expect(err).ShouldNot(HaveOccurred())
606+
categoriesMappingMeta := vm.Metadata.CategoriesMapping
607+
for k, v := range expectedCategories {
608+
g.Expect(categoriesMappingMeta).To(HaveKey(k))
609+
vals := make([]any, len(v))
610+
for i := range v {
611+
vals[i] = v[i]
612+
}
613+
g.Expect(categoriesMappingMeta[k]).To(ConsistOf(vals...))
614+
}
615+
}
616+
},
617+
defaultTimeout,
618+
defaultInterval,
619+
).Should(Succeed())
609620
}
610621

611622
type verifyResourceConfigOnNutanixMachinesParams struct {

0 commit comments

Comments
 (0)