terraform-ibm-modules/terraform-ibm-watsonx-saas-da

Allow to use references to existing instances

Closed this issue · 6 comments

in-1911 commented

Description

When the Watson SaaS DA is used to deploy resources for multiple solutions or is used in a stack context, the users should be able to use references to existing instances instead of provisioning new ones, especially for services with expensive plans like Discovery or Assistant.

  • Allow an optional parameter for Watson Discovery instance GUID. If supplied, no new instance is created, the plan parameter is ignored. The output values related to Discovery instance are set to corresponding attributes of the instance
  • Allow an optional parameter for Watson Assistant instance GUID. If supplied, no new instance is created, the plan parameter is ignored. The output values related to Discovery instance are set to corresponding attributes of the instance

The main advantage of this feature is the ability to seamlessly integrate the existing instances in stack deployments where the dependent components can use the output of the Watson SaaS DA in the same way whether new instances are provisioned or existing ones are referenced.

New or affected modules

mail.tf

By submitting this issue, you agree to follow our Code of Conduct

mkrudele commented

  1. I think we should use the CRN instead of the GUID. Do you agree?

  2. For each service we provide as output:

    • <service_name>_crn
    • <service_name>_dashboard_url
    • <service_name>_guid
    • <service_name>_name
    • <service_name>_plan_id

    In case the user selects do not install for the service plan and provides the instance CRN we are able to set only the <service_name>_crn and the <service_name>_guid. Is that ok?

  3. If the user inadvertently select a service plan and also set the CRN, what's the expected behavior?

in-1911 commented

  1. On the CRN vs GUID - to me it does not really make a difference, but I think the resource instance object takes an GUID (although for some resources they seem to be the same).
    I do not have a problem if we switch to CRN instead.

  2. If you also return the name for consistency when the existing CRN is passed - although currently I do not need it, but you would have it from resource_instance anyway, right? In addition to that, I would expect output related to sub-resources (if any) also returned, if the "create new" path returns it. E.g. if a project is created in a new instance per inputs, the same should happen and the corresponding outputs (if any) returned when using an existing instance.

  3. The "existing instance reference" parameter should take precedence over other inputs - i.e. in your case (both plan and existing reference supplied), the existing instance should be used and plan selection ignored, and that's how it will actually happen in the stack context. On the other hand, if both the plan is "do not install" and the existing instance is empty, no instance should be created and outputs are set to null - but that's a valid input combination. All of that will help with addressing the issue of how the Projects handle environment parameters in case a stack sets inputs for member DAs - we cannot have a logic that sets both plan (to "do not install") and instance reference if the latter is non-empty, that logic should be in the terraform.

mkrudele commented

About 2. we could get all the otuputs from the terraform data_ object of the provided instance. However, the deployer must have the permissions to read the resource (i.e. Assistant or Discovery), otherwise we'll fail. Do you envision any use case that we must support where the deployer does not have that permission?

in-1911 commented

I think it's reasonable to assume that if the Watson SaaS DA is used in a project, the associated identity would have at least read access to the instance. We even require a separate WatsonX Admin apikey (when instances need to be provisioned).

In the stack context, if an instance reference is passed, it means this instance would be used somewhere else in the stack and some level of access to it would be required anyway.

Fetching the instance details has the advantage of actually validating the reference.

  • 👍1
andreainnocenti commented

opened a PR to address this #129
I also opened an issue IBM-Cloud/terraform-provider-ibm#5542 since the TF provider does not return the same data using an existing instance.

ocofaigh commented

version 1.4.0 is out with this