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.

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

feat: implement resource and data sources to create and fetch direct link route reports #4065

Merged
merged 14 commits into from
Oct 14, 2022

Conversation

ajay-malhotra1
Copy link
Contributor

@ajay-malhotra1 ajay-malhotra1 commented Sep 27, 2022

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccIBMDLRouteReportDataSource_basic'
=== RUN   TestAccIBMDLRouteReportDataSource_basic
--- PASS: TestAccIBMDLRouteReportDataSource_basic (309.57s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/directlink	310.318s
...
$ make testacc TESTARGS='-run=TestAccIBMDLRouteReportResource_basic'
=== RUN   TestAccIBMDLRouteReportResource_basic
--- PASS: TestAccIBMDLRouteReportResource_basic (281.03s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/directlink	281.729s
...
$ make testacc TESTARGS='-run=TestAccIBMDLRouteReportsDataSource_basic'
=== RUN   TestAccIBMDLRouteReportsDataSource_basic
--- PASS: TestAccIBMDLRouteReportsDataSource_basic (356.05s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/directlink	356.788s
...

@@ -75,6 +75,46 @@ func DataSourceIBMDLGateway() *schema.Resource {
},
},

dlAsPrepends: {
Type: schema.TypeList,
Optional: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

All the inner attributes are computed, parent attribute should be marked as computed..

dlAsPrepends: {
Type: schema.TypeList,
Optional: true,
ForceNew: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

All the inner attributes are computed, parent attribute should be marked as computed..


getGatewayRouteReportOptionsModel := &directlinkv1.GetGatewayRouteReportOptions{GatewayID: &gatewayId, ID: &routeReportId}
report, response, err := directLink.GetGatewayRouteReport(getGatewayRouteReportOptionsModel)
if err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

check if report == nil as well along with err !=nil
if err != nil && report == nil {

Since you are setting id only when report is not nil not returning error while report == nil will result in unusual terraform behaviour

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the nil check for report separately below to return a different error

gateway = ibm_dl_gateway.test_dl_gateway.id
}

data "ibm_dl_route_reports" "test_dl_reports" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This datasource will execute before ibm_dl_route_report is actually created
Add depends_on on ibm_dl_route_reports datasource for actual functunality testing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This datasource is referencing the resource ibm_dl_route_report now and will be called after that route report resource creation.

gatewayId := d.Get(dlGatewayId).(string)
createGatewayRouteReportOptionsModel := &directlinkv1.CreateGatewayRouteReportOptions{GatewayID: &gatewayId}
routeReport, response, err := directLink.CreateGatewayRouteReport(createGatewayRouteReportOptionsModel)
if err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

check for routeReport == nil along with err !=nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the nil check for routeReport separately below to return a different error

getGatewayRouteReportOptionsModel := &directlinkv1.GetGatewayRouteReportOptions{GatewayID: &gatewayId, ID: &routeReportId}
report, response, err := directLink.GetGatewayRouteReport(getGatewayRouteReportOptionsModel)
if err != nil {
log.Println("[WARN] Error fetching DL Route Reports", response, err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

check for response status == 404.. if 404 setId to "". This helps in lifecycle management.
Refer other resources for reference

@@ -17554,6 +17554,257 @@
}
}
],
"ibm_dl_route_report": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is auto-generated during release.. Kindly remove changes from this file

}
```

## Argument reference
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please make sure argument reference matches with resource schema.
Any Required / Optional / Optional and Computed arguments in the schema should be mentioned in Argument reference section

Any argument that is marked as only computed in schema should be mentioned in Attribute reference..

All

- `virtual_connection_type` - (String) Virtual Connection type

## Import
The `ibm_dl_gateway` resource can be imported by using gateway ID.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Import is possible with the id that is set for d.SetId in the create method of the resource..

Id is set to a combination in the resource

d.SetId(fmt.Sprintf("%s/%s", gatewayId, *routeReport.ID))

Resource can be imported using <gateway_ID>/<report_ID>. Please update the docs accordingly.

layout: "ibm"
page_title: "IBM : dl_route_report"
description: |-
Manages IBM Direct Link Gateway.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kindly change to appropriate description of the resource

Copy link
Collaborator

@kavya498 kavya498 left a comment

Choose a reason for hiding this comment

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

Kindly address review comments, rebase and resolve conflicts
Thanks

@ajay-malhotra1
Copy link
Contributor Author

@kavya498 The review comments have been addressed. Can you please review it again

@hkantare hkantare merged commit 3253ad1 into IBM-Cloud:master Oct 14, 2022
@ajay-malhotra1 ajay-malhotra1 deleted the rr branch October 14, 2022 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants