-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Bug 1788707: Backport Azure private DNS to 4.2 branch #2825
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 1788707: Backport Azure private DNS to 4.2 branch #2825
Conversation
…ndle private_dns zone
Using the upstream azurerm provider is not possible for now because of following reasons:
1) There is not srv record resource for private dns zone
2) The version of provider that has the private dns zone resources `1.34.0` has a lot of bugs like
* hashicorp/terraform-provider-azurerm#4452
* hashicorp/terraform-provider-azurerm#4453
* hashicorp/terraform-provider-azurerm#4501
Some of these bugs are fixed, and some are in flight.
Another reason moving to `1.36.0` which might have all the fixes we need is the provider has moved to using
`standalone terraform plugin SDK v1.1.1` [1]. Because we vendor both terraform and providers, this causes errors like
`panic: gob: registering duplicate types for "github.com/zclconf/go-cty/cty.primitiveType": cty.primitiveType != cty.primitiveType`
Therefore, we would have to move towards a single vendor for terraform and plugins for correct inter-operation, which is tricker due to conflicts elsewhere
A simple 4 resource plugin that re-uses the already vendored azurerm provider as library and carries over the required resources seems like an easy fix for now.
[1]: hashicorp/terraform-provider-azurerm#4474
(cherry picked from commit af00810)
Using the Private DNS Zone also allows us to remove our previous hack [1] [1]: openshift@8ac9ab4 (cherry picked from commit c1e24af) Conflicts: data/data/azure/master/variables.tf
the ingress-operator can handle the Private DNS Zone since [1] [1]: openshift/cluster-ingress-operator#300 (cherry picked from commit 7d3119f)
(cherry picked from commit e80fde1)
… deleting public records Updates the destroy to look for both DNS Zones type Private and Private DNS Zones to find the private records and the corresponding DNS Zone type Public. Since the zone name for both types of private dns zone is the same, we can try both to calculate the private records and then re-use the same codepath to delete the public records. (cherry picked from commit 65111a0)
|
@jhixson74: No Bugzilla bug is referenced in the title of this pull request. DetailsIn response to this:
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. |
|
@jhixson74: No Bugzilla bug is referenced in the title of this pull request. DetailsIn response to this:
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. |
|
/test e2e-azure |
|
@jhixson74: This pull request references Bugzilla bug 1788707, which is invalid:
Comment DetailsIn response to this:
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. |
|
@jhixson74: This pull request references Bugzilla bug 1788707, which is invalid:
Comment DetailsIn response to this:
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. |
|
/bugzilla refresh |
|
@jhixson74: This pull request references Bugzilla bug 1788707, which is invalid:
Comment DetailsIn response to this:
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. |
|
/bugzilla refresh |
|
@jhixson74: This pull request references Bugzilla bug 1788707, which is invalid:
Comment DetailsIn response to this:
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. |
|
/bugzilla refresh |
|
@jhixson74: This pull request references Bugzilla bug 1788707, which is invalid:
Comment DetailsIn response to this:
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. |
|
/bugzilla refresh |
|
@mtnbikenc: This pull request references Bugzilla bug 1788707, which is valid. The bug has been moved to the POST state. DetailsIn response to this:
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. |
|
/test e2e-azure |
|
There's no 4.3 analog to this installer PR, and the 4.3 rhbz#1772804 is VERIFIED with no installer changes. Is that intentional? Can you explain why 4.3+ don't need this installer change? |
|
Azure job died with: That's a MachineAPIOperatorDown and a bunch of TargetDown. I didn't dig deeper to see if they were related to this PR or not. |
This code is a feature in 4.3. The same commits in this PR are already in 4.3. This PR is for back porting the changes to 4.2. |
The only problem I am aware of as to why the Azure test will fail is the ingress operator not having support for the new Azure private DNS in 4.2. The PR for that (openshift/cluster-ingress-operator#344) merged today, so the test should now work. I am not clear on if this change is immediate or if there is a delay before it is available for CI. |
|
/test e2e-azure |
PR merging should be all that is needed before changes appear in subsequent CI jobs. But if you want to check for a particular job, can drill in like:
|
|
master_subnet_cidr was nuked in 4.3 except in the vnet module, where it has become a local. The cherry pick relies on this, so that commit removed the older 4.2 references. |
jstuever
left a comment
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
|
/approve |
|
/approve Trying again. |
|
/assign @jhixson74 |
|
/approve Eventually Prow will hear about this ;). |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jstuever, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@jhixson74: All pull requests linked via external trackers have merged. Bugzilla bug 1788707 has been moved to the MODIFIED state. DetailsIn response to this:
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. |
This brings Azure private DNS into the 4.2 branch
https://issues.redhat.com/browse/CORS-1316