Skip to content

Commit

Permalink
Always infer gossip DNS from cluster name
Browse files Browse the repository at this point in the history
  • Loading branch information
hakman committed Oct 2, 2022
1 parent 4412d82 commit 8502614
Show file tree
Hide file tree
Showing 27 changed files with 64 additions and 61 deletions.
2 changes: 1 addition & 1 deletion nodeup/pkg/model/dns/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var _ fi.ModelBuilder = &GossipBuilder{}

// Build is responsible for configuring the gossip DNS tasks.
func (b *GossipBuilder) Build(c *fi.ModelBuilderContext) error {
useGossip := dns.IsGossipHostname(b.Cluster.Spec.MasterInternalName)
useGossip := dns.IsGossipCluster(b.Cluster)
if !useGossip {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion nodeup/pkg/model/kube_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func (b *KubeProxyBuilder) buildPod() (*v1.Pod, error) {
sslCertsHost.VolumeMount.MountPath = "/etc/ssl/certs"
}

if dns.IsGossipHostname(b.Cluster.Name) {
if dns.IsGossipCluster(b.Cluster) {
// Map /etc/hosts from host, so that we see the updates that are made by protokube
kubemanifest.AddHostPathMapping(pod, container, "etchosts", "/etc/hosts")
}
Expand Down
6 changes: 3 additions & 3 deletions nodeup/pkg/model/protokube.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ var _ fi.ModelBuilder = &ProtokubeBuilder{}

// Build is responsible for generating the options for protokube
func (t *ProtokubeBuilder) Build(c *fi.ModelBuilderContext) error {
useGossip := dns.IsGossipHostname(t.Cluster.Spec.MasterInternalName)
useGossip := dns.IsGossipCluster(t.Cluster)

// check is not a master and we are not using gossip (https://github.com/kubernetes/kops/pull/3091)
if !t.IsMaster && !useGossip {
Expand Down Expand Up @@ -209,8 +209,8 @@ func (t *ProtokubeBuilder) ProtokubeFlags(k8sVersion semver.Version) (*Protokube
// argv = append(argv, "--zone=*/*")
}

if dns.IsGossipHostname(t.Cluster.Spec.MasterInternalName) {
klog.Warningf("MasterInternalName %q implies gossip DNS", t.Cluster.Spec.MasterInternalName)
if dns.IsGossipCluster(t.Cluster) {
klog.Warningf("Cluster name %q implies gossip DNS", t.Cluster.Name)
f.Gossip = fi.Bool(true)
if t.Cluster.Spec.GossipConfig != nil {
f.GossipProtocol = t.Cluster.Spec.GossipConfig.Protocol
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/kops/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -1614,7 +1614,7 @@ func validateExternalDNS(cluster *kops.Cluster, spec *kops.ExternalDNSConfig, fl
}

if spec.Provider == kops.ExternalDNSProviderExternalDNS {
if dns.IsGossipHostname(cluster.Spec.MasterInternalName) {
if dns.IsGossipCluster(cluster) {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("provider"), "external-dns does not support gossip clusters"))
}
}
Expand Down
18 changes: 14 additions & 4 deletions pkg/dns/gossip.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,20 @@ limitations under the License.

package dns

import "strings"
import (
"strings"

kopsapi "k8s.io/kops/pkg/apis/kops"
)

// TODO: Are .local names necessarily invalid for "real DNS"? Do we need more qualification here?
func IsGossipHostname(name string) bool {
normalized := "." + strings.TrimSuffix(name, ".")
return strings.HasSuffix(normalized, ".k8s.local")
func IsGossipClusterName(name string) bool {
return strings.HasSuffix(strings.TrimSuffix(name, "."), ".k8s.local")
}

func IsGossipCluster(cluster *kopsapi.Cluster) bool {
if cluster == nil {
return false
}
return IsGossipClusterName(cluster.Name)
}
22 changes: 11 additions & 11 deletions pkg/dns/gossip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,29 +22,29 @@ import (

func TestIsGossipHostname(t *testing.T) {
tests := []struct {
hostname string
expected bool
clusterName string
expected bool
}{
{
hostname: "mycluster.k8s.local",
expected: true,
clusterName: "mycluster.k8s.local",
expected: true,
},
{
hostname: "mycluster.k8s.io",
expected: false,
clusterName: "mycluster.k8s.io",
expected: false,
},
{
hostname: "mycluster.k8s.local.",
expected: true,
clusterName: "mycluster.k8s.local.",
expected: true,
},
{
hostname: "k8s.local",
expected: true,
clusterName: "k8s.local",
expected: false,
},
}

for _, test := range tests {
result := IsGossipHostname(test.hostname)
result := IsGossipClusterName(test.clusterName)
if result != test.expected {
t.Errorf("Actual result %v, expected %v", result, test.expected)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubeconfig/create_kubecfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.Se
useELBName := false

// If the master DNS is a gossip DNS name; there's no way that name can resolve outside the cluster
if dns.IsGossipHostname(master) {
if dns.IsGossipCluster(cluster) {
useELBName = true
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/model/awsmodel/api_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
}
}

if dns.IsGossipHostname(b.Cluster.Name) || b.UsePrivateDNS() {
if dns.IsGossipCluster(b.Cluster) || b.UsePrivateDNS() {
// Ensure the LB hostname is included in the TLS certificate,
// if we're not going to use an alias for it
clb.ForAPIServer = true
Expand Down
8 changes: 4 additions & 4 deletions pkg/model/awsmodel/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type DNSModelBuilder struct {
var _ fi.ModelBuilder = &DNSModelBuilder{}

func (b *DNSModelBuilder) ensureDNSZone(c *fi.ModelBuilderContext) error {
if dns.IsGossipHostname(b.Cluster.Name) {
if dns.IsGossipCluster(b.Cluster) {
return nil
}

Expand Down Expand Up @@ -84,7 +84,7 @@ func (b *DNSModelBuilder) Build(c *fi.ModelBuilderContext) error {
// We now create the DNS Zone for AWS even in the case of public zones;
// it has to exist for the IAM record anyway.
// TODO: We can now rationalize the code paths
if !dns.IsGossipHostname(b.Cluster.Name) {
if !dns.IsGossipCluster(b.Cluster) {
if err := b.ensureDNSZone(c); err != nil {
return err
}
Expand All @@ -107,7 +107,7 @@ func (b *DNSModelBuilder) Build(c *fi.ModelBuilderContext) error {
// This will point our external DNS record to the load balancer, and put the
// pieces together for kubectl to work

if !dns.IsGossipHostname(b.Cluster.Name) {
if !dns.IsGossipCluster(b.Cluster) {
if err := b.ensureDNSZone(c); err != nil {
return err
}
Expand Down Expand Up @@ -137,7 +137,7 @@ func (b *DNSModelBuilder) Build(c *fi.ModelBuilderContext) error {
// This will point the internal API DNS record to the load balancer.
// This means kubelet connections go via the load balancer and are more HA.

if !dns.IsGossipHostname(b.Cluster.Name) {
if !dns.IsGossipCluster(b.Cluster) {
if err := b.ensureDNSZone(c); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/awsmodel/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (b *IAMModelBuilder) buildIAMRolePolicy(role iam.Subject, iamName string, i
},
}

if !dns.IsGossipHostname(b.Cluster.ObjectMeta.Name) {
if !dns.IsGossipCluster(b.Cluster) {
// This is slightly tricky; we need to know the hosted zone id,
// but we might be creating the hosted zone dynamically.
// We create a stub-reference which will be combined by the execution engine.
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/azuremodel/api_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (b *APILoadBalancerModelBuilder) Build(c *fi.ModelBuilderContext) error {

c.AddTask(lb)

if dns.IsGossipHostname(b.Cluster.Name) || b.UsePrivateDNS() {
if dns.IsGossipCluster(b.Cluster) || b.UsePrivateDNS() {
lb.ForAPIServer = true
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/model/components/etcdmanager/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ func (b *EtcdManagerBuilder) buildPod(etcdCluster kops.EtcdClusterSpec, instance
// The dns suffix logic mirrors the existing logic, so we should be compatible with existing clusters
// (etcd makes it difficult to change peer urls, treating it as a cluster event, for reasons unknown)
dnsInternalSuffix := ""
if dns.IsGossipHostname(b.Cluster.Spec.MasterInternalName) {
if dns.IsGossipCluster(b.Cluster) {
// @TODO: This is hacky, but we want it so that we can have a different internal & external name
dnsInternalSuffix = b.Cluster.Spec.MasterInternalName
dnsInternalSuffix = strings.TrimPrefix(dnsInternalSuffix, "api.")
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/components/kopscontroller/template_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type templateFunctions struct {

// KopsControllerConfig returns the yaml configuration for kops-controller
func (t *templateFunctions) GossipServices() ([]*corev1.Service, error) {
if !dns.IsGossipHostname(t.Cluster.Name) {
if !dns.IsGossipCluster(t.Cluster) {
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,5 +421,5 @@ func (b *KopsModelContext) NetworkingIsCilium() bool {

// IsGossip returns true if we are using gossip instead of "real" DNS
func (b *KopsModelContext) IsGossip() bool {
return dns.IsGossipHostname(b.Cluster.Name)
return dns.IsGossipCluster(b.Cluster)
}
2 changes: 1 addition & 1 deletion pkg/model/domodel/api_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (b *APILoadBalancerModelBuilder) Build(c *fi.ModelBuilderContext) error {
c.AddTask(loadbalancer)

// Temporarily do not know the role of the following function
if dns.IsGossipHostname(b.Cluster.Name) || b.UsePrivateDNS() {
if dns.IsGossipCluster(b.Cluster) || b.UsePrivateDNS() {
// Ensure the LB hostname is included in the TLS certificate,
// if we're not going to use an alias for it
loadbalancer.ForAPIServer = true
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/openstackmodel/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ func (b *FirewallModelBuilder) addCNIRules(c *fi.ModelBuilderContext, sgMap map[

// addProtokubeRules - Add rules for protokube if gossip DNS is enabled
func (b *FirewallModelBuilder) addProtokubeRules(c *fi.ModelBuilderContext, sgMap map[string]*openstacktasks.SecurityGroup) error {
if dns.IsGossipHostname(b.ClusterName()) {
if dns.IsGossipCluster(b.Cluster) {
masterName := b.SecurityGroupName(kops.InstanceGroupRoleMaster)
nodeName := b.SecurityGroupName(kops.InstanceGroupRoleNode)
masterSG := sgMap[masterName]
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/openstackmodel/servergroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func (b *ServerGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
}
c.AddTask(lbfipTask)

if dns.IsGossipHostname(b.Cluster.Name) || b.UsePrivateDNS() {
if dns.IsGossipCluster(b.Cluster) || b.UsePrivateDNS() {
b.associateFIPToKeypair(lbfipTask)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/resources/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -1808,7 +1808,7 @@ func deleteRoute53Records(cloud fi.Cloud, zone *route53.HostedZone, resourceTrac
func ListRoute53Records(cloud fi.Cloud, clusterName string) ([]*resources.Resource, error) {
var resourceTrackers []*resources.Resource

if dns.IsGossipHostname(clusterName) {
if dns.IsGossipClusterName(clusterName) {
return resourceTrackers, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/resources/gce/gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -1282,7 +1282,7 @@ func (d *clusterDiscoveryGCE) isKopsManagedDNSName(name string) bool {
}

func (d *clusterDiscoveryGCE) listGCEDNSZone() ([]*resources.Resource, error) {
if dns.IsGossipHostname(d.clusterName) {
if dns.IsGossipClusterName(d.clusterName) {
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/resources/openstack/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const (

func (os *clusterDiscoveryOS) ListDNSRecordsets() ([]*resources.Resource, error) {
// if dnsclient does not exist (designate disabled) or using gossip DNS
if os.osCloud.DNSClient() == nil || dns.IsGossipHostname(os.clusterName) {
if os.osCloud.DNSClient() == nil || dns.IsGossipClusterName(os.clusterName) {
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/validation/validate_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (v *clusterValidatorImpl) Validate() (*ValidationCluster, error) {
validation := &ValidationCluster{}

// Do not use if we are running gossip
if !dns.IsGossipHostname(clusterName) {
if !dns.IsGossipCluster(v.cluster) {
hasPlaceHolderIPAddress, err := hasPlaceHolderIP(v.host)
if err != nil {
return nil, err
Expand Down
8 changes: 4 additions & 4 deletions upup/pkg/fi/cloudup/apply_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
if cluster.Spec.KubernetesVersion == "" {
return fmt.Errorf("KubernetesVersion not set")
}
if cluster.Spec.DNSZone == "" && !dns.IsGossipHostname(cluster.ObjectMeta.Name) {
if cluster.Spec.DNSZone == "" && !dns.IsGossipCluster(cluster) {
return fmt.Errorf("DNSZone not set")
}

Expand Down Expand Up @@ -483,7 +483,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
modelContext.SSHPublicKeys = sshPublicKeys
modelContext.Region = cloud.Region()

if dns.IsGossipHostname(cluster.ObjectMeta.Name) {
if dns.IsGossipCluster(cluster) {
klog.V(2).Infof("Gossip DNS: skipping DNS validation")
} else {
err = validateDNS(cluster, cloud)
Expand Down Expand Up @@ -792,7 +792,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
return fmt.Errorf("error running tasks: %v", err)
}

if dns.IsGossipHostname(cluster.Name) {
if dns.IsGossipCluster(cluster) {
shouldPrecreateDNS = false
}

Expand Down Expand Up @@ -1306,7 +1306,7 @@ func (n *nodeUpConfigBuilder) BuildConfig(ig *kops.InstanceGroup, apiserverAddit
return nil, nil, fmt.Errorf("cannot determine role for instance group: %v", ig.ObjectMeta.Name)
}

useGossip := dns.IsGossipHostname(cluster.Spec.MasterInternalName)
useGossip := dns.IsGossipCluster(cluster)
isMaster := role == kops.InstanceGroupRoleMaster
hasAPIServer := isMaster || role == kops.InstanceGroupRoleAPIServer

Expand Down
15 changes: 4 additions & 11 deletions upup/pkg/fi/cloudup/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,25 +138,18 @@ func precreateDNS(ctx context.Context, cluster *kops.Cluster, cloud fi.Cloud) er
// This avoids hitting negative TTL on DNS lookups, which tend to be very long
// If we get the names wrong here, it doesn't really matter (extra DNS name, slower boot)

recordKeys := buildPrecreateDNSHostnames(cluster)

{
var filtered []recordKey
for _, recordKey := range recordKeys {
if !kopsdns.IsGossipHostname(recordKey.hostname) {
filtered = append(filtered, recordKey)
}
}
recordKeys = filtered
// Nothing to do for Gossip clusters
if !kopsdns.IsGossipCluster(cluster) {
return nil
}

recordKeys := buildPrecreateDNSHostnames(cluster)
if len(recordKeys) == 0 {
klog.V(2).Infof("No DNS records to pre-create")
return nil
}

klog.V(2).Infof("Checking DNS records")

zone, err := findZone(cluster, cloud)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions upup/pkg/fi/cloudup/new_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1227,7 +1227,7 @@ func setupTopology(opt *NewClusterOptions, cluster *api.Cluster, allZones sets.S
bastionGroup.Spec.MinSize = fi.Int32(1)
bastions = append(bastions, bastionGroup)

if !dns.IsGossipHostname(cluster.Name) {
if !dns.IsGossipCluster(cluster) {
cluster.Spec.Topology.Bastion = &api.BastionSpec{
PublicName: "bastion." + cluster.Name,
}
Expand Down Expand Up @@ -1291,7 +1291,7 @@ func setupAPI(opt *NewClusterOptions, cluster *api.Cluster) error {
} else {
switch cluster.Spec.Topology.Masters {
case api.TopologyPublic:
if dns.IsGossipHostname(cluster.Name) {
if dns.IsGossipCluster(cluster) {
// gossip DNS names don't work outside the cluster, so we use a LoadBalancer instead
cluster.Spec.API.LoadBalancer = &api.LoadBalancerAccessSpec{}
} else {
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/openstack/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func NewOpenstackCloud(tags map[string]string, spec *kops.ClusterSpec, uagent st
}

var dnsClient *gophercloud.ServiceClient
if !dns.IsGossipHostname(tags[TagClusterName]) {
if !dns.IsGossipClusterName(tags[TagClusterName]) {
// TODO: This should be replaced with the environment variable methods as done above
endpointOpt, err := config.GetServiceConfig("Designate")
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/populate_cluster_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (c *populateClusterSpec) run(clientset simple.Clientset) error {
klog.V(2).Infof("Normalizing kubernetes version: %q -> %q", cluster.Spec.KubernetesVersion, versionWithoutV)
cluster.Spec.KubernetesVersion = versionWithoutV
}
if cluster.Spec.DNSZone == "" && !dns.IsGossipHostname(cluster.ObjectMeta.Name) {
if cluster.Spec.DNSZone == "" && !dns.IsGossipCluster(cluster) {
dns, err := cloud.DNS()
if err != nil {
return err
Expand Down
Loading

0 comments on commit 8502614

Please sign in to comment.