Skip to content

NE-568: Add dnsManagementPolicy option to ingresscontrollers #781

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
Merged
openshift-merge-robot merged 2 commits into from
Oct 1, 2022
Merged

NE-568: Add dnsManagementPolicy option to ingresscontrollers #781

openshift-merge-robot merged 2 commits into from
Oct 1, 2022

Conversation

@thejasn
Copy link
Contributor

@thejasn thejasn commented Jun 15, 2022 •
edited
Loading

Add dnsManagementPolicy option to ingresscontrollers

  • pkg/operator/controller/dns/controller.go
    • Create DNS provider based on dnsManagementPolicy on DNSRecord CR.
    • Add sanitize function for purging deprecated fields on DNS record status.
    • Refactor publishRecordToZones()
  • pkg/operator/controller/ingress/controller.go - Reconcile changes made to dnsManagementPolicy on ingress spec.
  • pkg/operator/controller/ingress/dns.go
    • Create desired DNSRecord with dnsManagementPolicy derived from ingresscontroller status.
    • Converge domain mismatch check with UmanagedDNS policy.
  • pkg/operator/controller/ingress/status.go - Update status of types DNSManaged and DNSReady.
  • Renamed test/e2e/util.go -> test/e2e/util_test.go to allow gopls to index variables(kclient) declared under other test files.

EP: openshift/enhancements#1114
CFE: CFE-339, CFE-342

Signed-off-by: thejasn [email protected]

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 15, 2022
@openshift-ci openshift-ci bot requested review from frobware and gcs278 June 15, 2022 07:52
@thejasn thejasn force-pushed the feature/configurable-dns-lb branch from cd67799 to 284fd56 Compare June 15, 2022 07:53
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2022
@thejasn thejasn force-pushed the feature/configurable-dns-lb branch 7 times, most recently from 5e062e7 to 27b650c Compare June 16, 2022 08:53
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 21, 2022
@thejasn thejasn force-pushed the feature/configurable-dns-lb branch from 27b650c to 26c40e3 Compare June 29, 2022 06:46
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2022
@thejasn thejasn force-pushed the feature/configurable-dns-lb branch 2 times, most recently from 06e14c4 to be83650 Compare July 4, 2022 13:14
@thejasn
Copy link
Contributor Author

thejasn commented Jul 5, 2022

/retest

@thejasn
Copy link
Contributor Author

thejasn commented Jul 5, 2022

/test e2e-aws-operator

@thejasn thejasn force-pushed the feature/configurable-dns-lb branch from be83650 to 77102c7 Compare July 18, 2022 06:24
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2022
@thejasn thejasn force-pushed the feature/configurable-dns-lb branch 2 times, most recently from 96a2c24 to 154b25b Compare July 18, 2022 06:42
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2022
@thejasn
Copy link
Contributor Author

thejasn commented Jul 18, 2022

/test e2e-aws-single-node

@thejasn thejasn force-pushed the feature/configurable-dns-lb branch 2 times, most recently from 6abe0ef to 79399fd Compare July 18, 2022 09:35
@thejasn
Copy link
Contributor Author

thejasn commented Jul 18, 2022

/test e2e-aws-single-node

@thejasn
Copy link
Contributor Author

thejasn commented Jul 18, 2022

/test e2e-aws-operator

@thejasn
Copy link
Contributor Author

thejasn commented Sep 29, 2022

/test e2e-gcp-ovn-serial

1 similar comment
@thejasn
Copy link
Contributor Author

thejasn commented Sep 30, 2022

/test e2e-gcp-ovn-serial

@Miciah
Copy link
Contributor

Miciah commented Sep 30, 2022

/approve

@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 30, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 30, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah

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
@thejasn thejasn force-pushed the feature/configurable-dns-lb branch from f7fee8e to 0684b7b Compare September 30, 2022 15:22
@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 30, 2022
thejasn added 2 commits September 30, 2022 20:57
Add dnsManagementPolicy option to ingresscontrollers
9b30575 
Add published condition on DNSRecord
Deprecation of Failed DNS record condition
Bump api to include defaulting fix
Apply suggestions from code review

Co-authored-by: Miciah Dashiel Butler Masters <[email protected]>

Fix unmanaged dns test flake
Fix merging conditions during migration
Fix e2e tests timout due to operatorprogressing condition
6fe0d26
@thejasn thejasn force-pushed the feature/configurable-dns-lb branch from 0684b7b to 6fe0d26 Compare September 30, 2022 15:27
@Miciah
Copy link
Contributor

Miciah commented Sep 30, 2022

/lgtm
because the last push was only to rebase.

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

gcs278 commented Sep 30, 2022

/lgtm

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 30, 2022

/retest-required

Remaining retests: 0 against base HEAD dbefffe and 2 for PR HEAD 6fe0d26 in total

@thejasn
Copy link
Contributor Author

thejasn commented Sep 30, 2022

/test e2e-aws-operator

@thejasn
Copy link
Contributor Author

thejasn commented Sep 30, 2022

/test e2e-aws-operator
unrelated failure

        --- FAIL: TestAll/serial/TestRouterCompressionOperation (75.09s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference

@thejasn
Copy link
Contributor Author

thejasn commented Sep 30, 2022

/test e2e-aws-operator

6 similar comments
@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator

@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator

@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator

@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator

@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator

@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator

@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator
test failed while collecting must gather

@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator
failed during cluster installation due to AWS

@thejasn
Copy link
Contributor Author

thejasn commented Oct 1, 2022

/test e2e-aws-operator

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 1, 2022

@thejasn: all tests passed!

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-merge-robot openshift-merge-robot merged commit 5c36287 into openshift:master Oct 1, 2022
@Miciah Miciah mentioned this pull request Jan 29, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Miciah Miciah Miciah left review comments

@alebedev87 alebedev87 alebedev87 left review comments

@frobware frobware Awaiting requested review from frobware

+3 more reviewers

@rfredette rfredette rfredette left review comments

@candita candita candita left review comments

@gcs278 gcs278 gcs278 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@rfredette rfredette

@Miciah Miciah

@gcs278 gcs278

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.