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

Use zone to filter PowerVS service instance #1853

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Use zone to filter PowerVS service instance #1853

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: 1 addition & 0 deletions api/v1beta2/ibmpowervscluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ type IBMPowerVSClusterSpec struct {
// when omitted system will dynamically create the service instance with name CLUSTER_NAME-serviceInstance.
// when ServiceInstance.ID is set, its expected that there exist a service instance in PowerVS workspace with id or else system will give error.
// when ServiceInstance.Name is set, system will first check for service instance with Name in PowerVS workspace, if not exist system will create new instance.
// if there are more than one service instance exist with the ServiceInstance.Name in given Zone, installation fails with an error. Use ServiceInstance.ID in those situations to use the specific service instance.
Copy link
Member

Choose a reason for hiding this comment

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

where is this failure logic written?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mkumatag Here https://github.com/kubernetes-sigs/cluster-api-provider-ibmcloud/pull/1853/files#diff-c1cb986cbf553e2fe5a2a8955848ea3044570c1d27c74efef8553f3c8289856cR143

// ServiceInstance.Regex is not yet supported not yet supported and system will ignore the value.
// +optional
ServiceInstance *IBMPowerVSResourceReference `json:"serviceInstance,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion cloud/scope/powervs_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ func (s *PowerVSClusterScope) isServiceInstanceExists() (string, bool, error) {
// getServiceInstance return resource instance by name.
func (s *PowerVSClusterScope) getServiceInstance() (*resourcecontrollerv2.ResourceInstance, error) {
//TODO: Support regular expression
return s.ResourceClient.GetServiceInstance("", *s.GetServiceName(infrav1beta2.ResourceTypeServiceInstance))
return s.ResourceClient.GetServiceInstance("", *s.GetServiceName(infrav1beta2.ResourceTypeServiceInstance), s.IBMPowerVSCluster.Spec.Zone)
}

// createServiceInstance creates the service instance.
Expand Down
3 changes: 2 additions & 1 deletion cloud/scope/powervs_image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type PowerVSImageScopeParams struct {
Logger logr.Logger
IBMPowerVSImage *infrav1beta2.IBMPowerVSImage
ServiceEndpoint []endpoints.ServiceEndpoint
Zone *string
}

// PowerVSImageScope defines a scope defined around a Power VS Cluster.
Expand Down Expand Up @@ -116,7 +117,7 @@ func NewPowerVSImageScope(params PowerVSImageScopeParams) (scope *PowerVSImageSc
if params.IBMPowerVSImage.Spec.ServiceInstance != nil && params.IBMPowerVSImage.Spec.ServiceInstance.Name != nil {
name = *params.IBMPowerVSImage.Spec.ServiceInstance.Name
}
serviceInstance, err := rc.GetServiceInstance("", name)
serviceInstance, err := rc.GetServiceInstance("", name, params.Zone)
if err != nil {
params.Logger.Error(err, "error failed to get service instance id from name", "name", name)
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion cloud/scope/powervs_machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac
serviceInstanceName = *params.IBMPowerVSCluster.Spec.ServiceInstance.Name
}
}
serviceInstance, err := rc.GetServiceInstance(serviceInstanceID, serviceInstanceName)
serviceInstance, err := rc.GetServiceInstance(serviceInstanceID, serviceInstanceName, params.IBMPowerVSCluster.Spec.Zone)
if err != nil {
params.Logger.Error(err, "failed to get PowerVS service instance details", "name", serviceInstanceName, "id", serviceInstanceID)
return nil, err
Expand Down
1 change: 1 addition & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ spec:
when omitted system will dynamically create the service instance with name CLUSTER_NAME-serviceInstance.
when ServiceInstance.ID is set, its expected that there exist a service instance in PowerVS workspace with id or else system will give error.
when ServiceInstance.Name is set, system will first check for service instance with Name in PowerVS workspace, if not exist system will create new instance.
if there are more than one service instance exist with the ServiceInstance.Name in given Zone, installation fails with an error. Use ServiceInstance.ID in those situations to use the specific service instance.
ServiceInstance.Regex is not yet supported not yet supported and system will ignore the value.
properties:
id:
Expand Down
1 change: 1 addition & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ spec:
when omitted system will dynamically create the service instance with name CLUSTER_NAME-serviceInstance.
when ServiceInstance.ID is set, its expected that there exist a service instance in PowerVS workspace with id or else system will give error.
when ServiceInstance.Name is set, system will first check for service instance with Name in PowerVS workspace, if not exist system will create new instance.
if there are more than one service instance exist with the ServiceInstance.Name in given Zone, installation fails with an error. Use ServiceInstance.ID in those situations to use the specific service instance.
ServiceInstance.Regex is not yet supported not yet supported and system will ignore the value.
properties:
id:
Expand Down
10 changes: 6 additions & 4 deletions controllers/ibmpowervsimage_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,17 @@ func (r *IBMPowerVSImageReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, err
}

cluster, err := scope.GetClusterByName(ctx, r.Client, ibmImage.Namespace, ibmImage.Spec.ClusterName)
if err != nil {
return ctrl.Result{}, err
}

// Create the scope.
imageScope, err := scope.NewPowerVSImageScope(scope.PowerVSImageScopeParams{
Client: r.Client,
Logger: log,
IBMPowerVSImage: ibmImage,
Zone: cluster.Spec.Zone,
ServiceEndpoint: r.ServiceEndpoint,
})
if err != nil {
Expand All @@ -91,10 +97,6 @@ func (r *IBMPowerVSImageReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return r.reconcileDelete(imageScope)
}

cluster, err := scope.GetClusterByName(ctx, r.Client, ibmImage.Namespace, ibmImage.Spec.ClusterName)
if err != nil {
return ctrl.Result{}, err
}
return r.reconcile(cluster, imageScope)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud/services/resourcecontroller/resourcecontroller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type ResourceController interface {
ListResourceInstances(listResourceInstancesOptions *resourcecontrollerv2.ListResourceInstancesOptions) (result *resourcecontrollerv2.ResourceInstancesList, response *core.DetailedResponse, err error)
GetResourceInstance(*resourcecontrollerv2.GetResourceInstanceOptions) (*resourcecontrollerv2.ResourceInstance, *core.DetailedResponse, error)
CreateResourceInstance(*resourcecontrollerv2.CreateResourceInstanceOptions) (*resourcecontrollerv2.ResourceInstance, *core.DetailedResponse, error)
GetServiceInstance(string, string) (*resourcecontrollerv2.ResourceInstance, error)
GetServiceInstance(string, string, *string) (*resourcecontrollerv2.ResourceInstance, error)
DeleteResourceInstance(*resourcecontrollerv2.DeleteResourceInstanceOptions) (*core.DetailedResponse, error)

GetInstanceByName(string, string, string) (*resourcecontrollerv2.ResourceInstance, error)
Expand Down
13 changes: 11 additions & 2 deletions pkg/cloud/services/resourcecontroller/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (s *Service) DeleteResourceInstance(options *resourcecontrollerv2.DeleteRes

// GetServiceInstance returns service instance with given name or id. If not found, returns nil.
// TODO: Combine GetSreviceInstance() and GetInstanceByName().
func (s *Service) GetServiceInstance(id, name string) (*resourcecontrollerv2.ResourceInstance, error) {
func (s *Service) GetServiceInstance(id, name string, zone *string) (*resourcecontrollerv2.ResourceInstance, error) {
var serviceInstancesList []resourcecontrollerv2.ResourceInstance
f := func(start string) (bool, string, error) {
listServiceInstanceOptions := &resourcecontrollerv2.ListResourceInstancesOptions{
Expand All @@ -110,7 +110,16 @@ func (s *Service) GetServiceInstance(id, name string) (*resourcecontrollerv2.Res
return false, "", err
}
if serviceInstances != nil {
serviceInstancesList = append(serviceInstancesList, serviceInstances.Resources...)
if zone != nil && *zone != "" {
for _, resource := range serviceInstances.Resources {
if *resource.RegionID == *zone {
serviceInstancesList = append(serviceInstancesList, resource)
}
}
} else {
serviceInstancesList = append(serviceInstancesList, serviceInstances.Resources...)
}

nextURL, err := serviceInstances.GetNextStart()
if err != nil {
return false, "", err
Expand Down