Skip to content

Commit c29cd93

Browse files
rvanderp3rvanderp3
committed
reconcile a slice of additional tag IDs
1 parent 1685a44 commit c29cd93

File tree

6 files changed

+183
-46
lines changed

6 files changed

+183
-46
lines changed

‎pkg/controller/vsphere/actuator_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,8 @@ func TestMachineEvents(t *testing.T) {
180180
g.Expect(k8sClient.Delete(context.Background(), userDataSecret)).To(Succeed())
181181
}()
182182

183-
g.Expect(createTagAndCategory(session, tagToCategoryName("CLUSTERID"), "CLUSTERID")).To(Succeed())
183+
_, err = createTagAndCategory(session, tagToCategoryName("CLUSTERID"), "CLUSTERID")
184+
g.Expect(err).ToNot(HaveOccurred())
184185

185186
ctx := context.Background()
186187

‎pkg/controller/vsphere/reconciler.go

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ func (r *Reconciler) update() error {
250250
Ref: vmRef,
251251
}
252252

253-
if err := vm.reconcileTags(r.Context, r.session, r.machine); err != nil {
253+
if err := vm.reconcileTags(r.Context, r.session, r.machine, r.providerSpec); err != nil {
254254
metrics.RegisterFailedInstanceUpdate(&metrics.MachineLabels{
255255
Name: r.machine.Name,
256256
Namespace: r.machine.Namespace,
@@ -1314,25 +1314,28 @@ func (vm *virtualMachine) getPowerState() (types.VirtualMachinePowerState, error
13141314

13151315
// reconcileTags ensures that the required tags are present on the virtual machine, eg the Cluster ID
13161316
// that is used by the installer on cluster deletion to ensure ther are no leaked resources.
1317-
func (vm *virtualMachine) reconcileTags(ctx context.Context, sessionInstance *session.Session, machine *machinev1.Machine) error {
1317+
func (vm *virtualMachine) reconcileTags(ctx context.Context, sessionInstance *session.Session, machine *machinev1.Machine, providerSpec *machinev1.VSphereMachineProviderSpec) error {
13181318
if err := sessionInstance.WithCachingTagsManager(vm.Context, func(c *session.CachingTagsManager) error {
13191319
klog.Infof("%v: Reconciling attached tags", machine.GetName())
13201320

13211321
clusterID := machine.Labels[machinev1.MachineClusterIDLabel]
1322-
1323-
attached, err := vm.checkAttachedTag(ctx, clusterID, c)
1324-
if err != nil {
1325-
return err
1326-
}
1327-
1328-
if !attached {
1329-
klog.Infof("%v: Attaching %s tag to vm", machine.GetName(), clusterID)
1330-
// the tag should already be created by installer
1331-
if err := c.AttachTag(ctx, clusterID, vm.Ref); err != nil {
1322+
tagIDs := []string{clusterID}
1323+
tagIDs = append(tagIDs, providerSpec.TagIDs...)
1324+
klog.Infof("%v: Reconciling %s tags to vm", machine.GetName(), tagIDs)
1325+
for _, tagID := range tagIDs {
1326+
attached, err := vm.checkAttachedTag(ctx, tagID, c)
1327+
if err != nil {
13321328
return err
13331329
}
1334-
}
13351330

1331+
if !attached {
1332+
klog.Infof("%v: Attaching %s tag to vm", machine.GetName(), tagID)
1333+
// the tag should already be created by installer or the administrator
1334+
if err := c.AttachTag(ctx, tagID, vm.Ref); err != nil {
1335+
return err
1336+
}
1337+
}
1338+
}
13361339
return nil
13371340
}); err != nil {
13381341
return err
@@ -1359,9 +1362,16 @@ func (vm *virtualMachine) checkAttachedTag(ctx context.Context, tagName string,
13591362
}
13601363

13611364
for _, tag := range tags {
1362-
if tag.Name == tagName {
1363-
return true, nil
1365+
if session.IsName(tagName) {
1366+
if tag.Name == tagName {
1367+
return true, nil
1368+
}
1369+
} else {
1370+
if tag.ID == tagName {
1371+
return true, nil
1372+
}
13641373
}
1374+
13651375
}
13661376

13671377
return false, nil
@@ -1377,22 +1387,34 @@ func tagToCategoryName(tagName string) string {
13771387
}
13781388

13791389
func (vm *virtualMachine) foundTag(ctx context.Context, tagName string, m *session.CachingTagsManager) (bool, error) {
1380-
tags, err := m.ListTagsForCategory(ctx, tagToCategoryName(tagName))
1381-
if err != nil {
1382-
if isNotFoundErr(err) {
1383-
return false, nil
1390+
var tags []string
1391+
var err error
1392+
1393+
if session.IsName(tagName) {
1394+
tags, err = m.ListTagsForCategory(ctx, tagToCategoryName(tagName))
1395+
if err != nil {
1396+
if isNotFoundErr(err) {
1397+
return false, nil
1398+
}
1399+
return false, err
13841400
}
1385-
return false, err
1401+
} else {
1402+
tags = []string{tagName}
13861403
}
1387-
1404+
klog.V(4).Infof("validating the presence of tags: %+v", tags)
13881405
for _, id := range tags {
13891406
tag, err := m.GetTag(ctx, id)
13901407
if err != nil {
13911408
return false, err
13921409
}
1393-
1394-
if tag.Name == tagName {
1395-
return true, nil
1410+
if session.IsName(tagName) {
1411+
if tag.Name == tagName {
1412+
return true, nil
1413+
}
1414+
} else {
1415+
if tag.ID == tagName {
1416+
return true, nil
1417+
}
13961418
}
13971419
}
13981420

‎pkg/controller/vsphere/reconciler_test.go

Lines changed: 62 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,7 +1367,7 @@ func TestReconcileTags(t *testing.T) {
13671367
name string
13681368
expectedError bool
13691369
attachTag bool
1370-
testCondition func(tagName string) error
1370+
testCondition func(tagName string) (string, error)
13711371
tagName string
13721372
}{
13731373
{
@@ -1380,35 +1380,59 @@ func TestReconcileTags(t *testing.T) {
13801380
expectedError: false,
13811381
attachTag: true,
13821382
tagName: "BAAAAAAR",
1383-
testCondition: func(tagName string) error {
1384-
return createTagAndCategory(sessionObj, tagToCategoryName(tagName), tagName)
1383+
testCondition: func(tagName string) (string, error) {
1384+
_, err := createTagAndCategory(sessionObj, tagToCategoryName(tagName), tagName)
1385+
return "", err
1386+
},
1387+
},
1388+
{
1389+
name: "Successfully attach additional tags",
1390+
expectedError: false,
1391+
attachTag: true,
1392+
tagName: "BAAAAAAR2",
1393+
testCondition: func(tagName string) (string, error) {
1394+
if _, err := createTagAndCategory(sessionObj, tagToCategoryName(tagName), tagName); err != nil {
1395+
return "", err
1396+
}
1397+
1398+
additionalTag := "test-tag"
1399+
if additionalTagID, err := createTagAndCategory(sessionObj, tagToCategoryName(additionalTag), additionalTag); err != nil {
1400+
return "", err
1401+
} else {
1402+
return additionalTagID, nil
1403+
}
1404+
13851405
},
13861406
},
13871407
{
13881408
name: "Fail on vSphere API error",
13891409
expectedError: false,
13901410
tagName: "BAAAAAZ",
1391-
testCondition: func(tagName string) error {
1392-
return sessionObj.Logout(context.Background())
1411+
testCondition: func(tagName string) (string, error) {
1412+
return "", sessionObj.Logout(context.Background())
13931413
},
13941414
},
13951415
}
13961416

13971417
for _, tc := range testCases {
13981418
t.Run(tc.name, func(t *testing.T) {
1419+
providerSpec := &machinev1.VSphereMachineProviderSpec{}
13991420
if tc.testCondition != nil {
1400-
err := tc.testCondition(tc.tagName)
1421+
additionalTagID, err := tc.testCondition(tc.tagName)
14011422
if err != nil {
14021423
t.Fatalf("Not expected error %v", err)
14031424
}
1425+
if len(additionalTagID) > 0 {
1426+
providerSpec.TagIDs = []string{additionalTagID}
1427+
}
14041428
}
14051429

14061430
err := vm.reconcileTags(context.TODO(), sessionObj, &machinev1.Machine{
14071431
ObjectMeta: metav1.ObjectMeta{
14081432
Name: "machine",
14091433
Labels: map[string]string{machinev1.MachineClusterIDLabel: tc.tagName},
14101434
},
1411-
})
1435+
}, providerSpec)
14121436

14131437
if tc.expectedError {
14141438
if err == nil {
@@ -1431,8 +1455,29 @@ func TestReconcileTags(t *testing.T) {
14311455
t.Fatalf("Expected tags to be found")
14321456
}
14331457

1434-
if tags[0].Name != tc.tagName {
1435-
t.Fatalf("Expected tag %s, got %s", tc.tagName, tags[0].Name)
1458+
expectedTags := []string{tc.tagName}
1459+
if len(providerSpec.TagIDs) > 0 {
1460+
expectedTags = append(expectedTags, providerSpec.TagIDs...)
1461+
}
1462+
1463+
for _, expectedTag := range expectedTags {
1464+
gotTag := false
1465+
for _, attachedTag := range tags {
1466+
if session.IsName(expectedTag) {
1467+
if attachedTag.Name == expectedTag {
1468+
gotTag = true
1469+
break
1470+
}
1471+
} else {
1472+
if attachedTag.ID == expectedTag {
1473+
gotTag = true
1474+
break
1475+
}
1476+
}
1477+
}
1478+
if !gotTag {
1479+
t.Fatalf("Expected tag %s to be found", expectedTag)
1480+
}
14361481
}
14371482

14381483
return nil
@@ -2673,7 +2718,7 @@ func TestUpdate(t *testing.T) {
26732718
},
26742719
}
26752720

2676-
if err := createTagAndCategory(session, tagToCategoryName("CLUSTERID"), "CLUSTERID"); err != nil {
2721+
if _, err := createTagAndCategory(session, tagToCategoryName("CLUSTERID"), "CLUSTERID"); err != nil {
26772722
t.Fatalf("cannot create tag and category: %v", err)
26782723
}
26792724

@@ -2919,11 +2964,11 @@ func TestReconcileMachineWithCloudState(t *testing.T) {
29192964
cluster := simulator.Map.Any("ClusterComputeResource").(*simulator.ClusterComputeResource)
29202965
dc := simulator.Map.Any("Datacenter").(*simulator.Datacenter)
29212966

2922-
if err := createTagAndCategory(session, zoneKey, testZone); err != nil {
2967+
if _, err := createTagAndCategory(session, zoneKey, testZone); err != nil {
29232968
t.Fatalf("cannot create tag and category: %v", err)
29242969
}
29252970

2926-
if err := createTagAndCategory(session, regionKey, testRegion); err != nil {
2971+
if _, err := createTagAndCategory(session, regionKey, testRegion); err != nil {
29272972
t.Fatalf("cannot create tag and category: %v", err)
29282973
}
29292974

@@ -3017,7 +3062,8 @@ func TestReconcileMachineWithCloudState(t *testing.T) {
30173062
}
30183063
}
30193064

3020-
func createTagAndCategory(session *session.Session, categoryName, tagName string) error {
3065+
func createTagAndCategory(session *session.Session, categoryName, tagName string) (string, error) {
3066+
var tagID string
30213067
if err := session.WithRestClient(context.TODO(), func(c *rest.Client) error {
30223068
tagsMgr := tags.NewManager(c)
30233069

@@ -3030,7 +3076,7 @@ func createTagAndCategory(session *session.Session, categoryName, tagName string
30303076
return err
30313077
}
30323078

3033-
_, err = tagsMgr.CreateTag(context.TODO(), &tags.Tag{
3079+
tagID, err = tagsMgr.CreateTag(context.TODO(), &tags.Tag{
30343080
CategoryID: id,
30353081
Name: tagName,
30363082
})
@@ -3040,10 +3086,10 @@ func createTagAndCategory(session *session.Session, categoryName, tagName string
30403086

30413087
return nil
30423088
}); err != nil {
3043-
return err
3089+
return "", err
30443090
}
30453091

3046-
return nil
3092+
return tagID, nil
30473093
}
30483094

30493095
func TestVmDisksManipulation(t *testing.T) {

‎pkg/controller/vsphere/session/tag_ids_caching_client.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,11 @@ func newTagsCachingClient(tagsManager *tags.Manager, sessionKey string) *Caching
107107
}
108108
}
109109

110-
// isName returns true if the id is not an urn.
110+
// IsName returns true if the id is not an urn.
111111
// this method came from vSphere sdk,
112112
// see https://github.com/vmware/govmomi/blob/a2fb82dc55a8eb00932233aa8028ce97140df784/vapi/tags/tags.go#L121 for
113113
// more context.
114-
func isName(id string) bool {
114+
func IsName(id string) bool {
115115
return !strings.HasPrefix(id, "urn:")
116116
}
117117

@@ -128,7 +128,7 @@ func isObjectNotFoundErr(err error) bool {
128128
//
129129
// In case if a tag was not found in vCenter, this would be cached for 12 hours and lookup won't happen till cache expiration.
130130
func (t *CachingTagsManager) GetTag(ctx context.Context, id string) (*tags.Tag, error) {
131-
if !isName(id) { // if id is passed no cache check needed, use original GetTag method instantly
131+
if !IsName(id) { // if id is passed no cache check needed, use original GetTag method instantly
132132
return t.Manager.GetTag(ctx, id)
133133
}
134134

@@ -181,7 +181,7 @@ func (t *CachingTagsManager) GetTag(ctx context.Context, id string) (*tags.Tag,
181181
//
182182
// In case if a tag was not found in vCenter, this would be cached for 12 hours and lookup won't happen till cache expiration.
183183
func (t *CachingTagsManager) GetCategory(ctx context.Context, id string) (*tags.Category, error) {
184-
if !isName(id) { // if id is passed no cache check needed, use original GetTag method instantly
184+
if !IsName(id) { // if id is passed no cache check needed, use original GetTag method instantly
185185
return t.Manager.GetCategory(ctx, id)
186186
}
187187

@@ -230,7 +230,7 @@ func (t *CachingTagsManager) GetCategory(ctx context.Context, id string) (*tags.
230230
// The id parameter can be a Category ID or Category Name.
231231
// Uses caching GetCategory method.
232232
func (t *CachingTagsManager) ListTagsForCategory(ctx context.Context, id string) ([]string, error) {
233-
if isName(id) {
233+
if IsName(id) {
234234
category, err := t.GetCategory(ctx, id)
235235
if err != nil {
236236
return nil, err

‎pkg/webhooks/machine_webhook.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ var (
118118
maxProcessor: 143,
119119
},
120120
}
121+
122+
// VSphere variables
123+
124+
// tagUrnPattern is helps validate the format of a given tag URN
125+
tagUrnPattern = regexp.MustCompile(`^(urn):(vmomi):(InventoryServiceTag):([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}):([^:]+)$`)
121126
)
122127

123128
const (
@@ -1418,6 +1423,16 @@ func validateVSphere(m *machinev1beta1.Machine, config *admissionConfig) (bool,
14181423
}
14191424
}
14201425

1426+
if len(providerSpec.TagIDs) > 10 {
1427+
errs = append(errs, field.Invalid(field.NewPath("providerSpec", "tagIDs"), providerSpec.TagIDs, "a maximum of 10 tags are allowed"))
1428+
}
1429+
1430+
for _, tagID := range providerSpec.TagIDs {
1431+
if tagUrnPattern.FindStringSubmatch(tagID) == nil {
1432+
errs = append(errs, field.Required(field.NewPath("providerSpec", "tagIDs"), "tag ID must be in the format of urn:vmomi:InventoryServiceTag:<UUID>:GLOBAL"))
1433+
}
1434+
}
1435+
14211436
if providerSpec.CredentialsSecret == nil {
14221437
errs = append(errs, field.Required(field.NewPath("providerSpec", "credentialsSecret"), "credentialsSecret must be provided"))
14231438
} else {

0 commit comments

Comments
 (0)