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

Power VS: Enable IBM DNS service for private cluster support #6228

Merged
merged 3 commits into from
Oct 1, 2022
Merged

Power VS: Enable IBM DNS service for private cluster support #6228

merged 3 commits into from
Oct 1, 2022

Conversation

mjturek
Copy link
Contributor

@mjturek mjturek commented Aug 15, 2022 •
edited
Loading

If creating a public cluster, continue to use CIS DNS. If creating a private cluster, create DNS records in IBM DNS Services. This is controlled by the "PublishStrategy" install config element.

@openshift-ci openshift-ci bot requested review from jstuever and rna-afk August 15, 2022 21:11
@mjturek
Copy link
Contributor Author

mjturek commented Aug 15, 2022

/retest

@mjturek mjturek force-pushed the use-ibm-dns-for-private branch 2 times, most recently from edba468 to 9176c2b Compare August 17, 2022 22:37
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2022
@mjturek mjturek changed the title Power VS: Create LB DNS records in IBM DNS for private clusters [WIP] Power VS: Create LB DNS records in IBM DNS for private clusters Aug 23, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 23, 2022
@mjturek mjturek force-pushed the use-ibm-dns-for-private branch from 5787b11 to b95b8be Compare August 25, 2022 17:56
@mjturek
Copy link
Contributor Author

mjturek commented Aug 25, 2022

Requires #6227 and #6157

@mjturek mjturek changed the title [WIP] Power VS: Create LB DNS records in IBM DNS for private clusters Power VS: Create LB DNS records in IBM DNS for private clusters Aug 25, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 25, 2022
@mjturek mjturek changed the title Power VS: Create LB DNS records in IBM DNS for private clusters Power VS: Enable IBM DNS service for private cluster support Aug 25, 2022
@mjturek mjturek force-pushed the use-ibm-dns-for-private branch 2 times, most recently from 86e8526 to b5ae339 Compare August 31, 2022 21:19
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 31, 2022
@mjturek mjturek force-pushed the use-ibm-dns-for-private branch from b5ae339 to 280054a Compare September 1, 2022 19:29
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 1, 2022
@mjturek mjturek force-pushed the use-ibm-dns-for-private branch from 280054a to 2763e24 Compare September 2, 2022 02:06
@mjturek mjturek force-pushed the use-ibm-dns-for-private branch 2 times, most recently from 03e352f to e0e8afa Compare September 13, 2022 01:32
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 13, 2022
@mjturek
Copy link
Contributor Author

mjturek commented Sep 13, 2022

@clnperez @hamzy @miyamotoh @cs-zhang @23TNC

PTAL

@mjturek
Copy link
Contributor Author

mjturek commented Sep 13, 2022

/retest-required

@miyamotoh
Copy link
Contributor

The count control in TF with the hardcoded Internal looks a bit tedious, but I'm not sure if there's a standard/better way. LGTM.

@mjturek
Copy link
Contributor Author

mjturek commented Sep 13, 2022 •
edited
Loading

The count control in TF with the hardcoded Internal looks a bit tedious, but I'm not sure if there's a standard/better way. LGTM.

@miyamotoh yeah agreed. I could have separate modules for private and public DNS that would remove a lot of the count controls but want to consider doing that later since I know this currently works

@mjturek mjturek force-pushed the use-ibm-dns-for-private branch from e0e8afa to f83e74c Compare September 14, 2022 13:35
@mjturek
Copy link
Contributor Author

mjturek commented Sep 14, 2022

/retest-required

@mjturek mjturek mentioned this pull request Sep 14, 2022
Merged
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2022
@clnperez
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 28, 2022
@mjturek
Copy link
Contributor Author

mjturek commented Sep 28, 2022

@rna-afk Could you take a look? We think this is ready to merge

@mjturek mjturek force-pushed the use-ibm-dns-for-private branch from 21ec631 to 1ca9370 Compare September 29, 2022 13:12
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 29, 2022
@mjturek mjturek force-pushed the use-ibm-dns-for-private branch 2 times, most recently from f814349 to 01d3006 Compare September 29, 2022 16:21
@mjturek mjturek force-pushed the use-ibm-dns-for-private branch from 01d3006 to 5988bfe Compare September 29, 2022 16:58
@mjturek
Copy link
Contributor Author

mjturek commented Sep 29, 2022

@rna-afk fixed the unit test issues. Please take a look.

@miyamotoh
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 29, 2022
@mjturek
Copy link
Contributor Author

mjturek commented Sep 29, 2022

/retest-required

@r4f4
Copy link
Contributor

r4f4 commented Sep 29, 2022

/test tf-fmt

@patrickdillon
Copy link
Contributor

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 30, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 30, 2022
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 2b6bc4b and 2 for PR HEAD 5988bfe in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 30, 2022 •
edited
Loading

@mjturek: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-imdsv2 b95b8beee8f028293d680e9f91782945687ed82a link false /test e2e-aws-imdsv2
ci/prow/e2e-azure-upi b95b8beee8f028293d680e9f91782945687ed82a link true /test e2e-azure-upi
ci/prow/e2e-aws-upi b95b8beee8f028293d680e9f91782945687ed82a link true /test e2e-aws-upi
ci/prow/e2e-vsphere-upi b95b8beee8f028293d680e9f91782945687ed82a link true /test e2e-vsphere-upi
ci/prow/e2e-aws 2763e24c2d29031dc6397ad7bafc9f672815c233 link true /test e2e-aws
ci/prow/e2e-azure 2763e24c2d29031dc6397ad7bafc9f672815c233 link true /test e2e-azure
ci/prow/e2e-gcp 2763e24c2d29031dc6397ad7bafc9f672815c233 link true /test e2e-gcp
ci/prow/e2e-vsphere 2763e24c2d29031dc6397ad7bafc9f672815c233 link true /test e2e-vsphere
ci/prow/e2e-ibmcloud 2763e24c2d29031dc6397ad7bafc9f672815c233 link false /test e2e-ibmcloud
ci/prow/okd-e2e-gcp-ovn-upgrade 21ec6317ec029c67cd73e48f0ac622fdd6d95d13 link false /test okd-e2e-gcp-ovn-upgrade
ci/prow/e2e-libvirt 5988bfe link false /test e2e-libvirt
ci/prow/okd-e2e-aws-ovn 5988bfe link false /test okd-e2e-aws-ovn
ci/prow/okd-e2e-aws-upgrade 5988bfe link false /test okd-e2e-aws-upgrade

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 7c780d1 and 1 for PR HEAD 5988bfe in total

@openshift-merge-robot openshift-merge-robot merged commit 0a19d75 into openshift:master Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clnperez clnperez clnperez left review comments

@jstuever jstuever Awaiting requested review from jstuever

@rna-afk rna-afk Awaiting requested review from rna-afk

Assignees

@miyamotoh miyamotoh

@clnperez clnperez

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mjturek @miyamotoh @clnperez @r4f4 @patrickdillon @openshift-ci-robot @openshift-merge-robot