Localise caching to power vs controller

cloud/scope/powervs_machine.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
-	"sync"
 
 	"github.com/IBM-Cloud/power-go-client/ibmpisession"
 	"github.com/IBM-Cloud/power-go-client/power/client/p_cloud_p_vm_instances"
@@ -52,9 +51,6 @@ import (
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/record"
 )
 
-// used to initialise the dhcpCacheStore.
-var dhcpIPCacheOnce sync.Once
-
 // PowerVSMachineScopeParams defines the input parameters used to create a new PowerVSMachineScope.
 type PowerVSMachineScopeParams struct {
 	Logger            logr.Logger
@@ -65,6 +61,7 @@ type PowerVSMachineScopeParams struct {
 	IBMPowerVSMachine *infrav1beta1.IBMPowerVSMachine
 	IBMPowerVSImage   *infrav1beta1.IBMPowerVSImage
 	ServiceEndpoint   []endpoints.ServiceEndpoint
+	DHCPIPCacheStore  cache.Store
 }
 
 // PowerVSMachineScope defines a scope defined around a Power VS Machine.
@@ -191,10 +188,7 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac
 		return
 	}
 	scope.IBMPowerVSClient = c
-
-	dhcpIPCacheOnce.Do(func() {
-		scope.DHCPIPCacheStore = cache.NewTTLStore(options.CacheKeyFunc, options.CacheTTL)
-	})
+	scope.DHCPIPCacheStore = params.DHCPIPCacheStore
 	return scope, nil
 }
 
@@ -455,10 +449,10 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 		m.Error(err, "failed to fetch the DHCP IP address from cache store", "VM", *instance.ServerName)
 	}
 	if exists {
-		m.Info("found IP for VM from DHCP cache", "IP", obj.(options.VMip).IP, "VM", *instance.ServerName)
+		m.Info("found IP for VM from DHCP cache", "IP", obj.(powervs.VMip).IP, "VM", *instance.ServerName)
 		addresses = append(addresses, corev1.NodeAddress{
 			Type:    corev1.NodeInternalIP,
-			Address: obj.(options.VMip).IP,
+			Address: obj.(powervs.VMip).IP,
 		})
 		m.IBMPowerVSMachine.Status.Addresses = addresses
 		return
@@ -482,7 +476,7 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 		return
 	}
 	// Get all the DHCP servers
-	dhcpServer, err := m.IBMPowerVSClient.GetDHCPServers()
+	dhcpServer, err := m.IBMPowerVSClient.GetAllDHCPServers()
 	if err != nil {
 		m.Error(err, "failed to get DHCP server")
 		return
@@ -492,7 +486,7 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 	for _, server := range dhcpServer {
 		if *server.Network.ID == *networkID {
 			m.Info("found DHCP server for network", "DHCP server ID", *server.ID, "network ID", *networkID)
-			dhcpServerDetails, err = m.IBMPowerVSClient.GetDHCPServerByID(*server.ID)
+			dhcpServerDetails, err = m.IBMPowerVSClient.GetDHCPServer(*server.ID)
 			if err != nil {
 				m.Error(err, "failed to get DHCP server details", "DHCP server ID", *server.ID)
 				return
@@ -527,7 +521,7 @@ func (m *PowerVSMachineScope) SetAddresses(instance *models.PVMInstance) {
 		Address: *internalIP,
 	})
 	// Update the cache with the ip and VM name
-	err = m.DHCPIPCacheStore.Add(options.VMip{
+	err = m.DHCPIPCacheStore.Add(powervs.VMip{
 		Name: *instance.ServerName,
 		IP:   *internalIP,
 	})

cloud/scope/powervs_machine_test.go

@@ -26,18 +26,19 @@ import (
 	"github.com/IBM/go-sdk-core/v5/core"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2/klogr"
 	"k8s.io/utils/pointer"
-	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options"
 	capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	infrav1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs/mock"
 
 	. "github.com/onsi/ginkgo"
@@ -93,7 +94,7 @@ func setupPowerVSMachineScope(clusterName string, machineName string, imageID *s
 		Machine:           machine,
 		IBMPowerVSCluster: powervsCluster,
 		IBMPowerVSMachine: powervsMachine,
-		DHCPIPCacheStore:  cache.NewTTLStore(options.CacheKeyFunc, options.CacheTTL),
+		DHCPIPCacheStore:  cache.NewTTLStore(powervs.CacheKeyFunc, powervs.CacheTTL),
 	}
 }
 
@@ -408,7 +409,7 @@ func TestSetAddresses(t *testing.T) {
 	}
 
 	defaultDhcpCacheStoreFunc := func() cache.Store {
-		return cache.NewTTLStore(options.CacheKeyFunc, options.CacheTTL)
+		return cache.NewTTLStore(powervs.CacheKeyFunc, powervs.CacheTTL)
 	}
 
 	testCases := []struct {
@@ -534,7 +535,7 @@ func TestSetAddresses(t *testing.T) {
 			testcase: "error while getting DHCP servers",
 			powerVSClientFunc: func(ctrl *gomock.Controller) *mock.MockPowerVS {
 				mockPowerVSClient := mock.NewMockPowerVS(ctrl)
-				mockPowerVSClient.EXPECT().GetDHCPServers().Return(nil, fmt.Errorf("intentional error"))
+				mockPowerVSClient.EXPECT().GetAllDHCPServers().Return(nil, fmt.Errorf("intentional error"))
 				return mockPowerVSClient
 			},
 			pvmInstance:         newPowerVSInstance(instanceName, networkID, instanceMac),
@@ -546,7 +547,7 @@ func TestSetAddresses(t *testing.T) {
 			testcase: "dhcp server details not found associated to network id",
 			powerVSClientFunc: func(ctrl *gomock.Controller) *mock.MockPowerVS {
 				mockPowerVSClient := mock.NewMockPowerVS(ctrl)
-				mockPowerVSClient.EXPECT().GetDHCPServers().Return(newDHCPServer(dhcpServerID, "test-network"), nil)
+				mockPowerVSClient.EXPECT().GetAllDHCPServers().Return(newDHCPServer(dhcpServerID, "test-network"), nil)
 				return mockPowerVSClient
 			},
 			pvmInstance:         newPowerVSInstance(instanceName, networkID, instanceMac),
@@ -558,8 +559,8 @@ func TestSetAddresses(t *testing.T) {
 			testcase: "error on getting DHCP server details",
 			powerVSClientFunc: func(ctrl *gomock.Controller) *mock.MockPowerVS {
 				mockPowerVSClient := mock.NewMockPowerVS(ctrl)
-				mockPowerVSClient.EXPECT().GetDHCPServers().Return(newDHCPServer(dhcpServerID, networkID), nil)
-				mockPowerVSClient.EXPECT().GetDHCPServerByID(dhcpServerID).Return(nil, fmt.Errorf("intentnional error"))
+				mockPowerVSClient.EXPECT().GetAllDHCPServers().Return(newDHCPServer(dhcpServerID, networkID), nil)
+				mockPowerVSClient.EXPECT().GetDHCPServer(dhcpServerID).Return(nil, fmt.Errorf("intentnional error"))
 				return mockPowerVSClient
 			},
 			pvmInstance:         newPowerVSInstance(instanceName, networkID, instanceMac),
@@ -571,8 +572,8 @@ func TestSetAddresses(t *testing.T) {
 			testcase: "dhcp server lease does not have lease for instance",
 			powerVSClientFunc: func(ctrl *gomock.Controller) *mock.MockPowerVS {
 				mockPowerVSClient := mock.NewMockPowerVS(ctrl)
-				mockPowerVSClient.EXPECT().GetDHCPServers().Return(newDHCPServer(dhcpServerID, networkID), nil)
-				mockPowerVSClient.EXPECT().GetDHCPServerByID(dhcpServerID).Return(newDHCPServerDetails(dhcpServerID, leaseIP, "ff:11:33:dd:00:33"), nil)
+				mockPowerVSClient.EXPECT().GetAllDHCPServers().Return(newDHCPServer(dhcpServerID, networkID), nil)
+				mockPowerVSClient.EXPECT().GetDHCPServer(dhcpServerID).Return(newDHCPServerDetails(dhcpServerID, leaseIP, "ff:11:33:dd:00:33"), nil)
 				return mockPowerVSClient
 			},
 			pvmInstance:         newPowerVSInstance(instanceName, networkID, instanceMac),
@@ -584,8 +585,8 @@ func TestSetAddresses(t *testing.T) {
 			testcase: "success in getting ip address from dhcp server",
 			powerVSClientFunc: func(ctrl *gomock.Controller) *mock.MockPowerVS {
 				mockPowerVSClient := mock.NewMockPowerVS(ctrl)
-				mockPowerVSClient.EXPECT().GetDHCPServers().Return(newDHCPServer(dhcpServerID, networkID), nil)
-				mockPowerVSClient.EXPECT().GetDHCPServerByID(dhcpServerID).Return(newDHCPServerDetails(dhcpServerID, leaseIP, instanceMac), nil)
+				mockPowerVSClient.EXPECT().GetAllDHCPServers().Return(newDHCPServer(dhcpServerID, networkID), nil)
+				mockPowerVSClient.EXPECT().GetDHCPServer(dhcpServerID).Return(newDHCPServerDetails(dhcpServerID, leaseIP, instanceMac), nil)
 				return mockPowerVSClient
 			},
 			pvmInstance: newPowerVSInstance(instanceName, networkID, instanceMac),
@@ -600,8 +601,8 @@ func TestSetAddresses(t *testing.T) {
 			testcase: "ip stored in cache expired, fetch from dhcp server",
 			powerVSClientFunc: func(ctrl *gomock.Controller) *mock.MockPowerVS {
 				mockPowerVSClient := mock.NewMockPowerVS(ctrl)
-				mockPowerVSClient.EXPECT().GetDHCPServers().Return(newDHCPServer(dhcpServerID, networkID), nil)
-				mockPowerVSClient.EXPECT().GetDHCPServerByID(dhcpServerID).Return(newDHCPServerDetails(dhcpServerID, leaseIP, instanceMac), nil)
+				mockPowerVSClient.EXPECT().GetAllDHCPServers().Return(newDHCPServer(dhcpServerID, networkID), nil)
+				mockPowerVSClient.EXPECT().GetDHCPServer(dhcpServerID).Return(newDHCPServerDetails(dhcpServerID, leaseIP, instanceMac), nil)
 				return mockPowerVSClient
 			},
 			pvmInstance: newPowerVSInstance(instanceName, networkID, instanceMac),
@@ -610,8 +611,8 @@ func TestSetAddresses(t *testing.T) {
 				Address: leaseIP,
 			}),
 			dhcpCacheStoreFunc: func() cache.Store {
-				cacheStore := cache.NewTTLStore(options.CacheKeyFunc, time.Millisecond)
-				_ = cacheStore.Add(options.VMip{
+				cacheStore := cache.NewTTLStore(powervs.CacheKeyFunc, time.Millisecond)
+				_ = cacheStore.Add(powervs.VMip{
 					Name: instanceName,
 					IP:   "192.168.99.98",
 				})
@@ -632,8 +633,8 @@ func TestSetAddresses(t *testing.T) {
 				Address: leaseIP,
 			}),
 			dhcpCacheStoreFunc: func() cache.Store {
-				cacheStore := cache.NewTTLStore(options.CacheKeyFunc, options.CacheTTL)
-				_ = cacheStore.Add(options.VMip{
+				cacheStore := cache.NewTTLStore(powervs.CacheKeyFunc, powervs.CacheTTL)
+				_ = cacheStore.Add(powervs.VMip{
 					Name: instanceName,
 					IP:   leaseIP,
 				})

controllers/ibmpowervsmachine_controller.go

@@ -25,6 +25,7 @@ import (
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	capierrors "sigs.k8s.io/cluster-api/errors"
@@ -36,8 +37,8 @@ import (
 
 	infrav1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/cloud/scope"
+	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/endpoints"
-	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options"
 	capibmrecord "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/record"
 )
 
@@ -50,6 +51,13 @@ type IBMPowerVSMachineReconciler struct {
 	Scheme          *runtime.Scheme
 }
 
+// dhcpCacheStore is a cache store to hold the Power VS VM DHCP IP.
+var dhcpCacheStore cache.Store
+
+func init() {
+	dhcpCacheStore = powervs.InitialiseDHCPCacheStore()
+}
+
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=ibmpowervsmachines,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=ibmpowervsmachines/status,verbs=get;update;patch
 
@@ -118,6 +126,7 @@ func (r *IBMPowerVSMachineReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		IBMPowerVSMachine: ibmPowerVSMachine,
 		IBMPowerVSImage:   ibmPowerVSImage,
 		ServiceEndpoint:   r.ServiceEndpoint,
+		DHCPIPCacheStore:  dhcpCacheStore,
 	})
 	if err != nil {
 		return ctrl.Result{}, errors.Errorf("failed to create scope: %+v", err)
@@ -165,7 +174,7 @@ func (r *IBMPowerVSMachineReconciler) reconcileDelete(scope *scope.PowerVSMachin
 		return ctrl.Result{}, errors.Wrapf(err, "error deleting IBMPowerVSMachine %s/%s", scope.IBMPowerVSMachine.Namespace, scope.IBMPowerVSMachine.Name)
 	}
 	// Remove the cached VM IP
-	err := scope.DHCPIPCacheStore.Delete(options.VMip{Name: scope.IBMPowerVSMachine.Name})
+	err := scope.DHCPIPCacheStore.Delete(powervs.VMip{Name: scope.IBMPowerVSMachine.Name})
 	if err != nil {
 		scope.Error(err, "failed to delete the VM entry from DHCP cache store", "VM", scope.IBMPowerVSMachine.Name)
 	}

controllers/ibmpowervsmachine_controller_test.go

@@ -32,7 +32,6 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/klog/v2/klogr"
 	"k8s.io/utils/pointer"
-	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options"
 	capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/conditions"
@@ -42,6 +41,7 @@ import (
 
 	infrav1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/cloud/scope"
+	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs/mock"
 
 	. "github.com/onsi/gomega"
@@ -301,7 +301,7 @@ func TestIBMPowerVSMachineReconciler_Delete(t *testing.T) {
 						InstanceID: "powervs-instance-id",
 					},
 				},
-				DHCPIPCacheStore: cache.NewTTLStore(options.CacheKeyFunc, options.CacheTTL),
+				DHCPIPCacheStore: cache.NewTTLStore(powervs.CacheKeyFunc, powervs.CacheTTL),
 			}
 			mockpowervs.EXPECT().DeleteInstance(machineScope.IBMPowerVSMachine.Status.InstanceID).Return(nil)
 			_, err := reconciler.reconcileDelete(machineScope)
@@ -480,7 +480,7 @@ func TestIBMPowerVSMachineReconciler_ReconcileOperations(t *testing.T) {
 					},
 				},
 				IBMPowerVSClient: mockpowervs,
-				DHCPIPCacheStore: cache.NewTTLStore(options.CacheKeyFunc, options.CacheTTL),
+				DHCPIPCacheStore: cache.NewTTLStore(powervs.CacheKeyFunc, powervs.CacheTTL),
 			}
 
 			instanceReferences := &models.PVMInstances{

pkg/options/caching.go → pkg/cloud/services/powervs/caching.go

@@ -14,9 +14,13 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package options
+package powervs
 
-import "time"
+import (
+	"time"
+
+	"k8s.io/client-go/tools/cache"
+)
 
 // CacheTTL is duration of time to store the vm ip in cache
 // Currently the default sync period is 10 minutes that means every 10 minutes
@@ -34,3 +38,8 @@ type VMip struct {
 func CacheKeyFunc(obj interface{}) (string, error) {
 	return obj.(VMip).Name, nil
 }
+
+// InitialiseDHCPCacheStore returns a new cache store.
+func InitialiseDHCPCacheStore() cache.Store {
+	return cache.NewTTLStore(CacheKeyFunc, CacheTTL)
+}

pkg/cloud/services/powervs/powervs.go

@@ -37,6 +37,6 @@ type PowerVS interface {
 	GetCosImages(id string) (*models.Job, error)
 	GetJob(id string) (*models.Job, error)
 	DeleteJob(id string) error
-	GetDHCPServers() (models.DHCPServers, error)
-	GetDHCPServerByID(id string) (*models.DHCPServerDetail, error)
+	GetAllDHCPServers() (models.DHCPServers, error)
+	GetDHCPServer(id string) (*models.DHCPServerDetail, error)
 }

pkg/cloud/services/powervs/service.go

@@ -111,13 +111,13 @@ func (s *Service) GetAllNetwork() (*models.Networks, error) {
 	return s.networkClient.GetAll()
 }
 
-// GetDHCPServers returns all the DHCP servers in the Power VS service instance.
-func (s *Service) GetDHCPServers() (models.DHCPServers, error) {
+// GetAllDHCPServers returns all the DHCP servers in the Power VS service instance.
+func (s *Service) GetAllDHCPServers() (models.DHCPServers, error) {
 	return s.dhcpClient.GetAll()
 }
 
-// GetDHCPServerByID returns the details for DHCP server associated with id.
-func (s *Service) GetDHCPServerByID(id string) (*models.DHCPServerDetail, error) {
+// GetDHCPServer returns the details for DHCP server associated with id.
+func (s *Service) GetDHCPServer(id string) (*models.DHCPServerDetail, error) {
 	return s.dhcpClient.Get(id)
 }