From 78b9d0e42b24b75f8225d585236317b4cd17728c Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Wed, 2 Mar 2022 20:45:14 +0530 Subject: [PATCH] Added flag to select ProviderID format for Machine (#580) * Added flag to select ProviderID format for Machine * Added generated files --- api/v1alpha4/zz_generated.conversion.go | 2 + api/v1beta1/ibmpowervsmachine_types.go | 6 +++ api/v1beta1/zz_generated.deepcopy.go | 10 +++++ cloud/scope/powervs_machine.go | 44 ++++++++++++++++--- ...e.cluster.x-k8s.io_ibmpowervsmachines.yaml | 38 ++++------------ ...r.x-k8s.io_ibmpowervsmachinetemplates.yaml | 33 +------------- controllers/ibmpowervsmachine_controller.go | 2 +- main.go | 27 ++++++++++++ pkg/options/options.go | 26 +++++++++++ 9 files changed, 120 insertions(+), 68 deletions(-) create mode 100644 pkg/options/options.go diff --git a/api/v1alpha4/zz_generated.conversion.go b/api/v1alpha4/zz_generated.conversion.go index 1eb1266c2..3f6543859 100644 --- a/api/v1alpha4/zz_generated.conversion.go +++ b/api/v1alpha4/zz_generated.conversion.go @@ -608,6 +608,8 @@ func autoConvert_v1beta1_IBMPowerVSMachineStatus_To_v1alpha4_IBMPowerVSMachineSt // WARNING: in.FailureReason requires manual conversion: does not exist in peer-type // WARNING: in.FailureMessage requires manual conversion: does not exist in peer-type // WARNING: in.Conditions requires manual conversion: does not exist in peer-type + // WARNING: in.Region requires manual conversion: does not exist in peer-type + // WARNING: in.Zone requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1beta1/ibmpowervsmachine_types.go b/api/v1beta1/ibmpowervsmachine_types.go index bcc821e3f..7448c6309 100644 --- a/api/v1beta1/ibmpowervsmachine_types.go +++ b/api/v1beta1/ibmpowervsmachine_types.go @@ -158,6 +158,12 @@ type IBMPowerVSMachineStatus struct { // Conditions defines current service state of the IBMPowerVSMachine. // +optional Conditions clusterv1.Conditions `json:"conditions,omitempty"` + + // Region specifies the Power VS Service instance region + Region *string `json:"region,omitempty"` + + // Zone specifies the Power VS Service instance zone + Zone *string `json:"zone,omitempty"` } //+kubebuilder:object:root=true diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index ba1445c6f..9871cec71 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -345,6 +345,16 @@ func (in *IBMPowerVSMachineStatus) DeepCopyInto(out *IBMPowerVSMachineStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Region != nil { + in, out := &in.Region, &out.Region + *out = new(string) + **out = **in + } + if in.Zone != nil { + in, out := &in.Zone, &out.Zone + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IBMPowerVSMachineStatus. diff --git a/cloud/scope/powervs_machine.go b/cloud/scope/powervs_machine.go index 49e5d1b95..780ce93a3 100644 --- a/cloud/scope/powervs_machine.go +++ b/cloud/scope/powervs_machine.go @@ -23,11 +23,9 @@ import ( "strconv" "strings" - "k8s.io/utils/pointer" - "github.com/go-logr/logr" "github.com/pkg/errors" - utils "github.com/ppc64le-cloud/powervs-utils" + powerVSUtils "github.com/ppc64le-cloud/powervs-utils" "github.com/IBM-Cloud/power-go-client/ibmpisession" "github.com/IBM-Cloud/power-go-client/power/client/p_cloud_p_vm_instances" @@ -38,6 +36,8 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2/klogr" + "k8s.io/utils/pointer" + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" @@ -144,11 +144,13 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac return } - region, err := utils.GetRegion(*res.RegionID) + region, err := powerVSUtils.GetRegion(*res.RegionID) if err != nil { err = errors.Wrap(err, "failed to get region") return } + scope.SetRegion(region) + scope.SetZone(*res.RegionID) options := powervs.ServiceOptions{ IBMPIOptions: &ibmpisession.IBMPIOptions{ @@ -397,6 +399,36 @@ func (m *PowerVSMachineScope) GetInstanceState() v1beta1.PowerVSInstanceState { return m.IBMPowerVSMachine.Status.InstanceState } -func (m *PowerVSMachineScope) SetProviderID() { - m.IBMPowerVSMachine.Spec.ProviderID = pointer.StringPtr(fmt.Sprintf("ibmpowervs://%s/%s", m.Machine.Spec.ClusterName, m.IBMPowerVSMachine.Name)) +func (m *PowerVSMachineScope) SetRegion(region string) { + m.IBMPowerVSMachine.Status.Region = ®ion +} + +func (m *PowerVSMachineScope) GetRegion() string { + if m.IBMPowerVSMachine.Status.Region == nil { + return "" + } + return *m.IBMPowerVSMachine.Status.Region +} + +func (m *PowerVSMachineScope) SetZone(zone string) { + m.IBMPowerVSMachine.Status.Zone = &zone +} + +func (m *PowerVSMachineScope) GetZone() string { + if m.IBMPowerVSMachine.Status.Zone == nil { + return "" + } + return *m.IBMPowerVSMachine.Status.Zone +} + +// SetProviderID will set the provider id for the machine +func (m *PowerVSMachineScope) SetProviderID(id *string) { + // Based on the ProviderIDFormat version the providerID format will be decided + if options.PowerVSProviderIDFormat == options.PowerVSProviderIDFormatV2 { + if id != nil { + m.IBMPowerVSMachine.Spec.ProviderID = pointer.StringPtr(fmt.Sprintf("ibmpowervs://%s/%s/%s/%s", m.GetRegion(), m.GetZone(), m.IBMPowerVSMachine.Spec.ServiceInstanceID, *id)) + } + } else { + m.IBMPowerVSMachine.Spec.ProviderID = pointer.StringPtr(fmt.Sprintf("ibmpowervs://%s/%s", m.Machine.Spec.ClusterName, m.IBMPowerVSMachine.Name)) + } } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachines.yaml index 5fb2df2aa..6c3e32a10 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachines.yaml @@ -205,37 +205,9 @@ spec: resource that holds the details for provisioning the Image for a Cluster. properties: - apiVersion: - description: API version of the referent. - type: string - fieldPath: - description: 'If referring to a piece of an object instead of - an entire object, this string should contain a valid JSON/Go - field access statement, such as desiredState.manifest.containers[2]. - For example, if the object reference is to a container within - a pod, this would take on a value like: "spec.containers{name}" - (where "name" refers to the name of the container that triggered - the event) or if no container name is specified "spec.containers[2]" - (container with index 2 in this pod). This syntax is chosen - only to have some well-defined way of referencing a part of - an object. TODO: this design is not final and this field is - subject to change in the future.' - type: string - kind: - description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' - type: string - namespace: - description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' - type: string - resourceVersion: - description: 'Specific resourceVersion to which this reference - is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' - type: string - uid: - description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object memory: @@ -390,6 +362,12 @@ spec: ready: description: Ready is true when the provider resource is ready. type: boolean + region: + description: Region specifies the Power VS Service instance region + type: string + zone: + description: Zone specifies the Power VS Service instance zone + type: string type: object type: object served: true diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachinetemplates.yaml index 07648b589..599b7ea84 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachinetemplates.yaml @@ -162,38 +162,9 @@ spec: resource that holds the details for provisioning the Image for a Cluster. properties: - apiVersion: - description: API version of the referent. - type: string - fieldPath: - description: 'If referring to a piece of an object instead - of an entire object, this string should contain a valid - JSON/Go field access statement, such as desiredState.manifest.containers[2]. - For example, if the object reference is to a container - within a pod, this would take on a value like: "spec.containers{name}" - (where "name" refers to the name of the container that - triggered the event) or if no container name is specified - "spec.containers[2]" (container with index 2 in this - pod). This syntax is chosen only to have some well-defined - way of referencing a part of an object. TODO: this design - is not final and this field is subject to change in - the future.' - type: string - kind: - description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' - type: string - namespace: - description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' - type: string - resourceVersion: - description: 'Specific resourceVersion to which this reference - is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' - type: string - uid: - description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object memory: diff --git a/controllers/ibmpowervsmachine_controller.go b/controllers/ibmpowervsmachine_controller.go index bdc93f454..7237e73e9 100644 --- a/controllers/ibmpowervsmachine_controller.go +++ b/controllers/ibmpowervsmachine_controller.go @@ -205,6 +205,7 @@ func (r *IBMPowerVSMachineReconciler) reconcileNormal(ctx context.Context, machi if err != nil { return ctrl.Result{}, err } + machineScope.SetProviderID(instance.PvmInstanceID) machineScope.SetInstanceID(instance.PvmInstanceID) machineScope.SetAddresses(instance) machineScope.SetHealth(instance.Health) @@ -227,7 +228,6 @@ func (r *IBMPowerVSMachineReconciler) reconcileNormal(ctx context.Context, machi } machineScope.Info(*ins.PvmInstanceID) } - machineScope.SetProviderID() // Requeue after 2 minute if machine is not ready to update status of the machine properly if !machineScope.IsReady() { diff --git a/main.go b/main.go index b15f90e44..0739a85c6 100644 --- a/main.go +++ b/main.go @@ -18,6 +18,7 @@ package main import ( "flag" + "fmt" "os" "time" @@ -35,6 +36,7 @@ import ( infrastructurev1alpha4 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1alpha4" infrastructurev1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1" "sigs.k8s.io/cluster-api-provider-ibmcloud/controllers" + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options" // +kubebuilder:scaffold:imports ) @@ -68,6 +70,11 @@ func main() { ctrl.SetLogger(klogr.New()) + if err := validateFlags(); err != nil { + setupLog.Error(err, "Flag validation failure") + os.Exit(1) + } + if watchNamespace != "" { setupLog.Info("Watching cluster-api objects only in namespace for reconciliation", "namespace", watchNamespace) } @@ -210,4 +217,24 @@ func initFlags(fs *pflag.FlagSet) { 10*time.Minute, "The minimum interval at which watched resources are reconciled.", ) + + fs.StringVar( + &options.PowerVSProviderIDFormat, + "powervs-provider-id-fmt", + options.PowerVSProviderIDFormatV1, + "ProviderID format is used set the Provider ID format for Machine", + ) +} + +func validateFlags() error { + switch options.PowerVSProviderIDFormat { + case options.PowerVSProviderIDFormatV1: + setupLog.Info("Using v1 version of ProviderID format") + case options.PowerVSProviderIDFormatV2: + setupLog.Info("Using v2 version of ProviderID format") + default: + errStr := fmt.Errorf("Invalid value for flag powervs-provider-id-fmt: %s, Supported values: v1, v2 ", options.PowerVSProviderIDFormat) + return errStr + } + return nil } diff --git a/pkg/options/options.go b/pkg/options/options.go new file mode 100644 index 000000000..4c9f4db1a --- /dev/null +++ b/pkg/options/options.go @@ -0,0 +1,26 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package options + +// PowerVSProviderIDFormat is used to identify the Provider ID format for Machine +var PowerVSProviderIDFormat string + +// PowerVSProviderIDFormatV2 will set provider id to machine as ibmpowervs:///// +const PowerVSProviderIDFormatV2 = "v2" + +// PowerVSProviderIDFormatV1 will set provider id to machine as ibmpowervs:/// +const PowerVSProviderIDFormatV1 = "v1"