From 271367ba0f83260cd5cd7dc018063ff2d74806fd Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Sun, 8 Jan 2017 22:36:47 -0500 Subject: [PATCH] Don't add DNSZone task twice --- pkg/model/dns.go | 86 +++++++++++++++++++++------------------------ pkg/model/names.go | 7 +++- upup/pkg/fi/task.go | 25 +++++++++++++ 3 files changed, 72 insertions(+), 46 deletions(-) diff --git a/pkg/model/dns.go b/pkg/model/dns.go index 8f3b45eb58..feb91334b9 100644 --- a/pkg/model/dns.go +++ b/pkg/model/dns.go @@ -17,6 +17,8 @@ limitations under the License. package model import ( + "fmt" + "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/awstasks" "strings" @@ -29,56 +31,60 @@ type DNSModelBuilder struct { var _ fi.ModelBuilder = &DNSModelBuilder{} +func (b *DNSModelBuilder) ensureDNSZone(c *fi.ModelBuilderContext) error { + // Configuration for a DNS zone + dnsZone := &awstasks.DNSZone{ + Name: s(b.NameForDNSZone()), + } + + topology := b.Cluster.Spec.Topology + if topology != nil && topology.DNS != nil { + switch topology.DNS.Type { + case kops.DNSTypePublic: + // Ignore + + case kops.DNSTypePrivate: + dnsZone.Private = fi.Bool(true) + dnsZone.PrivateVPC = b.LinkToVPC() + + default: + return fmt.Errorf("Unknown DNS type %q", topology.DNS.Type) + } + } + + if !strings.Contains(b.Cluster.Spec.DNSZone, ".") { + // Looks like a hosted zone ID + dnsZone.ZoneID = s(b.Cluster.Spec.DNSZone) + } else { + // Looks like a normal DNS name + dnsZone.DNSName = s(b.Cluster.Spec.DNSZone) + } + + return c.EnsureTask(dnsZone) +} + func (b *DNSModelBuilder) Build(c *fi.ModelBuilderContext) error { // Add a HostedZone if we are going to publish a dns record that depends on it if b.UsePrivateDNS() { - // UsePrivateDNS is only exposed as a feature flag currently - // TODO: We may still need a public zone to publish an ELB - // Check to see if we are using a bastion DNS record that points to the hosted zone // If we are, we need to make sure we include the hosted zone as a task - // Configuration for a DNS zone, attached to our VPC - dnsZone := &awstasks.DNSZone{ - Name: s("private-" + b.Cluster.Spec.DNSZone), - Private: fi.Bool(true), - PrivateVPC: b.LinkToVPC(), + if err := b.ensureDNSZone(c); err != nil { + return err } - - if !strings.Contains(b.Cluster.Spec.DNSZone, ".") { - // Looks like a hosted zone ID - dnsZone.ZoneID = s(b.Cluster.Spec.DNSZone) - } else { - // Looks like a normal ddns name - dnsZone.DNSName = s(b.Cluster.Spec.DNSZone) - } - - c.AddTask(dnsZone) } if b.UseLoadBalancerForAPI() { // This will point our DNS to the load balancer, and put the pieces // together for kubectl to be work - // Configuration for a DNS name for the master - dnsZone := &awstasks.DNSZone{ - Name: s(b.Cluster.Spec.DNSZone), - Private: fi.Bool(false), + if err := b.ensureDNSZone(c); err != nil { + return err } - if !strings.Contains(b.Cluster.Spec.DNSZone, ".") { - // Looks like a hosted zone ID - dnsZone.ZoneID = s(b.Cluster.Spec.DNSZone) - } else { - // Looks like a normal ddns name - dnsZone.DNSName = s(b.Cluster.Spec.DNSZone) - } - - c.AddTask(dnsZone) - apiDnsName := &awstasks.DNSName{ Name: s(b.Cluster.Spec.MasterPublicName), - Zone: &awstasks.DNSZone{Name: s(b.Cluster.Spec.DNSZone)}, + Zone: b.LinkToDNSZone(), ResourceType: s("A"), TargetLoadBalancer: b.LinkToELB("api"), } @@ -89,20 +95,10 @@ func (b *DNSModelBuilder) Build(c *fi.ModelBuilderContext) error { // Pulling this down into it's own if statement. The DNS configuration here // is similar to others, but I would like to keep it on it's own in case we need // to change anything. - dnsZone := &awstasks.DNSZone{ - Name: s(b.Cluster.Spec.DNSZone), - Private: fi.Bool(false), - } - if !strings.Contains(b.Cluster.Spec.DNSZone, ".") { - // Looks like a hosted zone ID - dnsZone.ZoneID = s(b.Cluster.Spec.DNSZone) - } else { - // Looks like a normal ddns name - dnsZone.DNSName = s(b.Cluster.Spec.DNSZone) + if err := b.ensureDNSZone(c); err != nil { + return err } - - c.AddTask(dnsZone) } return nil diff --git a/pkg/model/names.go b/pkg/model/names.go index b3fea3a6b3..4b1a3916dc 100644 --- a/pkg/model/names.go +++ b/pkg/model/names.go @@ -90,10 +90,15 @@ func (b *KopsModelContext) LinkToVPC() *awstasks.VPC { } func (b *KopsModelContext) LinkToDNSZone() *awstasks.DNSZone { - name := b.Cluster.Spec.DNSZone + name := b.NameForDNSZone() return &awstasks.DNSZone{Name: &name} } +func (b *KopsModelContext) NameForDNSZone() string { + name := b.Cluster.Spec.DNSZone + return name +} + func (b *KopsModelContext) IAMName(role kops.InstanceGroupRole) string { var name string diff --git a/upup/pkg/fi/task.go b/upup/pkg/fi/task.go index 57a36a6e2e..9a25514db4 100644 --- a/upup/pkg/fi/task.go +++ b/upup/pkg/fi/task.go @@ -19,6 +19,7 @@ package fi import ( "fmt" "github.com/golang/glog" + "reflect" "strings" ) @@ -57,6 +58,30 @@ func (c *ModelBuilderContext) AddTask(task Task) { c.Tasks[key] = task } +// EnsureTask ensures that the specified task is configured. +// It adds the task if it does not already exist. +// If it does exist, it verifies that the existing task reflect.DeepEqual the new task, +// if they are different an error is returned. +func (c *ModelBuilderContext) EnsureTask(task Task) error { + key := buildTaskKey(task) + + existing, found := c.Tasks[key] + if found { + if reflect.DeepEqual(task, existing) { + glog.V(8).Infof("EnsureTask ignoring identical ") + return nil + } else { + glog.Warningf("EnsureTask found task mismatch for %q", key) + glog.Warningf("\tExisting: %v", existing) + glog.Warningf("\tNew: %v", task) + + return fmt.Errorf("cannot add different task with same key %q", key) + } + } + c.Tasks[key] = task + return nil +} + func buildTaskKey(task Task) string { hasName, ok := task.(HasName) if !ok {