Add RecordSet controller for Designate DNS record support - #957
Open
zunken1337 wants to merge 2 commits into
Open
zunken1337 wants to merge 2 commits into
zunken1337 wants to merge 2 commits into
Conversation
First of three Designate controllers split out of a single oversized PR (zone, then DNSZoneShare and RecordSet as separate follow-up PRs) at mandre's request - ~16k LOC in one PR was too large to review. DNSZone covers zone CRUD (PRIMARY/SECONDARY, AXFR masters, serial, transferredAt) against a new shared osclients.DNSClient, following the same "one OpenStack service, one client" convention NetworkClient already established for Neutron. DNSZoneShare and RecordSet will each add their own methods to this same client in their own PRs, since all three are Designate resources on one ServiceClient. Credit: this picks up the gap left by @eshulman2's stalled k-orc#825 (zone CRUD only, no project-ownership transfer, no RecordSet) with a fresh implementation built via the current scaffolding, rather than rebasing k-orc#825 through months of generic-controller-framework changes - the path mandre suggested. k-orc#825 gets closed/superseded by this series; eshulman2's original design exploration there is gratefully acknowledged and is what first surfaced several of the real gaps this PR fixes (a projectID-in-status field that was missing entirely, and Masters typed as unvalidated []string rather than a real IP type). Full test coverage: unit test for the one mutable-field update path (description/ttl/masters, masters compared as a set not positionally), the full test/apivalidations envtest/CEL suite, and all 5 required KUTTL e2e scenarios. Verified via `make generate`, `make lint` (0 issues), and `make test` (full suite, zero regressions) on real hardware before pushing - gofmt specifically checked byte-for-byte after a prior session found it can corrupt an empty-string CEL comparison into a smart quote; avoided here by using has() instead, which also matches every other CEL rule already in this codebase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Joakim Johansson <joakimjohansson@cleura.nu>
Second of three Designate controllers split out of a single oversized PR at mandre's request (DNSZone, then this, then DNSZoneShare) - stacked on new-controller-dnszone since every RecordSet operation is scoped under its owning DNSZone. Extends the shared osclients.DNSClient (added in the DNSZone PR) with List/Create/Delete/Get/UpdateRecordSet. RecordSet's zoneRef is resolved once per actuator construction and reused across every method call, since Designate's recordset API requires the zone ID alongside the recordset's own ID for every operation (including Get) - a single-ID GetOSResourceByID can't carry both, so the zone is resolved once at actuator-construction time instead of per call. Full test coverage: unit test for the one mutable-field update path (description/ttl/records, records compared as a set since Designate doesn't guarantee return order), the full test/apivalidations envtest/CEL suite, and all 6 required KUTTL e2e scenarios - including recordset-dependency and recordset-import, which need the recordset's name to be an actual subdomain of its zone (Designate rejects "invalid_recordset_location" otherwise) and a zoneRef in the import filter (Designate's recordset list API is zone-scoped, so importing needs a zone reference even when unmanaged - same dual zoneRef/ zoneImportRef dependency pattern ApplicationCredential already uses for userRef). Verified via `make generate`, `make lint` (0 issues), and `make test` (full suite, zero regressions) on real hardware before pushing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Joakim Johansson <joakimjohansson@cleura.nu>
This was referenced Oct 2, 2026
This was referenced Oct 2, 2026
Contributor
Author
|
The `gazpacho` e2e failure is a known timing flake in `recordset-create-full`, not a real issue - I've now seen this exact same single test fail on this exact flavor three separate times across this work (twice in the predecessor combined PR #955, now here), always for the same reason: the resource genuinely reconciles successfully, just not within KUTTL's 240s step timeout under concurrent CI load. From this run's collected artifacts (`orc-managed-resources/openstack.k-orc.cloud_recordsets.yaml`), the object was created at `06:27:08Z` and reached `Available: True` / `Success` at `06:27:19Z` - 11 seconds later, well inside a normal reconcile window. The other two devstack flavors (epoxy, flamingo) and every other check pass clean on this PR. No code change made for this - didn't want to paper over a real timing issue with a longer timeout without more data, but flagging it here in case it's useful signal (e.g. if `gazpacho`, the newest OpenStack version flavor, runs under tighter resource constraints in CI than the other two). |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second of three Designate controllers split out of #955 at @mandre's request
(#955 (comment)). Adds
RecordSet(individual DNS records within a zone). Stacked on #956 (DNSZone) - everyRecordSetoperation is scoped under its owning zone, and this PR extends the sharedosclients.DNSClient#956 introduces. Please merge #956 first; this diff will shrink to justthe
RecordSet-specific changes once that happens.Architecture notes
osclients.DNSClient(from Add DNSZone controller for Designate DNS zone support #956) withList/Create/Delete/Get/UpdateRecordSet.RecordSetis a zone-scoped sub-resource in Designate's own API (Get/Create/Delete/Updateall require the zone ID alongside the recordset's own ID), which doesn't fit thegeneric actuator interfaces' single-ID
GetOSResourceByIDsignature directly. Resolved byhaving the actuator resolve its owning zone once at construction time and reuse that across
every method call on the same instance, rather than re-resolving per call.
RecordSet.spec.resource.namemust be explicit and end with a period, same reasoning asDNSZone.spec.resource.namein Add DNSZone controller for Designate DNS zone support #956.RecordSet(unmanaged +import.filter) needs azoneRefin the filter itself,since Designate's recordset list API is zone-scoped and there's no "list across all zones"
endpoint -
RecordSetFiltergained a requiredzoneReffield, resolved via the same dualDependency/*ImportDependencypatternApplicationCredentialalready uses foruserRef/userImportDependency.Testing
a set since Designate doesn't guarantee return order).
test/apivalidationssuite extended with immutability and required-field CEL validationtests.
recordset-dependencyandrecordset-import,which specifically need the recordset's name to be an actual subdomain of its zone (Designate
rejects
invalid_recordset_locationotherwise - a real mistake caught and fixed during thiswork, not just a hypothetical).
Test plan
go build ./...,go vet ./...,gofmt -l .cleango test ./...passes (only unrelatedtest/e2e, which needs a live kind cluster, fails inthis environment)
make generateoutput committedmake lintclean (0 issues)🤖 Generated with Claude Code