Merge pull request #17 from cjschaef/bz_2042265

Bug 2042265: Fix machine providerID format
openshift · Jan 21, 2022 · 7449a94 · 7449a94
2 parents f2e1a81 + e8fde4f
commit 7449a94
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 6 deletions.
diff --git a/pkg/actuators/client/client.go b/pkg/actuators/client/client.go
@@ -41,6 +41,7 @@ type Client interface {
 	InstanceGetProfile(profileName string) (bool, error)
 
 	// Helper functions
+	GetAccountID() (string, error)
 	GetCustomImageByName(imageName string, resourceGroupID string) (string, error)
 	GetVPCIDByName(vpcName string, resourceGroupID string) (string, error)
 	GetResourceGroupIDByName(resourceGroupName string) (string, error)
@@ -365,6 +366,14 @@ func (c *ibmCloudClient) GetVPCIDByName(vpcName string, resourceGroupID string)
 	return "", fmt.Errorf("could not retrieve vpc id of name: %v", vpcName)
 }
 
+// GetAccountID retrieves the Account ID for the IBMCloud Client
+func (c *ibmCloudClient) GetAccountID() (string, error) {
+	if c.AccountID == "" {
+		return "", fmt.Errorf("could not retrieve account id")
+	}
+	return c.AccountID, nil
+}
+
 // GetCustomImageByName retrieves custom image from VPC by region and name
 func (c *ibmCloudClient) GetCustomImageByName(imageName string, resourceGroupID string) (string, error) {
 	// Initialize List Images Options

diff --git a/pkg/actuators/client/mock/client_mock_generated.go b/pkg/actuators/client/mock/client_mock_generated.go
diff --git a/pkg/actuators/machine/actuator_test.go b/pkg/actuators/machine/actuator_test.go
@@ -305,6 +305,7 @@ func TestActuatorEvents(t *testing.T) {
 			}
 
 			mockIBMClient.EXPECT().InstanceCreate(machine.Name, gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
+			mockIBMClient.EXPECT().GetAccountID().Return("accountID", nil).AnyTimes()
 			mockIBMClient.EXPECT().InstanceGetByName(machine.Name, gomock.Any()).Return(stubInstanceGetByName(machine.Name, &v1.IBMCloudMachineProviderSpec{CredentialsSecret: &corev1.LocalObjectReference{Name: credentialsSecretName}})).AnyTimes()
 			mockIBMClient.EXPECT().InstanceDeleteByName(machine.Name, gomock.Any()).Return(nil).AnyTimes()
 

diff --git a/pkg/actuators/machine/reconciler.go b/pkg/actuators/machine/reconciler.go
@@ -189,7 +189,13 @@ func (r *Reconciler) reconcileMachineWithCloudState(conditionFailed *ibmcloudpro
 	}
 
 	clusterID := r.machine.Labels[machinev1.MachineClusterIDLabel]
-	providerID := fmt.Sprintf("ibmvpc://%s/%s/%s", clusterID, r.providerSpec.Zone, r.machine.GetName())
+	accountID, err := r.ibmClient.GetAccountID()
+	if err != nil {
+		return fmt.Errorf("get account id failed with an error: %q", err)
+	}
+	// Follow same providerID format as the cloud-provider-ibm
+	// https://github.com/openshift/cloud-provider-ibm/blob/e30391202c3f02694b2f5b3c2d73cb560d9c133d/ibm/ibm_instances.go#L113-L114
+	providerID := fmt.Sprintf("ibm://%s///%s/%s", accountID, clusterID, *newInstance.ID)
 	currProviderID := r.machine.Spec.ProviderID
 
 	// Provider ID check and update

diff --git a/pkg/actuators/machine/reconciler_test.go b/pkg/actuators/machine/reconciler_test.go
@@ -133,6 +133,7 @@ func TestCreate(t *testing.T) {
 			}
 
 			mockIBMClient.EXPECT().InstanceCreate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
+			mockIBMClient.EXPECT().GetAccountID().Return("accountID", nil).AnyTimes()
 			mockIBMClient.EXPECT().InstanceGetByName(gomock.Any(), gomock.Any()).Return(stubInstanceGetByName(machine.Name, &ibmcloudproviderv1.IBMCloudMachineProviderSpec{CredentialsSecret: &corev1.LocalObjectReference{Name: credentialsSecretName}})).AnyTimes()
 			mockIBMClient.EXPECT().InstanceDeleteByName(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
 			mockIBMClient.EXPECT().InstanceExistsByName(gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes()
@@ -409,8 +410,11 @@ func TestReconcileMachineWithCloudState(t *testing.T) {
 		},
 	})
 
+	// account ID returned from stubGetAccountID
+	accountID := "01234_5678_90ab_cdef"
+	mockIBMClient.EXPECT().GetAccountID().Return(accountID, nil).AnyTimes()
 	// instance ID returned from stubInstanceGetByName
-	intanceID := "0727_xyz-xyz-cccc-aaba-cacdaccad"
+	instanceID := "0727_xyz-xyz-cccc-aaba-cacdaccad"
 	mockIBMClient.EXPECT().InstanceGetByName(gomock.Any(), gomock.Any()).Return(stubInstanceGetByName(machine.Name, &ibmcloudproviderv1.IBMCloudMachineProviderSpec{CredentialsSecret: &corev1.LocalObjectReference{Name: credentialsSecretName}})).AnyTimes()
 
 	expectedNodeAddresses := []corev1.NodeAddress{
@@ -423,7 +427,7 @@ func TestReconcileMachineWithCloudState(t *testing.T) {
 			Address: "10.0.0.1",
 		},
 	}
-	expectedProviderID := fmt.Sprintf("ibmvpc://CLUSTERID/%s/%s", zone, instanceName)
+	expectedProviderID := fmt.Sprintf("ibm://%s///CLUSTERID/%s", accountID, instanceID)
 
 	r := newReconciler(machineScope)
 	if err := r.reconcileMachineWithCloudState(nil); err != nil {
@@ -443,8 +447,8 @@ func TestReconcileMachineWithCloudState(t *testing.T) {
 	if *r.providerStatus.InstanceState != "running" {
 		t.Errorf("Expected: %s, got: %s", "running", *r.providerStatus.InstanceState)
 	}
-	if *r.providerStatus.InstanceID != intanceID {
-		t.Errorf("Expected: %s, got: %s", intanceID, *r.providerStatus.InstanceID)
+	if *r.providerStatus.InstanceID != instanceID {
+		t.Errorf("Expected: %s, got: %s", instanceID, *r.providerStatus.InstanceID)
 	}
 }