Repository navigation
feat(placement): switch MC/DNS discovery to mc-registration secrets + injected domains - #88
Conversation
… injected domains
- DynamicSelector: discover eligible MCs via mc-registration-* SM secrets
(label mc-registration=true, JSON {projectId,mode}), filter mode=active.
Drop Maestro consumer API dependency entirely.
- DynamicSelector: DNS domains now injected at construction (Helm value ->
--hc-dns-domains flag) instead of reading the argocd-cluster secret,
since domains are a region-level resource shared across MCs.
- Remove Candidate struct, Selector interface, RoundRobinSelector -
DynamicSelector is now the sole selector implementation.
- cmd: drop --candidates/--base-domains/--maestro-http-addr flags, add
--hc-dns-domains, require --secretmanager-project.
- helm: drop candidates/baseDomains/maestro values, add hcDnsDomains,
require both secretManagerProject and hcDnsDomains in validateValues.
Implements docs/mc-registration-plan.md and
docs/gecko-placement-dns-zones-plan.md.
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patjlm 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 |
F1: eligibleMCs skips broken secrets (log+continue) instead of failing all placement F2: reconciler test asserts Status().Update() called on happy path F3: 30s TTL cache on eligible MC list to reduce SM RPCs under burst F4: render --hc-dns-domains unconditionally in Helm deployment F6: re-add Selector interface, decouple Reconciler from DynamicSelector F7: remove unreachable len(domains)==0 guard and its test F8: add TestParseDomains for cmd package F9: SM label filter uses = (equality) instead of : (containment)
|
/retest |
|
/test ci/prow/test |
|
@patjlm: The specified target(s) for Use 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-sigs/prow repository. |
| if v := envOr("MAESTRO_HTTP_ADDR", ""); v != "" && !cmd.Flags().Changed("maestro-http-addr") { | ||
| maestroHTTPAddr = v | ||
| if smProject == "" { | ||
| return fmt.Errorf("--secretmanager-project is required (or $SECRETMANAGER_PROJECT)") |
There was a problem hiding this comment.
@cristianoveiga i realize this may make more complex the local runs of this controller. let me know if you want me to reinstate the mc list flag.. but then we also need to add other params like maintenance...
alternatively, we go the externalsecret route. and on local deployment we can create our own secret?
There was a problem hiding this comment.
I like the idea of having the candidates list (either via a flag or a configmap/secret), but I think we can deal with that separately (in a future PR)
|
/lgtm |
5925229
into
openshift-online:main
MC name and GCP project ID are interchangeable per team confirmation, matching the mc-registration Secret Manager secret convention from PR openshift-online#88. Removes the mcProjects map and --mc-projects flag; targetCluster is used directly as the GCP project ID when creating Firestore clients. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MC name and GCP project ID are interchangeable per team confirmation, matching the mc-registration Secret Manager secret convention from PR openshift-online#88. Removes the mcProjects map and --mc-projects flag; targetCluster is used directly as the GCP project ID when creating Firestore clients. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Switches
gecko-placementMC discovery from the Maestro consumers API +argocd-clusterDNS lookup to per-MCmc-registration-*Secret Managersecrets and an injected
--hc-dns-domainsflag.DynamicSelectordiscovers eligible MCs by listingmc-registration-*SecretManager secrets (label
mc-registration=true), reading each secret's JSONpayload (
{"projectId": "...", "mode": "active|maintenance"}), and filteringto
mode == "active". Individual broken secrets are logged and skipped —placement continues as long as at least one healthy MC exists. Eligible MC
results are cached (30s TTL) to reduce Secret Manager RPCs under burst
reconciliations.
(Note:
maestro-consumer-namelabel and Maestro consumers API are still usedby
hyperfleet-placement-adapterfor the CLS/legacy cluster flow.)argocd-clustersecret.They're injected at construction time via a
--hc-dns-domainscomma-separated flag (Helm value
hcDnsDomains), since domains are aregion-level resource shared across all MCs, not per-MC.
Selectorinterface to keep reconciler testableindependently of Secret Manager wiring.
cmd/placement: dropped--candidates/--base-domains/--maestro-http-addrflags, added--hc-dns-domains,--secretmanager-projectis now required.
gecko-placement): droppedcandidates/baseDomains/maestrovalues, addedhcDnsDomains;validateValuesnow requires bothsecretManagerProjectandhcDnsDomains.Related
— creates the per-MC
mc-registration-*secrets, conditions thegecko-placementGSA's IAM to those secrets only, and wireshcDnsDomainsinto the ArgoCD Application template.Testing
go build/vet/test ./...incontrollersmodule — all passhelm lint/helm templatewith required values — renders correctly,fails fast without
secretManagerProject/hcDnsDomains