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
[release-4.5] Bug 1868257: dns: Reread cloud credentials secret if it changes #443
[release-4.5] Bug 1868257: dns: Reread cloud credentials secret if it changes #443
Conversation
Improve the logging in the DNS controller and DNS provider implementations to improve visibility into the controller's operation and to avoid printing old and possibly misleading status during reconciliation. * pkg/dns/aws/dns.go (change): * pkg/dns/azure/dns.go (Ensure, Delete): Log the dnsrecord's spec but not its status because the controller only updates the status after ensuring or deleting the dnsrecord. Log the zone along with the dnsrecord's spec. * pkg/operator/controller/dns/controller.go (Reconcile): Log when beginning reconciliation. When updating the status, log the updated dnsrecord rather than the old dnsrecord (with the old status), and always log when updating (not only when the update fails). (publishRecordToZones): Log when skipping a zone. Log the dnsrecord's spec but not its status because the controller only updates the status after publishRecordToZones returns.
* cmd/ingress-operator/start.go (NewStartCommand): Delete initialization of the DNS provider. (cloudCredentialsSecretName, createDNSProvider, getPlatformStatus): Move from here... * pkg/operator/controller/dns/controller.go: ...to here. (New): Delete parameter for DNS provider; the controller now initializes it. Add config parameter. Add watches on the DNS config and Infrastructure config objects. Use ToDNSRecords mapper function to reconcile all DNSRecord objects when a config object changes. (Config): New type to hold the namespace and operator release version, which are needed for initializing the DNS provider. (reconciler): Add Config and infraConfig fields. (Reconcile): Get the cluster Infrastructure config object, and initialize or re-initialize the DNS provider if it was not already initialized or if the Infrastructure config has changed. (ToDNSRecords): New method. Return reconcilation requests for all DNSRecords. * pkg/operator/operator.go (New): Update initialization of the DNS controller to pass the controller's config object instead of the DNS provider object. * manifests/00-cluster-role.yaml: Allow the operator to list and watch the dnses and infrastructures config resources. * pkg/manifests/bindata.go: Regenerate.
Watch the cloud credentials secret and recreate the DNS provider if the credentials change. https://bugzilla.redhat.com/show_bug.cgi?id=1854383 This commit fixes bug 1854383. * pkg/operator/controller/dns/controller.go (New): Watch the cloud credentials secret and trigger reconciliation if it changes. (reconciler): Add cloudCredentials field. (Reconcile): Refactor DNS provider create logic from here... (createDNSProviderIfNeeded): ...to here. Check if the cloud credentials secret data changed in addition to checking the infrastructure status to determine whether the DNS provider needs to be recreated.
@Miciah: This pull request references Bugzilla bug 1868257, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 6 validation(s) were run on this bug
In 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-aws-upgrade |
1 similar comment
/test e2e-aws-upgrade |
/test e2e-aws-upgrade |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Miciah, sgreene570 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
6 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@Miciah: All pull requests linked via external trackers have merged: Bugzilla bug 1868257 has been moved to the MODIFIED state. In 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. |
Manual cherry-pick of #417, #421, and #425.