Skip to content

Commit

Permalink
Use zone to filter PowerVS service instance (#1853)
Browse files Browse the repository at this point in the history
  • Loading branch information
dharaneeshvrd authored Jun 21, 2024
1 parent 962cbd6 commit 104ee47
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 10 deletions.
1 change: 1 addition & 0 deletions api/v1beta2/ibmpowervscluster_types.go
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.
// 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
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
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
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
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
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
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
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
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

0 comments on commit 104ee47

Please sign in to comment.