Closed in-1911 closed 3 months ago
mkrudele
commented
3 months ago I think we should use the CRN instead of the GUID. Do you agree?
For each service we provide as output:
In case the user selects do not install for the service plan and provides the instance CRN we are able to set only the
If the user inadvertently select a service plan and also set the CRN, what's the expected behavior?
in-1911
commented
3 months ago 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.
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.
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
3 months ago 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
3 months ago 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.
andreainnocenti
commented
3 months ago opened a PR to address this https://github.com/terraform-ibm-modules/terraform-ibm-watsonx-saas-da/pull/129 I also opened an issue https://github.com/IBM-Cloud/terraform-provider-ibm/issues/5542 since the TF provider does not return the same data using an existing instance.
ocofaigh
commented
3 months ago version 1.4.0 is out with this
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.
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