remove deprecated v1 provider id format (#2122)

Signed-off-by: Prajyot Parab <[email protected]>

Makefile

@@ -181,7 +181,6 @@ generate-go-conversions: $(CONVERSION_GEN) ## Generate conversions go code
 .PHONY: generate-templates
 generate-templates: $(KUSTOMIZE) ## Generate cluster templates
 	$(KUSTOMIZE) build $(TEMPLATES_DIR)/cluster-template --load-restrictor LoadRestrictionsNone > $(TEMPLATES_DIR)/cluster-template.yaml
-	$(KUSTOMIZE) build $(TEMPLATES_DIR)/cluster-template-powervs --load-restrictor LoadRestrictionsNone > $(TEMPLATES_DIR)/cluster-template-powervs.yaml
 	$(KUSTOMIZE) build $(TEMPLATES_DIR)/cluster-template-powervs-cloud-provider --load-restrictor LoadRestrictionsNone > $(TEMPLATES_DIR)/cluster-template-powervs-cloud-provider.yaml
 	$(KUSTOMIZE) build $(TEMPLATES_DIR)/cluster-template-powervs-clusterclass --load-restrictor LoadRestrictionsNone > $(TEMPLATES_DIR)/cluster-template-powervs-clusterclass.yaml
 	$(KUSTOMIZE) build $(TEMPLATES_DIR)/cluster-template-vpc-clusterclass --load-restrictor LoadRestrictionsNone > $(TEMPLATES_DIR)/cluster-template-vpc-clusterclass.yaml

cloud/scope/machine.go

@@ -1077,14 +1077,14 @@ func (m *MachineScope) SetNotReady() {
 func (m *MachineScope) SetProviderID(id *string) error {
 	// Based on the ProviderIDFormat version the providerID format will be decided.
 	if options.ProviderIDFormatType(options.ProviderIDFormat) == options.ProviderIDFormatV2 {
-		accountID, err := utils.GetAccountID()
+		accountID, err := utils.GetAccountIDWrapper()
 		if err != nil {
 			m.Logger.Error(err, "failed to get cloud account id", err.Error())
 			return err
 		}
 		m.IBMVPCMachine.Spec.ProviderID = ptr.To(fmt.Sprintf("ibm://%s///%s/%s", accountID, m.Machine.Spec.ClusterName, *id))
 	} else {
-		m.IBMVPCMachine.Spec.ProviderID = ptr.To(fmt.Sprintf("ibmvpc://%s/%s", m.Machine.Spec.ClusterName, m.IBMVPCMachine.Name))
+		return fmt.Errorf("invalid value for ProviderIDFormat")
 	}
 	return nil
 }

cloud/scope/machine_test.go

@@ -35,7 +35,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	infrav1beta2 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2"
+	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/utils"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/vpc/mock"
+	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options"
 
 	. "github.com/onsi/gomega"
 )
@@ -124,6 +126,29 @@ func TestNewMachineScope(t *testing.T) {
 	}
 }
 
+func TestSetVPCProviderID(t *testing.T) {
+	providerID := "foo-provider-id"
+
+	t.Run("Set Provider ID in invalid format", func(t *testing.T) {
+		g := NewWithT(t)
+		scope := setupMachineScope(clusterName, machineName, mock.NewMockVpc(gomock.NewController(t)))
+		options.ProviderIDFormat = string("v1")
+		err := scope.SetProviderID(core.StringPtr(providerID))
+		g.Expect(err).ToNot(BeNil())
+	})
+
+	t.Run("Set Provider ID in valid format", func(t *testing.T) {
+		g := NewWithT(t)
+		scope := setupMachineScope(clusterName, machineName, mock.NewMockVpc(gomock.NewController(t)))
+		options.ProviderIDFormat = string("v2")
+		utils.GetAccountIDFunc = func() (string, error) {
+			return "dummy-account-id", nil // Return dummy value
+		}
+		err := scope.SetProviderID(core.StringPtr(providerID))
+		g.Expect(err).To(BeNil())
+	})
+}
+
 func TestCreateMachine(t *testing.T) {
 	setup := func(t *testing.T) (*gomock.Controller, *mock.MockVpc) {
 		t.Helper()

cloud/scope/powervs_machine.go

@@ -954,9 +954,8 @@ func (m *PowerVSMachineScope) GetServiceInstanceID() (string, error) {
 // SetProviderID will set the provider id for the machine.
 func (m *PowerVSMachineScope) SetProviderID(instanceID string) error {
 	// Based on the ProviderIDFormat version the providerID format will be decided.
-	if options.ProviderIDFormatType(options.ProviderIDFormat) == options.ProviderIDFormatV1 {
-		m.IBMPowerVSMachine.Spec.ProviderID = ptr.To(fmt.Sprintf("ibmpowervs://%s/%s", m.Machine.Spec.ClusterName, m.IBMPowerVSMachine.Name))
-		return nil
+	if options.ProviderIDFormatType(options.ProviderIDFormat) != options.ProviderIDFormatV2 {
+		return fmt.Errorf("invalid value for ProviderIDFormat")
 	}
 	m.V(3).Info("setting provider id in v2 format")
 

cloud/scope/powervs_machine_test.go

@@ -912,14 +912,12 @@ func TestGetMachineInternalIP(t *testing.T) {
 func TestSetProviderID(t *testing.T) {
 	providerID := "foo-provider-id"
 
-	t.Run("Set Provider ID in v1 format", func(t *testing.T) {
+	t.Run("Set Provider ID in invalid format", func(t *testing.T) {
 		g := NewWithT(t)
 		scope := setupPowerVSMachineScope(clusterName, machineName, ptr.To(pvsImage), ptr.To(pvsNetwork), true, nil)
-		options.ProviderIDFormat = string(options.ProviderIDFormatV1)
-		err := scope.SetProviderID("foo-providerID")
-		expectedProviderID := ptr.To(fmt.Sprintf("ibmpowervs://%s/%s", scope.Machine.Spec.ClusterName, scope.IBMPowerVSMachine.Name))
-		g.Expect(*scope.IBMPowerVSMachine.Spec.ProviderID).To(Equal(*expectedProviderID))
-		g.Expect(err).To(BeNil())
+		options.ProviderIDFormat = string("v1")
+		err := scope.SetProviderID(providerID)
+		g.Expect(err).ToNot(BeNil())
 	})
 
 	t.Run("failed to get service instance ID", func(t *testing.T) {

controllers/ibmpowervsmachine_controller_test.go

@@ -46,6 +46,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs/mock"
 	mockVPC "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/vpc/mock"
+	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options"
 
 	"github.com/IBM-Cloud/power-go-client/power/models"
 	"github.com/IBM/go-sdk-core/v5/core"
@@ -374,6 +375,7 @@ func TestIBMPowerVSMachineReconciler_ReconcileOperations(t *testing.T) {
 		mockCtrl.Finish()
 	}
 	t.Run("Reconciling creating IBMPowerVSMachine ", func(t *testing.T) {
+		options.ProviderIDFormat = string("v2")
 		t.Run("Should requeue if Cluster infrastructure status is not ready", func(t *testing.T) {
 			g := NewWithT(t)
 			setup(t)

controllers/ibmvpcmachine_controller_test.go

@@ -39,6 +39,7 @@ import (
 	infrav1beta2 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/cloud/scope"
 	gtmock "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/globaltagging/mock"
+	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/utils"
 	vpcmock "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/vpc/mock"
 
 	. "github.com/onsi/gomega"
@@ -327,6 +328,10 @@ func TestIBMVPCMachineLBReconciler_reconcile(t *testing.T) {
 		return gomock.NewController(t), mockvpc, mockgt, machineScope, reconciler
 	}
 
+	utils.GetAccountIDFunc = func() (string, error) {
+		return "dummy-account-id", nil // Return dummy value
+	}
+
 	t.Run("Reconcile creating IBMVPCMachine associated with LoadBalancer", func(t *testing.T) {
 		instancelist := &vpcv1.InstanceCollection{
 			Instances: []vpcv1.Instance{

main.go

@@ -135,14 +135,10 @@ func initFlags(fs *pflag.FlagSet) {
 }
 
 func validateFlags() error {
-	switch options.ProviderIDFormatType(options.ProviderIDFormat) {
-	// Deprecated: ProviderIDFormatV1 is deprecated and will be removed in a future release.
-	case options.ProviderIDFormatV1:
-		setupLog.Info("Using v1 version of ProviderID format.V1 is deprecated and will be removed in a future release.Instead use V2.")
-	case options.ProviderIDFormatV2:
+	if options.ProviderIDFormatType(options.ProviderIDFormat) == options.ProviderIDFormatV2 {
 		setupLog.Info("Using v2 version of ProviderID format")
-	default:
-		return fmt.Errorf("invalid value for flag provider-id-fmt: %s, Supported values: %s, %s ", options.ProviderIDFormat, options.ProviderIDFormatV1, options.ProviderIDFormatV2)
+	} else {
+		return fmt.Errorf("invalid value for flag provider-id-fmt: %s, Only supported value is %s", options.ProviderIDFormat, options.ProviderIDFormatV2)
 	}
 
 	if err := logsv1.ValidateAndApply(logOptions, nil); err != nil {

pkg/cloud/services/utils/accounts.go

@@ -54,6 +54,14 @@ func GetAccount(auth core.Authenticator) (string, error) {
 	return token.Claims.(jwt.MapClaims)["account"].(map[string]interface{})["bss"].(string), nil
 }
 
+// GetAccountIDFunc is a variable that will hold the function reference.
+var GetAccountIDFunc = GetAccountID // Default to the original function
+
+// GetAccountIDWrapper is a function that calls GetAccountIDFunc.
+func GetAccountIDWrapper() (string, error) {
+	return GetAccountIDFunc() // Call the function that GetAccountIDFunc points to
+}
+
 // GetAccountID will parse and returns user cloud account ID.
 func GetAccountID() (string, error) {
 	auth, err := authenticator.GetAuthenticator()

pkg/options/options.go

@@ -20,13 +20,6 @@ package options
 type ProviderIDFormatType string
 
 const (
-	// ProviderIDFormatV1 will set provider id to machine as follows
-	// For VPC machines: ibmvpc://<cluster_name>/<vm_hostname>
-	// For Power VS machines: ibmpowervs://<cluster_name>/<vm_hostname>
-	// Deprecated: ProviderIDFormatV1 is deprecated and will be removed in a future release. see https://github.com/kubernetes-sigs/cluster-api-provider-ibmcloud/issues/1868 for more details.
-	// use ProviderIDFormatV2 instead ProviderIDFormatV1.
-	ProviderIDFormatV1 ProviderIDFormatType = "v1"
-
 	// ProviderIDFormatV2 will set provider id to machine as follows
 	// For VPC machines: ibm://<account_id>///<cluster_id>/<vpc_machine_id>
 	// For Power VS machines: ibmpowervs://<region>/<zone>/<service_instance_id>/<powervs_machine_id>