Skip to content

Commit 461e4ac

Browse files
yutpengyutpeng
committed
Add Tags to OpenShift nodes & iSCSI usage
1 parent 78ccfc1 commit 461e4ac

File tree

7 files changed

+63
-52
lines changed

7 files changed

+63
-52
lines changed

‎pkg/cloudprovider/providers/oci/instances.go

Lines changed: 50 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"fmt"
2020
"net"
21+
"strings"
2122

2223
"github.com/oracle/oci-go-sdk/v65/core"
2324
"k8s.io/apimachinery/pkg/labels"
@@ -29,6 +30,12 @@ import (
2930
cloudprovider "k8s.io/cloud-provider"
3031
)
3132

33+
const (
34+
OpenShiftTagNamesapcePrefix = "openshift-"
35+
OpenShiftBootVolumeType = "boot-volume-type"
36+
OpenShiftBootVolumeISCSI = "ISCSI"
37+
)
38+
3239
var _ cloudprovider.Instances = &CloudProvider{}
3340

3441
// mapNodeNameToInstanceName maps a kube NodeName to a OCI instance display
@@ -97,33 +104,38 @@ func (cp *CloudProvider) extractNodeAddresses(ctx context.Context, instanceID st
97104
addresses = append(addresses, api.NodeAddress{Type: api.NodeExternalIP, Address: ip.String()})
98105
}
99106

100-
useSecondaryVnic, err := cp.checkOpenShiftNodesSecondaryVnicByInstance(instanceID)
101-
if useSecondaryVnic {
102-
secondaryVnic, err := cp.client.Compute().GetSecondaryVNICForInstance(ctx, compartmentID, instanceID)
107+
OpenShiftTagNamesapce := cp.getOpenShiftTagNamespaceByInstance(ctx, instanceID)
108+
109+
if OpenShiftTagNamesapce != "" {
110+
secondaryVnics, err := cp.client.Compute().GetSecondaryVNICsForInstance(ctx, compartmentID, instanceID)
103111
if err != nil {
104-
return nil, errors.Wrap(err, "GetSecondaryVNICForInstance")
112+
return nil, err
105113
}
106114

107-
if secondaryVnic == nil {
115+
if secondaryVnics == nil || len(secondaryVnics) == 0 {
108116
return addresses, nil
109117
}
118+
for _, secondaryVnic := range secondaryVnics {
119+
if cp.checkOpenShiftISCSIBootVolumeTagByVnic(ctx, secondaryVnic, OpenShiftTagNamesapce) {
120+
if (secondaryVnic.IsPrimary == nil || !*secondaryVnic.IsPrimary) && secondaryVnic.PrivateIp != nil && *secondaryVnic.PrivateIp != "" {
121+
ip := net.ParseIP(*secondaryVnic.PrivateIp)
122+
if ip == nil {
123+
return nil, fmt.Errorf("instance has invalid private address: %q", *secondaryVnic.PrivateIp)
124+
}
125+
addresses = append(addresses, api.NodeAddress{Type: api.NodeInternalIP, Address: ip.String()})
126+
}
110127

111-
if (secondaryVnic.IsPrimary == nil || !*secondaryVnic.IsPrimary) && secondaryVnic.PrivateIp != nil && *secondaryVnic.PrivateIp != "" {
112-
ip := net.ParseIP(*secondaryVnic.PrivateIp)
113-
if ip == nil {
114-
return nil, fmt.Errorf("instance has invalid private address: %q", *secondaryVnic.PrivateIp)
115-
}
116-
addresses = append(addresses, api.NodeAddress{Type: api.NodeInternalIP, Address: ip.String()})
117-
}
118-
119-
if (secondaryVnic.IsPrimary == nil || !*secondaryVnic.IsPrimary) && secondaryVnic.PublicIp != nil && *secondaryVnic.PublicIp != "" {
120-
ip := net.ParseIP(*secondaryVnic.PublicIp)
121-
if ip == nil {
122-
return nil, errors.Errorf("instance has invalid public address: %q", *secondaryVnic.PublicIp)
128+
if (secondaryVnic.IsPrimary == nil || !*secondaryVnic.IsPrimary) && secondaryVnic.PublicIp != nil && *secondaryVnic.PublicIp != "" {
129+
ip := net.ParseIP(*secondaryVnic.PublicIp)
130+
if ip == nil {
131+
return nil, errors.Errorf("instance has invalid public address: %q", *secondaryVnic.PublicIp)
132+
}
133+
addresses = append(addresses, api.NodeAddress{Type: api.NodeExternalIP, Address: ip.String()})
134+
}
123135
}
124-
addresses = append(addresses, api.NodeAddress{Type: api.NodeExternalIP, Address: ip.String()})
125136
}
126137
}
138+
127139
// Changing this can have wide reaching impact.
128140
//
129141
// if vnic.HostnameLabel != nil && *vnic.HostnameLabel != "" {
@@ -340,32 +352,32 @@ func (cp *CloudProvider) getCompartmentIDByNodeName(nodeName string) (string, er
340352
return "", errors.New("compartmentID annotation missing in the node. Would retry")
341353
}
342354

343-
func (cp *CloudProvider) checkOpenShiftNodesSecondaryVnicByInstance(instanceID string) (bool, error) {
344-
var SecondaryVnicUsageInstances = []string{"BM.Standard3.64"}
345-
nodeList, err := cp.NodeLister.List(labels.Everything())
355+
func (cp *CloudProvider) getOpenShiftTagNamespaceByInstance(ctx context.Context, instanceID string) string {
356+
instance, err := cp.client.Compute().GetInstance(ctx, instanceID)
346357
if err != nil {
347-
return false, errors.Wrap(err, "error listing all the nodes using node informer")
358+
return ""
348359
}
349-
for _, node := range nodeList {
350-
providerID, err := MapProviderIDToInstanceID(node.Spec.ProviderID)
351-
if err != nil {
352-
return false, errors.New("Failed to map providerID to instanceID.")
353-
}
354-
if providerID == instanceID {
355-
if _, ok := node.Labels[OpenShiftNodeIdentifierLabel]; ok {
356-
if instanceType, ok := node.Labels[api.LabelInstanceTypeStable]; ok && contains(SecondaryVnicUsageInstances, instanceType) {
357-
return true, nil
358-
}
359-
}
360+
361+
if instance.DefinedTags == nil {
362+
return ""
363+
}
364+
365+
for namespace := range instance.DefinedTags {
366+
if strings.HasPrefix(namespace, OpenShiftTagNamesapcePrefix) {
367+
return namespace
360368
}
361369
}
362-
return false, errors.New("Failed to check OpenShift node using node lables. Returning false")
370+
return ""
363371
}
364372

365-
// contains is a utility method to check if a string is part of a slice
366-
func contains(s []string, e string) bool {
367-
for _, a := range s {
368-
if a == e {
373+
func (cp *CloudProvider) checkOpenShiftISCSIBootVolumeTagByVnic(ctx context.Context, vnic *core.Vnic, namespace string) bool {
374+
if vnic.DefinedTags == nil {
375+
return false
376+
}
377+
378+
if tags, namespaceExists := vnic.DefinedTags[namespace]; namespaceExists {
379+
// Check if the boot volume type key exists and its value is ISCSI
380+
if bootVolume, keyExists := tags[OpenShiftBootVolumeType]; keyExists && bootVolume == OpenShiftBootVolumeISCSI {
369381
return true
370382
}
371383
}

‎pkg/cloudprovider/providers/oci/instances_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,8 @@ func (MockComputeClient) GetPrimaryVNICForInstance(ctx context.Context, compartm
344344
return instanceVnics[instanceID], nil
345345
}
346346

347-
func (c *MockComputeClient) GetSecondaryVNICForInstance(ctx context.Context, compartmentID, instanceID string) (*core.Vnic, error) {
348-
return instanceVnics[instanceID], nil
347+
func (c *MockComputeClient) GetSecondaryVNICsForInstance(ctx context.Context, compartmentID, instanceID string) ([]*core.Vnic, error) {
348+
return []*core.Vnic{instanceVnics[instanceID]}, nil
349349
}
350350

351351
func (MockComputeClient) FindVolumeAttachment(ctx context.Context, compartmentID, volumeID string) (core.VolumeAttachment, error) {

‎pkg/cloudprovider/providers/oci/node_info_controller.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,9 @@ import (
4141

4242
// metadata labeling for placement info
4343
const (
44-
FaultDomainLabel = "oci.oraclecloud.com/fault-domain"
45-
CompartmentIDAnnotation = "oci.oraclecloud.com/compartment-id"
46-
OpenShiftNodeIdentifierLabel = "node.openshift.io/os_id"
47-
timeout = 10 * time.Second
44+
FaultDomainLabel = "oci.oraclecloud.com/fault-domain"
45+
CompartmentIDAnnotation = "oci.oraclecloud.com/compartment-id"
46+
timeout = 10 * time.Second
4847
)
4948

5049
// NodeInfoController helps compute workers in the cluster

‎pkg/csi/driver/bv_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ func (c *MockComputeClient) GetPrimaryVNICForInstance(ctx context.Context, compa
568568
return nil, nil
569569
}
570570

571-
func (c *MockComputeClient) GetSecondaryVNICForInstance(ctx context.Context, compartmentID, instanceID string) (*core.Vnic, error) {
571+
func (c *MockComputeClient) GetSecondaryVNICsForInstance(ctx context.Context, compartmentID, instanceID string) ([]*core.Vnic, error) {
572572
return nil, nil
573573
}
574574

‎pkg/oci/client/compute.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type ComputeInterface interface {
3535

3636
GetPrimaryVNICForInstance(ctx context.Context, compartmentID, instanceID string) (*core.Vnic, error)
3737

38-
GetSecondaryVNICForInstance(ctx context.Context, compartmentID, instanceID string) (*core.Vnic, error)
38+
GetSecondaryVNICsForInstance(ctx context.Context, compartmentID, instanceID string) ([]*core.Vnic, error)
3939

4040
VolumeAttachmentInterface
4141
}
@@ -185,9 +185,9 @@ func (c *client) GetPrimaryVNICForInstance(ctx context.Context, compartmentID, i
185185
return nil, errors.WithStack(errNotFound)
186186
}
187187

188-
func (c *client) GetSecondaryVNICForInstance(ctx context.Context, compartmentID, instanceID string) (*core.Vnic, error) {
188+
func (c *client) GetSecondaryVNICsForInstance(ctx context.Context, compartmentID, instanceID string) ([]*core.Vnic, error) {
189189
logger := c.logger.With("instanceID", instanceID, "compartmentID", compartmentID)
190-
190+
secondaryVnics := []*core.Vnic{}
191191
var page *string
192192
for {
193193
resp, err := c.listVNICAttachments(ctx, core.ListVnicAttachmentsRequest{
@@ -222,7 +222,7 @@ func (c *client) GetSecondaryVNICForInstance(ctx context.Context, compartmentID,
222222
}
223223

224224
if !*vnic.IsPrimary {
225-
return vnic, nil
225+
secondaryVnics = append(secondaryVnics, vnic)
226226
}
227227
}
228228

@@ -231,7 +231,7 @@ func (c *client) GetSecondaryVNICForInstance(ctx context.Context, compartmentID,
231231
}
232232
}
233233

234-
return nil, errors.WithStack(errNotFound)
234+
return secondaryVnics, nil
235235
}
236236

237237
func (c *client) GetInstanceByNodeName(ctx context.Context, compartmentID, vcnID, nodeName string) (*core.Instance, error) {

‎pkg/volume/provisioner/block/block_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ func (c *MockComputeClient) GetPrimaryVNICForInstance(ctx context.Context, compa
246246
return nil, nil
247247
}
248248

249-
func (c *MockComputeClient) GetSecondaryVNICForInstance(ctx context.Context, compartmentID, instanceID string) (*core.Vnic, error) {
249+
func (c *MockComputeClient) GetSecondaryVNICsForInstance(ctx context.Context, compartmentID, instanceID string) ([]*core.Vnic, error) {
250250
return nil, nil
251251
}
252252

‎pkg/volume/provisioner/fss/fss_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ func (c *MockComputeClient) GetPrimaryVNICForInstance(ctx context.Context, compa
244244
return nil, nil
245245
}
246246

247-
func (c *MockComputeClient) GetSecondaryVNICForInstance(ctx context.Context, compartmentID, instanceID string) (*core.Vnic, error) {
247+
func (c *MockComputeClient) GetSecondaryVNICsForInstance(ctx context.Context, compartmentID, instanceID string) ([]*core.Vnic, error) {
248248
return nil, nil
249249
}
250250

0 commit comments

Comments
 (0)