refactor(exoscale): migrate provider to egoscale v3#6371
Open
natalie-o-perret wants to merge 1 commit intokubernetes-sigs:masterfrom
Open
refactor(exoscale): migrate provider to egoscale v3#6371natalie-o-perret wants to merge 1 commit intokubernetes-sigs:masterfrom
natalie-o-perret wants to merge 1 commit intokubernetes-sigs:masterfrom
Conversation
k8s-ci-robot
added
the
do-not-merge/work-in-progress
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
k8s-ci-robot
added
the
needs-ok-to-test
Contributor
|
Hi @natalie-o-perret. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
k8s-ci-robot
added
cncf-cla: no
natalie-o-perret
force-pushed
the
refactor/exoscale-provider-v3
branch
from
6a10551 to
8a5880d
Compare
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
Member
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
k8s-ci-robot
added
the
do-not-merge/work-in-progress
natalie-o-perret
force-pushed
the
refactor/exoscale-provider-v3
branch
from
8a5880d to
fc9d858
Compare
Coverage Report for CI Build 24484425352Coverage decreased (-0.03%) to 80.49%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions52 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
Member
|
Could you share similar results for this PR #5085 (comment). Need to make sure it works |
Member
ivankatliarchuk
left a comment
ivankatliarchuk
left a comment
There was a problem hiding this comment.
worth to execute make cover-html
And cover with unit tests, modified lines
|
|
||
| record.Type = &epoint.RecordType | ||
| record.Content = &epoint.Targets[0] | ||
| req := v3.UpdateDNSDomainRecordRequest{ |
Member
There was a problem hiding this comment.
Only Content and Ttl are updated. The original code set record.Type = &epoint.RecordType. How the type is handled?
Author
There was a problem hiding this comment.
The v3 UpdateDNSDomainRecordRequest struct does not have a Type field, it only exposes Content, Name, Priority, and Ttl:
type UpdateDNSDomainRecordRequest struct {
Content string `json:"content,omitempty"`
Name string `json:"name,omitempty"`
Priority int64 `json:"priority,omitempty"`
Ttl int64 `json:"ttl,omitempty"`
}DNS record types are immutable on the Exoscale API (you cannot change an A record into a CNAME. You'd delete and recreate).
So the old v2 code setting record.Type = &epoint.RecordType was effectively a no-op at the API level.
To make sure we match the correct record during update/delete, the loop now includes a type-filter guard:
if string(record.Type) != epoint.RecordType {
continue
}This prevents updating/deleting a record that has the same name but a different type (e.g., matching a TXT record when the endpoint is an A record).
Contributor
|
What if we build a webhook and move this out of tree instead? |
…, GetDomainFilter and apex record fix
natalie-o-perret
force-pushed
the
refactor/exoscale-provider-v3
branch
from
fc9d858 to
f33fc10
Compare
Author
Tested the v3-migrated provider against a live Exoscale DNS zone. The Go test imports and calls A Python wrapper cross-checks each step with Zone name and record UUIDs are redacted. Go integration test source (main.go)package main
import (
"context"
"fmt"
"os"
"time"
v3 "github.com/exoscale/egoscale/v3"
"github.com/exoscale/egoscale/v3/credentials"
"sigs.k8s.io/external-dns/endpoint"
"sigs.k8s.io/external-dns/plan"
exoscale "sigs.k8s.io/external-dns/provider/exoscale"
)
func main() {
zone := os.Getenv("TEST_ZONE")
key := os.Getenv("TEST_KEY")
secret := os.Getenv("TEST_SECRET")
apizone := os.Getenv("TEST_APIZONE")
if apizone == "" {
apizone = "ch-gva-2"
}
if zone == "" || key == "" || secret == "" {
fmt.Fprintln(os.Stderr, "set TEST_ZONE, TEST_KEY, TEST_SECRET")
os.Exit(1)
}
creds := credentials.NewStaticCredentials(key, secret)
ep := v3.Endpoint(fmt.Sprintf("https://api-%s.exoscale.com/v2", apizone))
client, err := v3.NewClient(creds, v3.ClientOptWithEndpoint(ep))
if err != nil {
panic(err)
}
provider := exoscale.NewExoscaleProviderWithClient(
&liveClient{c: client},
false,
0,
exoscale.ExoscaleWithDomain(endpoint.NewDomainFilter([]string{zone})),
)
ctx := context.Background()
// Step 1: BEFORE
fmt.Println("=== STEP 1: Records() BEFORE ===")
printRecords(provider, ctx)
// Step 2: CREATE apex A record
fmt.Printf("\n=== STEP 2: CREATE apex A record (%s -> 203.0.113.42) ===\n", zone)
err = provider.ApplyChanges(ctx, &plan.Changes{
Create: []*endpoint.Endpoint{
endpoint.NewEndpoint(zone, endpoint.RecordTypeA, "203.0.113.42"),
},
})
if err != nil {
panic(fmt.Sprintf("CREATE failed: %v", err))
}
fmt.Println("OK")
time.Sleep(2 * time.Second)
// Step 3: Records() AFTER CREATE
fmt.Println("\n=== STEP 3: Records() AFTER CREATE ===")
printRecords(provider, ctx)
// Step 4: UPDATE apex A record to new IP
fmt.Printf("\n=== STEP 4: UPDATE apex A record (%s -> 198.51.100.1) ===\n", zone)
err = provider.ApplyChanges(ctx, &plan.Changes{
UpdateOld: []*endpoint.Endpoint{
endpoint.NewEndpoint(zone, endpoint.RecordTypeA, "203.0.113.42"),
},
UpdateNew: []*endpoint.Endpoint{
endpoint.NewEndpoint(zone, endpoint.RecordTypeA, "198.51.100.1"),
},
})
if err != nil {
panic(fmt.Sprintf("UPDATE failed: %v", err))
}
fmt.Println("OK")
time.Sleep(2 * time.Second)
// Step 5: Records() AFTER UPDATE
fmt.Println("\n=== STEP 5: Records() AFTER UPDATE ===")
printRecords(provider, ctx)
// Step 6: DELETE apex A record
fmt.Printf("\n=== STEP 6: DELETE apex A record ===\n")
err = provider.ApplyChanges(ctx, &plan.Changes{
Delete: []*endpoint.Endpoint{
endpoint.NewEndpoint(zone, endpoint.RecordTypeA, "198.51.100.1"),
},
})
if err != nil {
panic(fmt.Sprintf("DELETE failed: %v", err))
}
fmt.Println("OK")
// Step 7: Records() FINAL
fmt.Println("\n=== STEP 7: Records() FINAL (should be empty) ===")
printRecords(provider, ctx)
fmt.Println("\n=== ALL STEPS PASSED ===")
}
func printRecords(p *exoscale.ExoscaleProvider, ctx context.Context) {
recs, err := p.Records(ctx)
if err != nil {
panic(err)
}
if len(recs) == 0 {
fmt.Println(" (no managed records)")
}
for _, r := range recs {
fmt.Printf(" %s %s %v TTL=%d\n", r.DNSName, r.RecordType, r.Targets, r.RecordTTL)
}
}
// liveClient wraps the real v3.Client to satisfy the EgoscaleClientI interface.
type liveClient struct{ c *v3.Client }
func (w *liveClient) ListDNSDomains(ctx context.Context) ([]v3.DNSDomain, error) {
resp, err := w.c.ListDNSDomains(ctx)
if err != nil {
return nil, err
}
return resp.DNSDomains, nil
}
func (w *liveClient) ListDNSDomainRecords(ctx context.Context, id v3.UUID) ([]v3.DNSDomainRecord, error) {
resp, err := w.c.ListDNSDomainRecords(ctx, id)
if err != nil {
return nil, err
}
return resp.DNSDomainRecords, nil
}
func (w *liveClient) CreateDNSDomainRecord(ctx context.Context, id v3.UUID, req v3.CreateDNSDomainRecordRequest) error {
op, err := w.c.CreateDNSDomainRecord(ctx, id, req)
if err != nil {
return err
}
_, err = w.c.Wait(ctx, op, v3.OperationStateSuccess)
return err
}
func (w *liveClient) DeleteDNSDomainRecord(ctx context.Context, domainID v3.UUID, recordID v3.UUID) error {
op, err := w.c.DeleteDNSDomainRecord(ctx, domainID, recordID)
if err != nil {
return err
}
_, err = w.c.Wait(ctx, op, v3.OperationStateSuccess)
return err
}
func (w *liveClient) UpdateDNSDomainRecord(ctx context.Context, domainID v3.UUID, recordID v3.UUID, req v3.UpdateDNSDomainRecordRequest) error {
op, err := w.c.UpdateDNSDomainRecord(ctx, domainID, recordID, req)
if err != nil {
return err
}
_, err = w.c.Wait(ctx, op, v3.OperationStateSuccess)
return err
}Python orchestrator (run_e2e.py)#!/usr/bin/env python3
"""
Live E2E test for external-dns Exoscale provider: DNS record lifecycle.
Runs the Go integration test through the actual provider code (ApplyChanges / Records),
then cross-checks each step with `exo dns show` and `dig`.
Output is formatted as a markdown comment suitable for pasting on the PR.
Usage:
export TEST_ZONE="<your-test-zone>"
export TEST_KEY="<exoscale-api-key>"
export TEST_SECRET="<exoscale-api-secret>"
python3 run_e2e.py [--redact] [--mask-ids]
"""
import argparse
import os
import re
import subprocess
import sys
import time
ZONE = os.environ.get("TEST_ZONE", "")
KEY = os.environ.get("TEST_KEY", "")
SECRET = os.environ.get("TEST_SECRET", "")
APIZONE = os.environ.get("TEST_APIZONE", "ch-gva-2")
GO_TEST_DIR = os.path.dirname(os.path.abspath(__file__))
def run(cmd, *, check=True, timeout=60):
"""Run a subprocess, return stdout."""
r = subprocess.run(cmd, capture_output=True, text=True, timeout=timeout)
if check and r.returncode != 0:
print(f"COMMAND FAILED: {' '.join(cmd)}", file=sys.stderr)
print(r.stdout, file=sys.stderr)
print(r.stderr, file=sys.stderr)
sys.exit(1)
return r.stdout.strip()
def exo_dns_show():
"""Return `exo dns show <zone>` output (table of all records)."""
return run(["exo", "dns", "show", ZONE])
def dig_apex(rtype="A"):
"""Query Exoscale authoritative NS for the apex record."""
try:
out = run(
["dig", f"@ns1.exoscale.com", ZONE, rtype, "+short"],
check=False,
timeout=10,
)
return out if out else "(empty)"
except FileNotFoundError:
return "(dig not available)"
def banner(title):
return f"\n### {title}\n"
def code_block(content, lang=""):
return f"```{lang}\n{content}\n```"
def redact_output(text, zone, mask_ids=False):
"""Replace the real zone name and optionally mask UUIDs."""
text = text.replace(zone, "<redacted>.net")
if mask_ids:
text = re.sub(
r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}",
"XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX",
text,
)
return text
def main():
parser = argparse.ArgumentParser()
parser.add_argument("--redact", action="store_true", help="Redact zone name in output")
parser.add_argument("--mask-ids", action="store_true", help="Mask record UUIDs with XX")
args = parser.parse_args()
if not ZONE or not KEY or not SECRET:
print("ERROR: set TEST_ZONE, TEST_KEY, TEST_SECRET env vars", file=sys.stderr)
sys.exit(1)
output_parts = []
output_parts.append("## Live E2E test: Exoscale provider (egoscale v3)\n")
output_parts.append(
"This test exercises the v3-migrated provider code (`ApplyChanges` / `Records`) "
"against a real Exoscale DNS zone, covering CREATE, UPDATE, and DELETE "
"of an A record.\n"
)
# Step 1: Verify clean state
output_parts.append(banner("Step 1: BEFORE (clean zone, no managed records)"))
output_parts.append("**`exo dns show`**")
output_parts.append(code_block(exo_dns_show()))
output_parts.append(f"**`dig @ns1.exoscale.com {ZONE} A`**")
output_parts.append(code_block(dig_apex()))
# Steps 2-7: Run Go integration test
output_parts.append(banner("Steps 2-7: Provider lifecycle (Go integration test)"))
output_parts.append(
"Calls `NewExoscaleProviderWithClient` with a live `egoscale/v3` API client, "
"then runs CREATE => Records() => UPDATE => Records() => DELETE => Records()."
)
env = {**os.environ, "TEST_ZONE": ZONE, "TEST_KEY": KEY,
"TEST_SECRET": SECRET, "TEST_APIZONE": APIZONE}
go_result = subprocess.run(
["go", "run", "."], capture_output=True, text=True,
timeout=120, cwd=GO_TEST_DIR, env=env,
)
go_out = "\n".join(
line for line in (go_result.stdout + go_result.stderr).splitlines()
if not line.startswith("INFO[") and not line.startswith("time=")
).strip()
if go_result.returncode != 0:
output_parts.append(code_block(go_out))
output_parts.append("\n**FAILED**: see output above.\n")
result = "\n".join(output_parts)
if args.redact:
result = redact_output(result, ZONE, mask_ids=args.mask_ids)
print(result)
sys.exit(1)
output_parts.append(code_block(go_out))
# Cross-check: re-create for dig verification
output_parts.append(banner("Cross-check: `dig` after provider CREATE"))
output_parts.append("Re-creating the record to verify via authoritative DNS query:")
run(["exo", "dns", "add", "A", ZONE, "-n", "", "-a", "203.0.113.42"])
time.sleep(2)
output_parts.append(f"\n**`dig @ns1.exoscale.com {ZONE} A +short`**")
output_parts.append(code_block(dig_apex()))
output_parts.append(f"\n**`exo dns show`** (A record visible):")
output_parts.append(code_block(exo_dns_show()))
# Cleanup
show_output = run(["exo", "dns", "show", ZONE])
for line in show_output.splitlines():
if "203.0.113.42" in line:
record_id = line.split("|")[1].strip() if "|" in line else ""
if record_id:
run(["exo", "dns", "remove", "-f", ZONE, record_id])
break
output_parts.append(banner("FINAL (zone clean after test)"))
output_parts.append(code_block(exo_dns_show()))
# Result
output_parts.append(banner("Result"))
output_parts.append(
"All operations completed successfully. The v3-migrated provider correctly "
"handles the full CREATE -> READ -> UPDATE -> DELETE lifecycle "
"against a live Exoscale DNS zone."
)
result = "\n".join(output_parts)
if args.redact:
result = redact_output(result, ZONE, mask_ids=args.mask_ids)
print(result)
if __name__ == "__main__":
main()Step 1: BEFORE (clean zone, no managed records)
Steps 2-7: Provider lifecycle (Go integration test)Calls Cross-check:
|
Author
That is definitely something we can look into. Tbs, Moving to a webhook-based out-of-tree provider would be a bigger scope change. We can discuss that as a separate effort. For now, this PR focuses on unblocking the |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
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
5 participants
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.
Relates to:
Changes:
egoscale v2->egoscale/v3Wait()internally--exoscale-zones-cache-durationflag (mirrors the AWS/OCI pattern, defaults to0s= disabled) https://github.com/kubernetes-sigs/external-dns/tree/master/provider/blueprintGetDomainFilter()returning zones from the API (with bare and subdomain-matching variants) feat(pdns): support GetDomainFilter interface #6234EndpointZoneIDandRecords()FQDN building fixed for bare zone names (same bugs as fix(exoscale): handle apex DNS records #6369, re-applied on this branch which was cut from master)