Skip to content

Commit 84197db

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1204 from rvanderp3/SPLAT-1386
SPLAT-1386: reconcile additional tags assigned to machine
2 parents 71a25fd + efc18e8 commit 84197db

File tree

8 files changed

+238
-47
lines changed

8 files changed

+238
-47
lines changed

‎Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FROM registry.ci.openshift.org/openshift/release:golang-1.19 AS builder
1+
FROM registry.ci.openshift.org/openshift/release:golang-1.21 AS builder
22
WORKDIR /go/src/github.com/openshift/machine-api-operator
33
COPY . .
44
RUN NO_DOCKER=1 make build

‎docs/user/vsphere/tags.md

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
Administrators may wish to attach additional tags to newly provisioned VMs. The cluster API provider for vSphere
2+
allows for this in the [`VirtualMachineCloneSpec`](https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/b9b2c22ea68c13cbf706f2116ced804b5afb124e/apis/v1beta1/types.go#L189-L193).
3+
A similar provision has been made in the [`VSphereMachineProviderSpec`](https://github.com/openshift/api/blob/6d48d55c0598ec78adacdd847dcf934035ec2e1b/machine/v1beta1/types_vsphereprovider.go#L54-L59).
4+
5+
Up to 10 tags are attachable by their tag URN(for example: `urn:vmomi:InventoryServiceTag:f6dfddfd-b28a-44da-9503-635d3fc245ac:GLOBAL`), not by the name of the tag and tag category. As a result, a given tag must exist before the
6+
machine controller will attempt to associate the tag. The URN can be retrieved from the vCenter UI by accessing the `Tag & Custom Attributes` screen and selecting the desired tag. The tag URN will be in the URL:
7+
```
8+
https://v8c-2-vcenter.ocp2.dev.cluster.com/ui/app/tags/tag/urn:vmomi:InventoryServiceTag:f6dfddfd-b28a-44da-9503-635d3fc245ac:GLOBAL/permissions
9+
```
10+
11+
The below example demonstrates a machine spec which defines `tagIDs`:
12+
13+
```yaml
14+
apiVersion: machine.openshift.io/v1beta1
15+
kind: Machine
16+
metadata:
17+
generateName: ci-ln-ll0lbgk-c1627-gslcw-worker-0-
18+
name: ci-ln-ll0lbgk-c1627-gslcw-worker-0-6nppd
19+
spec:
20+
lifecycleHooks: {}
21+
metadata: {}
22+
providerID: 'vsphere://421bb8b8-e7da-6bfa-3e1b-cc6ef9945343'
23+
providerSpec:
24+
value:
25+
numCoresPerSocket: 4
26+
diskGiB: 120
27+
snapshot: ''
28+
userDataSecret:
29+
name: worker-user-data
30+
memoryMiB: 16384
31+
credentialsSecret:
32+
name: vsphere-cloud-credentials
33+
network:
34+
devices:
35+
- networkName: ci-vlan-1207
36+
metadata:
37+
creationTimestamp: null
38+
numCPUs: 4
39+
kind: VSphereMachineProviderSpec
40+
workspace:
41+
datacenter: IBMCloud
42+
datastore: /IBMCloud/datastore/vsanDatastore
43+
folder: /IBMCloud/vm/ci-ln-ll0lbgk-c1627-gslcw
44+
resourcePool: /IBMCloud/host/vcs-ci-workload/Resources/ipi-ci-clusters
45+
server: v8c-2-vcenter.ocp2.dev.cluster.com
46+
template: ci-ln-ll0lbgk-c1627-gslcw-rhcos-generated-region-generated-zone
47+
tagIDs:
48+
- 'urn:vmomi:InventoryServiceTag:8d893b87-28de-4ef0-90d2-af24f21b0f26:GLOBAL'
49+
apiVersion: machine.openshift.io/v1beta1
50+
status: {}
51+
```
52+
53+
54+
`machinesets` may also be defined to attach tags to `machines` created by the `machineset`.

‎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

0 commit comments

Comments
 (0)