-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Power VS: Enable global routing for cloud connection and specify dns #6289
Power VS: Enable global routing for cloud connection and specify dns #6289
Conversation
/LGTM |
/retest |
Because the VPC will not always be in the same DC as the Power VS service instance, we want to enable global routing to allow for this. Additionally we want to specify the DNS server as it's required until private cluster support lands.
63d10c8
to
6e8702c
Compare
@@ -12,15 +12,17 @@ resource "ibm_pi_dhcp" "new_dhcp_service" { | |||
count = var.pvs_network_name == "" ? 1 : 0 | |||
pi_cloud_instance_id = var.cloud_instance_id | |||
pi_cloud_connection_id = data.ibm_pi_cloud_connection.cloud_connection.id | |||
pi_dns_server = "1.1.1.1" |
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.
I prefer avoiding this for long run, if its workaround at the moment - lets mention in the comment as TODO to remove once the issue gets fixed in the infrastructure.
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.
@mkumatag In the future, once Private Cluster Support lands, we will always be passing pi_dns_server
and it will default to 1.1.1.1
.
Do you still think we need a TODO
?
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.
what happens if we don't pass pi_dns_server? the api itself doesn't still default it to 1.1.1.1?
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.
@mkumatag In the future, once Private Cluster Support lands, we will always be passing
pi_dns_server
and it will default to1.1.1.1
.Do you still think we need a
TODO
?
nope, we should have if/else sort of a block where if user supplies that thens set this param or else drop(because this is an optional param and powervs code should take care of setting right dns server) to rely on what powervs dhcp service gives.
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.
what happens if we don't pass pi_dns_server? the api itself doesn't still default it to 1.1.1.1?
This is not happening with the latest code change but eventually that's the expectation
@mjturek: The following tests failed, say
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. |
@rna-afk could you take a look at this? We need this to resume our CI efforts |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rna-afk 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 |
/lgtm |
Because the VPC will not always be in the same DC as the Power VS service instance, we want to enable global routing to allow for this. Additionally we want to specify the DNS server as it's required until private cluster support lands.