Relax DNS requirements on shared VPCs

Don't require EnableDNSHostnames on a shared VPC in >= 1.5.0

Create a feature flag for tolerating EnableDNSSupport=false.

Issue #786

pkg/featureflag/featureflag.go

@@ -37,6 +37,7 @@ func Bool(b bool) *bool {
 
 // DNSPreCreate controls whether we pre-create DNS records.
 var DNSPreCreate = New("DNSPreCreate", Bool(true))
+var VPCSkipEnableDNSSupport = New("VPCSkipEnableDNSSupport", Bool(true))
 
 var flags = make(map[string]*FeatureFlag)
 var flagsMutex sync.Mutex

pkg/model/context.go

@@ -18,8 +18,10 @@ package model
 
 import (
 	"fmt"
+	"github.com/blang/semver"
 	"github.com/golang/glog"
 	"k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/apis/kops/util"
 	"strings"
 )
 
@@ -190,3 +192,33 @@ func (m *KopsModelContext) UsePrivateDNS() bool {
 
 	return false
 }
+
+// KubernetesVersion parses the semver version of kubernetes, from the cluster spec
+func (c *KopsModelContext) KubernetesVersion() (semver.Version, error) {
+	kubernetesVersion := c.Cluster.Spec.KubernetesVersion
+
+	if kubernetesVersion == "" {
+		return semver.Version{}, fmt.Errorf("KubernetesVersion is required")
+	}
+
+	sv, err := util.ParseKubernetesVersion(kubernetesVersion)
+	if err != nil {
+		return semver.Version{}, fmt.Errorf("unable to determine kubernetes version from %q", kubernetesVersion)
+	}
+
+	return *sv, nil
+}
+
+// VersionGTE is a simplified semver comparison
+func VersionGTE(version semver.Version, major uint64, minor uint64) bool {
+	if version.Major > major {
+		return true
+	}
+	if version.Major > major {
+		return true
+	}
+	if version.Major == major && version.Minor >= minor {
+		return true
+	}
+	return false
+}

pkg/model/network.go

@@ -18,6 +18,7 @@ package model
 
 import (
 	"fmt"
+	"github.com/golang/glog"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
@@ -33,16 +34,30 @@ type NetworkModelBuilder struct {
 var _ fi.ModelBuilder = &NetworkModelBuilder{}
 
 func (b *NetworkModelBuilder) Build(c *fi.ModelBuilderContext) error {
+	kubernetesVersion, err := b.KubernetesVersion()
+	if err != nil {
+		return err
+	}
 
 	sharedVPC := b.Cluster.SharedVPC()
 
 	// VPC that holds everything for the cluster
 	{
 		t := &awstasks.VPC{
-			Name:               s(b.ClusterName()),
-			EnableDNSHostnames: fi.Bool(true),
-			EnableDNSSupport:   fi.Bool(true),
-			Shared:             fi.Bool(sharedVPC),
+			Name:             s(b.ClusterName()),
+			Shared:           fi.Bool(sharedVPC),
+			EnableDNSSupport: fi.Bool(true),
+		}
+
+		if sharedVPC && VersionGTE(kubernetesVersion, 1, 5) {
+			// If we're running k8s 1.5, and we have e.g.  --kubelet-preferred-address-types=InternalIP,Hostname,ExternalIP,LegacyHostIP
+			// then we don't need EnableDNSHostnames any more
+			glog.V(4).Infof("Kubernetes version %q; skipping EnableDNSHostnames requirement on VPC", kubernetesVersion)
+		} else {
+			// In theory we don't need to enable it for >= 1.5,
+			// but seems safer to stick with existing behaviour
+
+			t.EnableDNSHostnames = fi.Bool(true)
 		}
 
 		if b.Cluster.Spec.NetworkID != "" {

upup/pkg/fi/cloudup/awstasks/vpc.go

@@ -22,6 +22,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/golang/glog"
+	"k8s.io/kops/pkg/featureflag"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
 	"k8s.io/kops/upup/pkg/fi/cloudup/terraform"
@@ -132,11 +133,11 @@ func (_ *VPC) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *VPC) error {
 		}
 
 		if changes != nil && changes.EnableDNSSupport != nil {
-			return fmt.Errorf("VPC with id %q was set to be shared, but did not have EnableDNSSupport=true", fi.StringValue(e.ID))
-		}
-
-		if changes != nil && changes.EnableDNSHostnames != nil {
-			return fmt.Errorf("VPC with id %q was set to be shared, but did not have EnableDNSHostnames=true", fi.StringValue(e.ID))
+			if featureflag.VPCSkipEnableDNSSupport.Enabled() {
+				glog.Warningf("VPC did not have EnableDNSSupport=true, but ignoring because of VPCSkipEnableDNSSupport feature-flag")
+			} else {
+				return fmt.Errorf("VPC with id %q was set to be shared, but did not have EnableDNSSupport=true.", fi.StringValue(e.ID))
+			}
 		}
 
 		return nil