Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove deprecated v1 provider id format #2122

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

remove deprecated v1 provider id format #2122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion cloud/scope/machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,8 @@ func (m *MachineScope) SetProviderID(id *string) error {
}
m.IBMVPCMachine.Spec.ProviderID = ptr.To(fmt.Sprintf("ibm://%s///%s/%s", accountID, m.Machine.Spec.ClusterName, *id))
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in main.go we have a check to make sure only v2 provider id is valid, So couple of things here

  1. Is if check at 1079 needed?
  2. If we keep if condition then if its not v2 format we should return error.

m.IBMVPCMachine.Spec.ProviderID = ptr.To(fmt.Sprintf("ibmvpc://%s/%s", m.Machine.Spec.ClusterName, m.IBMVPCMachine.Name))
var err error
m.Logger.Error(err, "invalid value for ProviderIDFormat")
Comment on lines +1087 to +1088
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var err error
m.Logger.Error(err, "invalid value for ProviderIDFormat")
m.Logger.Error(nil, "invalid value for ProviderIDFormat")

}
return nil
}
Expand Down
5 changes: 0 additions & 5 deletions cloud/scope/powervs_machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ import (
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/vpc"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/endpoints"
ignV2Types "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/ignition"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/record"
)

Expand Down Expand Up @@ -954,10 +953,6 @@ 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
}
m.V(3).Info("setting provider id in v2 format")

serviceInstanceID, err := m.GetServiceInstanceID()
Expand Down
10 changes: 0 additions & 10 deletions cloud/scope/powervs_machine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,16 +912,6 @@ 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) {
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())
})

t.Run("failed to get service instance ID", func(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test case for the invalid provider format if it doesn't exist already

g := NewWithT(t)
scope := PowerVSMachineScope{
Expand Down
10 changes: 3 additions & 7 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 0 additions & 7 deletions pkg/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
296 changes: 0 additions & 296 deletions templates/cluster-template-powervs.yaml
View file
Open in desktop

This file was deleted.

16 changes: 0 additions & 16 deletions templates/cluster-template-powervs/kcp.yaml
View file
Open in desktop

This file was deleted.

Loading