Skip to content

Commit 406dbbf

Browse files
abhay-nutanixabhay-nutanix
authored
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
1 parent 880163e commit 406dbbf

File tree

7 files changed

+116
-33
lines changed

7 files changed

+116
-33
lines changed

‎controllers/helpers.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"fmt"
2323
"reflect"
2424
"regexp"
25+
"slices"
2526
"sort"
2627
"strings"
2728
"text/template"
@@ -783,11 +784,19 @@ func getOrCreateCategory(ctx context.Context, client *prismclientv3.Client, cate
783784
return categoryValue, nil
784785
}
785786

786-
// GetCategoryVMSpec returns a flatmap of categories and their values
787-
func GetCategoryVMSpec(ctx context.Context, client *prismclientv3.Client, categoryIdentifiers []*infrav1.NutanixCategoryIdentifier) (map[string]string, error) {
787+
// GetCategoryVMSpec returns the categories_mapping supporting multiple values per key.
788+
func GetCategoryVMSpec(
789+
ctx context.Context,
790+
client *prismclientv3.Client,
791+
categoryIdentifiers []*infrav1.NutanixCategoryIdentifier,
792+
) (map[string][]string, error) {
788793
log := ctrl.LoggerFrom(ctx)
789-
categorySpec := map[string]string{}
794+
categorySpec := map[string][]string{}
795+
790796
for _, ci := range categoryIdentifiers {
797+
if ci == nil {
798+
return nil, fmt.Errorf("category identifier cannot be nil")
799+
}
791800
categoryValue, err := getCategoryValue(ctx, client, ci.Key, ci.Value)
792801
if err != nil {
793802
errorMsg := fmt.Errorf("error occurred while to retrieving category value %s in category %s. error: %v", ci.Value, ci.Key, err)
@@ -799,8 +808,11 @@ func GetCategoryVMSpec(ctx context.Context, client *prismclientv3.Client, catego
799808
log.Error(errorMsg, "category value not found")
800809
return nil, errorMsg
801810
}
802-
categorySpec[ci.Key] = ci.Value
811+
if !slices.Contains(categorySpec[ci.Key], ci.Value) {
812+
categorySpec[ci.Key] = append(categorySpec[ci.Key], ci.Value)
813+
}
803814
}
815+
804816
return categorySpec, nil
805817
}
806818

‎controllers/helpers_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,33 @@ func TestListStorageContainers(t *testing.T) {
894894
}
895895
}
896896

897+
func TestGetCategoryVMSpecMapping_MultiValues(t *testing.T) {
898+
t.Run("returns flat map first value and mapping with all values", func(t *testing.T) {
899+
ctrl := gomock.NewController(t)
900+
defer ctrl.Finish()
901+
902+
ctx := context.Background()
903+
mockv3 := mocknutanixv3.NewMockService(ctrl)
904+
client := &prismclientv3.Client{V3: mockv3}
905+
906+
key := "CategoryKey"
907+
v1 := "CategoryValue1"
908+
v2 := "CategoryValue2"
909+
910+
ids := []*infrav1.NutanixCategoryIdentifier{{Key: key, Value: v1}, {Key: key, Value: v2}, {Key: key, Value: v1}}
911+
912+
// Expect lookups for both values to succeed
913+
mockv3.EXPECT().GetCategoryValue(ctx, key, v1).Return(&prismclientv3.CategoryValueStatus{Value: &v1}, nil)
914+
mockv3.EXPECT().GetCategoryValue(ctx, key, v2).Return(&prismclientv3.CategoryValueStatus{Value: &v2}, nil)
915+
mockv3.EXPECT().GetCategoryValue(ctx, key, v1).Return(&prismclientv3.CategoryValueStatus{Value: &v1}, nil)
916+
917+
mapping, err := GetCategoryVMSpec(ctx, client, ids)
918+
require.NoError(t, err)
919+
assert.Len(t, mapping[key], 2)
920+
assert.ElementsMatch(t, []string{v1, v2}, mapping[key])
921+
})
922+
}
923+
897924
func TestGetStorageContainerByNtnxResourceIdentifier(t *testing.T) {
898925
mockctl := gomock.NewController(t)
899926

‎controllers/nutanixmachine_controller.go‎

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

830-
// Set Categories to VM Sepc before creating VM
831-
categories, err := GetCategoryVMSpec(ctx, v3Client, r.getMachineCategoryIdentifiers(rctx))
830+
// Set categories on VM; support multiple values via categories_mapping when possible
831+
categoriesMapping, err := GetCategoryVMSpec(ctx, v3Client, r.getMachineCategoryIdentifiers(rctx))
832832
if err != nil {
833833
errorMsg := fmt.Errorf("error occurred while creating category spec for vm %s: %v", vmName, err)
834834
rctx.SetFailureStatus(createErrorFailureReason, errorMsg)
835835
return nil, errorMsg
836836
}
837837

838838
vmMetadata := &prismclientv3.Metadata{
839-
Kind: utils.StringPtr("vm"),
840-
SpecVersion: utils.Int64Ptr(1),
841-
Categories: categories,
839+
Kind: utils.StringPtr("vm"),
840+
SpecVersion: utils.Int64Ptr(1),
841+
UseCategoriesMapping: ptr.To(true),
842+
CategoriesMapping: categoriesMapping,
842843
}
843844
// Set Project in VM Spec before creating VM
844845
err = r.addVMToProject(rctx, vmMetadata)

‎controllers/nutanixmachine_controller_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,3 +1119,32 @@ func TestNutanixClusterReconcilerGetDiskList(t *testing.T) {
11191119
})
11201120
}
11211121
}
1122+
1123+
func TestReconcile_VMMetadataCategoriesMapping_MultipleValues(t *testing.T) {
1124+
ctrl := gomock.NewController(t)
1125+
defer ctrl.Finish()
1126+
1127+
ctx := context.Background()
1128+
mockV3 := mocknutanixv3.NewMockService(ctrl)
1129+
client := &prismclientv3.Client{V3: mockV3}
1130+
1131+
// Prepare inputs
1132+
clusterName := "TestCluster"
1133+
1134+
// Default category key/value lookups used by GetCategoryVMSpecMapping
1135+
defaultKey := infrav1.DefaultCAPICategoryKeyForName
1136+
mockV3.EXPECT().GetCategoryValue(ctx, defaultKey, clusterName).Return(&prismclientv3.CategoryValueStatus{Value: &clusterName}, nil)
1137+
mockV3.EXPECT().GetCategoryValue(ctx, "TestCategory", "TestValue1").Return(&prismclientv3.CategoryValueStatus{Value: ptr.To("TestValue1")}, nil)
1138+
mockV3.EXPECT().GetCategoryValue(ctx, "TestCategory", "TestValue2").Return(&prismclientv3.CategoryValueStatus{Value: ptr.To("TestValue2")}, nil)
1139+
mockV3.EXPECT().GetCategoryValue(ctx, "TestCategory", "TestValue1").Return(&prismclientv3.CategoryValueStatus{Value: ptr.To("TestValue1")}, nil)
1140+
1141+
ids := []*infrav1.NutanixCategoryIdentifier{
1142+
{Key: defaultKey, Value: clusterName},
1143+
{Key: "TestCategory", Value: "TestValue1"},
1144+
{Key: "TestCategory", Value: "TestValue2"},
1145+
{Key: "TestCategory", Value: "TestValue1"},
1146+
}
1147+
mapping, err := GetCategoryVMSpec(ctx, client, ids)
1148+
require.NoError(t, err)
1149+
require.ElementsMatch(t, []string{"TestValue1", "TestValue2"}, mapping["TestCategory"])
1150+
}

‎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("nutanix-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("nutanix-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("nutanix-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("nutanix-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
@@ -150,7 +150,7 @@ type testHelperInterface interface {
150150
updateVariableInE2eConfig(variableKey string, variableValue string)
151151
stripNutanixIDFromProviderID(providerID string) string
152152
verifyCategoryExists(ctx context.Context, categoryKey, categoyValue string)
153-
verifyCategoriesNutanixMachines(ctx context.Context, clusterName, namespace string, expectedCategories map[string]string)
153+
verifyCategoriesNutanixMachines(ctx context.Context, clusterName, namespace string, expectedCategories map[string][]string)
154154
verifyConditionOnNutanixCluster(params verifyConditionParams)
155155
verifyConditionOnNutanixMachines(params verifyConditionParams)
156156
verifyDisksOnNutanixMachines(ctx context.Context, params verifyDisksOnNutanixMachinesParams)
@@ -641,19 +641,30 @@ func (t testHelper) verifyCategoryExists(ctx context.Context, categoryKey, categ
641641
Expect(err).ShouldNot(HaveOccurred())
642642
}
643643

644-
func (t testHelper) verifyCategoriesNutanixMachines(ctx context.Context, clusterName, namespace string, expectedCategories map[string]string) {
645-
nutanixMachines := t.getMachinesForCluster(ctx, clusterName, namespace, bootstrapClusterProxy)
646-
for _, m := range nutanixMachines.Items {
647-
machineProviderID := m.Spec.ProviderID
648-
Expect(machineProviderID).NotTo(BeNil())
649-
machineVmUUID := t.stripNutanixIDFromProviderID(*machineProviderID)
650-
vm, err := t.nutanixClient.V3.GetVM(ctx, machineVmUUID)
651-
Expect(err).ShouldNot(HaveOccurred())
652-
categoriesMeta := vm.Metadata.Categories
653-
for k, v := range expectedCategories {
654-
Expect(categoriesMeta).To(HaveKeyWithValue(k, v))
655-
}
656-
}
644+
func (t testHelper) verifyCategoriesNutanixMachines(ctx context.Context, clusterName, namespace string, expectedCategories map[string][]string) {
645+
Eventually(
646+
func(g Gomega) {
647+
nutanixMachines := t.getMachinesForCluster(ctx, clusterName, namespace, bootstrapClusterProxy)
648+
for _, m := range nutanixMachines.Items {
649+
machineProviderID := m.Spec.ProviderID
650+
g.Expect(machineProviderID).NotTo(BeNil())
651+
machineVmUUID := t.stripNutanixIDFromProviderID(*machineProviderID)
652+
vm, err := t.nutanixClient.V3.GetVM(ctx, machineVmUUID)
653+
g.Expect(err).ShouldNot(HaveOccurred())
654+
categoriesMappingMeta := vm.Metadata.CategoriesMapping
655+
for k, v := range expectedCategories {
656+
g.Expect(categoriesMappingMeta).To(HaveKey(k))
657+
vals := make([]any, len(v))
658+
for i := range v {
659+
vals[i] = v[i]
660+
}
661+
g.Expect(categoriesMappingMeta[k]).To(ConsistOf(vals...))
662+
}
663+
}
664+
},
665+
defaultTimeout,
666+
defaultInterval,
667+
).Should(Succeed())
657668
}
658669

659670
type verifyResourceConfigOnNutanixMachinesParams struct {

0 commit comments

Comments
 (0)