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.

Sign up for GitHub

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

Jump to bottom

Add support for transit enabled cloud connections #3758

Merged
merged 2 commits into from
May 24, 2022
Merged

Add support for transit enabled cloud connections #3758

merged 2 commits into from
May 24, 2022

Conversation

rachari
Copy link
Collaborator

@rachari rachari commented May 9, 2022 •
edited
Loading

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

=== RUN   TestAccIBMPICloudConnectionTransitGateway
--- PASS: TestAccIBMPICloudConnectionTransitGateway (636.92s)
PASS

=== RUN   TestAccIBMPICloudConnectionDataSource_basic
--- PASS: TestAccIBMPICloudConnectionDataSource_basic (51.31s)
PASS

@rachari rachari marked this pull request as ready for review May 12, 2022 15:34
@rachari
Copy link
Collaborator Author

rachari commented May 17, 2022

@yussufsh , Can you please review this ?

yussufsh
yussufsh requested changes May 17, 2022
View reviewed changes
Copy link
Collaborator

@yussufsh yussufsh left a comment

Choose a reason for hiding this comment

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

Please as well check why there are so many changes in the markdown files.

ibm/service/power/data_source_ibm_pi_cloud_connection.go Outdated Show resolved Hide resolved
ibm/service/power/data_source_ibm_pi_cloud_connection_test.go Outdated Show resolved Hide resolved
ibm/service/power/data_source_ibm_pi_cloud_connections.go Outdated Show resolved Hide resolved
ibm/service/power/resource_ibm_pi_cloud_connection_test.go Outdated Show resolved Hide resolved
@rachari
Copy link
Collaborator Author

rachari commented May 19, 2022

@yussufsh, I also checked the changes in the markdown files. These were due to an editor extension / plugin which formatted the markdown as per the CommonMark spec. Let me know if I need to revert those formatting changes.

@rachari rachari requested a review from yussufsh May 19, 2022 12:02
@yussufsh
Copy link
Collaborator

yussufsh commented May 24, 2022 •
edited
Loading

@yussufsh, I also checked the changes in the markdown files. These were due to an editor extension / plugin which formatted the markdown as per the CommonMark spec. Let me know if I need to revert those formatting changes.

Please rebase your changes and squash them in one commit. Also use the latest tag v1.1.9 which is available now.

@rachari
Copy link
Collaborator Author

rachari commented May 24, 2022

@yussufsh The changes have been squashed into a single commit, power-go-client updated to latest tag v1.1.9 as suggested

@hkantare
Copy link
Collaborator

Can you fix conflicts

hkantare
hkantare reviewed May 24, 2022
View reviewed changes
ibm/service/power/resource_ibm_pi_cloud_connection.go
if v, ok := d.GetOk(PICloudConnectionTransitEnabled); ok {
body.TransitEnabled = v.(bool)
}

client := st.NewIBMPICloudConnectionClient(ctx, sess, cloudInstanceID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

In Read can you set back PICloudConnectionTransitEnabled

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PICloudConnectionTransitEnabled can't be set in CloudConnectionRead, it applies only for Create. In Read, PICloudConnectionConnectionMode shows the connection mode. I have added this in the latest commit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hkantare that attribute is not computed and is not provided in the response.

@rachari
Add support for transit enabled cloud connections
099b05e 
Code changes to use features from latest release of power-go-client

Fixed typo

Removed test case

Review comment code changes

Review comment code changes

Updated power-go-client to v1.1.9

added PICloudConnectionConnectionMode in CloudConnectionRead
yussufsh
yussufsh approved these changes May 24, 2022
View reviewed changes
Copy link
Collaborator

@yussufsh yussufsh left a comment

Choose a reason for hiding this comment

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

lgtm

@yussufsh yussufsh requested a review from hkantare May 24, 2022 09:51
@hkantare hkantare merged commit 9928a26 into IBM-Cloud:master May 24, 2022
SunithaGudisagarIBM pushed a commit to ibm-vpc/terraform-provider-ibm that referenced this pull request Sep 14, 2022
@rachari @SunithaGudisagarIBM1
Add support for transit enabled cloud connections (IBM-Cloud#3758)
2177920 
Code changes to use features from latest release of power-go-client

Fixed typo

Removed test case

Review comment code changes

Review comment code changes

Updated power-go-client to v1.1.9

added PICloudConnectionConnectionMode in CloudConnectionRead
SunithaGudisagarIBM pushed a commit to ibm-vpc/terraform-provider-ibm that referenced this pull request Sep 14, 2022
@rachari @SunithaGudisagarIBM1
Add support for transit enabled cloud connections (IBM-Cloud#3758)
cd0e00b 
Code changes to use features from latest release of power-go-client

Fixed typo

Removed test case

Review comment code changes

Review comment code changes

Updated power-go-client to v1.1.9

added PICloudConnectionConnectionMode in CloudConnectionRead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yussufsh yussufsh yussufsh approved these changes

@hkantare hkantare Awaiting requested review from hkantare

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rachari @yussufsh @hkantare