Skip to content

Bug 1809354: dns: Avoid unnecessary updates #390

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

Merged
openshift-merge-robot merged 5 commits into from
May 27, 2020
Merged

Bug 1809354: dns: Avoid unnecessary updates #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d953fa9
dns: Avoid spuriously updating a DNSRecord's status
Miciah Apr 12, 2020
50a2a36
dns: Log success or failure to publish record
Miciah May 12, 2020
cc0ca51
Bump openshift/api for DNSRecord observedGeneration
Miciah May 12, 2020
f5016cc
dns: Avoid publishing already published DNSRecords
Miciah May 12, 2020
8cae0a4
dns/aws: Delete record caching
Miciah May 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ require (
github.com/google/go-cmp v0.3.1
github.com/kevinburke/go-bindata v3.11.0+incompatible

github.com/openshift/api v0.0.0-20200416222006-8cfd79130f9c
github.com/openshift/api v0.0.0-20200522173408-17ada6e4245b
github.com/openshift/library-go v0.0.0-20200324092245-db2a8546af81

github.com/pkg/errors v0.8.1
Expand All @@ -28,8 +28,8 @@ require (
gopkg.in/fsnotify.v1 v1.4.7
gopkg.in/yaml.v2 v2.2.8

k8s.io/api v0.18.0
k8s.io/apimachinery v0.18.0
k8s.io/api v0.18.3
k8s.io/apimachinery v0.18.3
k8s.io/apiserver v0.18.0
k8s.io/client-go v0.18.0

Expand Down
12 changes: 10 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,10 @@ github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQ
github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
github.com/opencontainers/runc v0.0.0-20191031171055-b133feaeeb2e/go.mod h1:qT5XzbpPznkRYVz/mWwUaVBUv2rmF59PVA73FjuZG0U=
github.com/openshift/api v0.0.0-20200320142426-0de0d539b0c3/go.mod h1:7k3+uZYOir97walbYUqApHUA2OPhkQpVJHt0n7GJ6P4=
github.com/openshift/api v0.0.0-20200416222006-8cfd79130f9c h1:lpLa1z9GCxcpHsfHq7ETFdOthnyy1PlD7mvDzVrenjc=
github.com/openshift/api v0.0.0-20200416222006-8cfd79130f9c/go.mod h1:RKMJ5CBnljLfnej+BJ/xnOWc3kZDvJUaIAEq2oKSPtE=
github.com/openshift/api v0.0.0-20200522173408-17ada6e4245b h1:yEY4zF6pEEWdR09wjuDJhupLEDM2vYdUgLoc10C0fFk=
github.com/openshift/api v0.0.0-20200522173408-17ada6e4245b/go.mod h1:TkhafijfTiRi1Q3120/ZSE4oIWKQ4DGRh3byPywv4Mw=
github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160/go.mod h1:1CkcsT3aVebzRBzVTSbiKSkJMsC/CASqxesfqEMfJEc=
github.com/openshift/build-machinery-go v0.0.0-20200424080330-082bf86082cc/go.mod h1:1CkcsT3aVebzRBzVTSbiKSkJMsC/CASqxesfqEMfJEc=
github.com/openshift/client-go v0.0.0-20200320150128-a906f3d8e723/go.mod h1:wNBSSt4RZTHhUWyhBE3gxTR32QpF9DB2SfS14u2IxuE=
github.com/openshift/library-go v0.0.0-20200324092245-db2a8546af81 h1:bNUcSdyoACkjI2USyvDbAMb6lCtghdz563b0bfhPC8A=
github.com/openshift/library-go v0.0.0-20200324092245-db2a8546af81/go.mod h1:Qc5duoXHzAKyUfA0REIlG/rdfWzknOpp9SiDiyg5Y7A=
Expand Down Expand Up @@ -525,6 +526,8 @@ k8s.io/api v0.18.0-rc.1 h1:hEUIbswR8BaacroH6OX/rjrK1kvEIUFZt3EpEar7/CM=
k8s.io/api v0.18.0-rc.1/go.mod h1:ZOh6SbHjOYyaMLlWmB2+UOQKEWDpCnVEVpEyt7S2J9s=
k8s.io/api v0.18.0 h1:lwYk8Vt7rsVTwjRU6pzEsa9YNhThbmbocQlKvNBB4EQ=
k8s.io/api v0.18.0/go.mod h1:q2HRQkfDzHMBZL9l/y9rH63PkQl4vae0xRT+8prbrK8=
k8s.io/api v0.18.3 h1:2AJaUQdgUZLoDZHrun21PW2Nx9+ll6cUzvn3IKhSIn0=
k8s.io/api v0.18.3/go.mod h1:UOaMwERbqJMfeeeHc8XJKawj4P9TgDRnViIqqBeH2QA=
k8s.io/apiextensions-apiserver v0.18.0-beta.2/go.mod h1:Hnrg5jx8/PbxRbUoqDGxtQkULjwx8FDW4WYJaKNK+fk=
k8s.io/apiextensions-apiserver v0.18.0-rc.1 h1:rhdw261gFouyOz8A8G0J1bhpOF2lLUF+LWv1o0d81E8=
k8s.io/apiextensions-apiserver v0.18.0-rc.1/go.mod h1:U5F8MzX2yXVHRuSKY5xBkS9999/VHu6PJNOInIpVLe4=
Expand All @@ -535,6 +538,8 @@ k8s.io/apimachinery v0.18.0-rc.1 h1:pYWMpBgJdEe1OacuI89D49JLcTG5ZLx9D7a1dGOdYh4=
k8s.io/apimachinery v0.18.0-rc.1/go.mod h1:9SnR/e11v5IbyPCGbvJViimtJ0SwHG4nfZFjU77ftcA=
k8s.io/apimachinery v0.18.0 h1:fuPfYpk3cs1Okp/515pAf0dNhL66+8zk8RLbSX+EgAE=
k8s.io/apimachinery v0.18.0/go.mod h1:9SnR/e11v5IbyPCGbvJViimtJ0SwHG4nfZFjU77ftcA=
k8s.io/apimachinery v0.18.3 h1:pOGcbVAhxADgUYnjS08EFXs9QMl8qaH5U4fr5LGUrSk=
k8s.io/apimachinery v0.18.3/go.mod h1:OaXp26zu/5J7p0f92ASynJa1pZo06YlV9fG7BoWbCko=
k8s.io/apiserver v0.18.0-beta.2/go.mod h1:bnblMkMoCFnIfVnVftd0SXJPzyvrk3RtaqSbblphF/A=
k8s.io/apiserver v0.18.0-rc.1 h1:P+tvb0Hum3tsyz6cF/mKJwXhDB/DMGhkkx+K7MQWj04=
k8s.io/apiserver v0.18.0-rc.1/go.mod h1:RYE9w2Lijk1aWW3i3pS7kFGU0Afof+UDoOz1qW9aSYg=
Expand All @@ -548,6 +553,7 @@ k8s.io/client-go v0.18.0/go.mod h1:uQSYDYs4WhVZ9i6AIoEZuwUggLVEF64HOD37boKAtF8=
k8s.io/code-generator v0.18.0-beta.2/go.mod h1:+UHX5rSbxmR8kzS+FAv7um6dtYrZokQvjHpDSYRVkTc=
k8s.io/code-generator v0.18.0-rc.1/go.mod h1:+UHX5rSbxmR8kzS+FAv7um6dtYrZokQvjHpDSYRVkTc=
k8s.io/code-generator v0.18.0/go.mod h1:+UHX5rSbxmR8kzS+FAv7um6dtYrZokQvjHpDSYRVkTc=
k8s.io/code-generator v0.18.3/go.mod h1:TgNEVx9hCyPGpdtCWA34olQYLkh3ok9ar7XfSsr8b6c=
k8s.io/component-base v0.18.0-beta.2/go.mod h1:HVk5FpRnyzQ/MjBr9//e/yEBjTVa2qjGXCTuUzcD7ks=
k8s.io/component-base v0.18.0-rc.1/go.mod h1:NNlRaxZEdLqTs2+6yXiU2SHl8gKsbcy19Ii+Sfq53RM=
k8s.io/component-base v0.18.0/go.mod h1:u3BCg0z1uskkzrnAKFzulmYaEpZF7XC9Pf/uFyb1v2c=
Expand All @@ -560,6 +566,8 @@ k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I=
k8s.io/kube-aggregator v0.18.0-beta.2/go.mod h1:O3Td9mheraINbLHH4pzoFP2gRzG0Wk1COqzdSL4rBPk=
k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c h1:/KUFqjjqAcY4Us6luF5RDNZ16KJtb49HfR3ZHB9qYXM=
k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c/go.mod h1:GRQhZsXIAJ1xR0C9bd8UpWHZ5plfAS9fzPjJuQ6JL3E=
k8s.io/kube-openapi v0.0.0-20200410145947-61e04a5be9a6 h1:Oh3Mzx5pJ+yIumsAD0MOECPVeXsVot0UkiaCGVyfGQY=
k8s.io/kube-openapi v0.0.0-20200410145947-61e04a5be9a6/go.mod h1:GRQhZsXIAJ1xR0C9bd8UpWHZ5plfAS9fzPjJuQ6JL3E=
k8s.io/utils v0.0.0-20200229041039-0a110f9eb7ab h1:I3f2hcBrepGRXI1z4sukzAb8w1R4eqbsHrAsx06LGYM=
k8s.io/utils v0.0.0-20200229041039-0a110f9eb7ab/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89 h1:d4vVOjXm687F1iLSP2q3lyPPuyvTUt3aVoBpi2DqRsU=
Expand Down
9 changes: 9 additions & 0 deletions manifests/00-custom-resource-definition-internal.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,15 @@ spec:
description: status is the most recently observed status of the dnsRecord.
type: object
properties:
observedGeneration:
description: observedGeneration is the most recently observed generation
of the DNSRecord. When the DNSRecord is updated, the controller updates
the corresponding record in each managed zone. If an update for a
particular zone fails, that failure is recorded in the status condition
for the zone so that the controller can determine that it needs to
retry the update for that specific zone.
type: integer
format: int64
zones:
description: zones are the status of the record in each zone.
type: array
Expand Down
33 changes: 7 additions & 26 deletions pkg/dns/aws/dns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/aws/aws-sdk-go/service/route53"

kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"

configv1 "github.com/openshift/api/config/v1"
)
Expand Down Expand Up @@ -56,12 +55,6 @@ type Provider struct {

// lbZones is a cache of load balancer DNS names to LB hosted zone IDs.
lbZones map[string]string

// updatedRecords is a cache of records which have been created or updated
// during the life of this manager. The key is zoneID+domain+target. This is a
// quick hack to minimize AWS API calls, and also prevent changes to existing
// records (something not yet supported).
updatedRecords sets.String
}

// Config is the necessary input to configure the manager.
Expand Down Expand Up @@ -119,13 +112,12 @@ func NewProvider(config Config, operatorReleaseVersion string) (*Provider, error
}

return &Provider{
elb: elb.New(sess, aws.NewConfig().WithRegion(region)),
route53: route53.New(sess, r53Config),
tags: resourcegroupstaggingapi.New(sess, tagConfig),
config: config,
idsToTags: map[string]map[string]string{},
lbZones: map[string]string{},
updatedRecords: sets.NewString(),
elb: elb.New(sess, aws.NewConfig().WithRegion(region)),
route53: route53.New(sess, r53Config),
tags: resourcegroupstaggingapi.New(sess, tagConfig),
config: config,
idsToTags: map[string]map[string]string{},
lbZones: map[string]string{},
}, nil
}

Expand Down Expand Up @@ -275,26 +267,15 @@ func (m *Provider) change(record *iov1.DNSRecord, zone configv1.DNSZone, action
return fmt.Errorf("failed to get hosted zone for load balancer target %q: %v", target, err)
}

// Configure records and cache updates.
// TODO: handle the caching/diff detection in a better way.
m.lock.Lock()
defer m.lock.Unlock()
key := zoneID + domain + target
// Only process updates once for now because we're not diffing.
if m.updatedRecords.Has(key) && action == upsertAction {
log.V(2).Info("skipping cached DNS record update", "record", record)
return nil
}
// Configure records.
err = m.updateAlias(domain, zoneID, target, targetHostedZoneID, string(action))
if err != nil {
return fmt.Errorf("failed to update alias in zone %s: %v", zoneID, err)
}
switch action {
case upsertAction:
m.updatedRecords.Insert(key)
log.Info("upserted DNS record", "record", record)
case deleteAction:
m.updatedRecords.Delete(key)
log.Info("deleted DNS record", "record", record)
}
return nil
Expand Down
Loading