-
Notifications
You must be signed in to change notification settings - Fork 32
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.
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
[bug_fix] Fix importing for mso_schema_template_anp_epg_contract to select the contract with the correct type (DCNE-205) #299
base: master
Are you sure you want to change the base?
Conversation
…elect the contract with the correct type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a modification in certain version? If I remember, at one point the contract could have both consumer and provider relationship. Did this change in recent version?
I see when looking at git blame that the relationship_type is already part of the original code from 4 years ago and is also part of the read function. I assumed ( perhaps wrongly ) that since it is already done also in the read function this is missing in import only by mistake. If it was introduced at a later stage in NDO/MSO then I think it is already broken for the read function for a while, and thus the resource should not be working properly in older versions. I can look up when this is introduced, will look through release notes and get back to you. |
@anvitha-jain Are you referring to the the resource in Terraform ACI that you worked on? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I think we are mixing it up with the template contract resource filterRelationships, filterRelationshipsConsumerToProvider and filterRelationshipsProviderToConsumer attributes which are separate lists. The only issue that could exist here is that there are other values for relationship_type in the API documentation and we are not listing them all. |
@@ -45,5 +45,5 @@ No attributes are exported. | |||
An existing MSO Schema Template Application Network Profile Endpoint Group Contract can be [imported][docs-import] into this resource via its Id/path, via the following command: [docs-import]: <https://www.terraform.io/docs/import/index.html> | |||
|
|||
```bash | |||
terraform import mso_schema_template_anp_epg_contract.contract1 {schema_id}/template/{template_name}/anp/{anp_name}/epg/{epg_name}/contract/{contract_name} | |||
terraform import mso_schema_template_anp_epg_contract.contract1 {schema_id}/template/{template_name}/anp/{anp_name}/epg/{epg_name}/contract/{contract_name}/relationship_type/{consumer|provider} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
terraform import mso_schema_template_anp_epg_contract.contract1 {schema_id}/template/{template_name}/anp/{anp_name}/epg/{epg_name}/contract/{contract_name}/relationship_type/{consumer|provider} | |
terraform import mso_schema_template_anp_epg_contract.contract1 {schema_id}/template/{template_name}/anp/{anp_name}/epg/{epg_name}/contract/{contract_name}/relationshipType/{consumer|provider} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
fixes #298